Merge pull request #487 from basecamp/android/chrome-65-composition-h…

…otfix

Android: Ignore Chrome 65’s new composition events during cursor movement

src/trix/controllers/input/composition_input.coffee

@@ -1,3 +1,5 @@
+{browser} = Trix
+
 class Trix.CompositionInput extends Trix.BasicObject
   constructor: (@inputController) ->
     {@responder, @delegate, @inputSummary} = @inputController
@@ -6,35 +8,41 @@ class Trix.CompositionInput extends Trix.BasicObject
   start: (data) ->
     @data.start = data
 
-    if @inputSummary.eventName is "keypress" and @inputSummary.textAdded
-      @responder?.deleteInDirection("left")
+    if @isSignificant()
+      if @inputSummary.eventName is "keypress" and @inputSummary.textAdded
+        @responder?.deleteInDirection("left")
 
-    unless @selectionIsExpanded()
-      @insertPlaceholder()
-      @requestRender()
+      unless @selectionIsExpanded()
+        @insertPlaceholder()
+        @requestRender()
 
-    @range = @responder?.getSelectedRange()
+      @range = @responder?.getSelectedRange()
 
   update: (data) ->
     @data.update = data
 
-    if range = @selectPlaceholder()
-      @forgetPlaceholder()
-      @range = range
+    if @isSignificant()
+      if range = @selectPlaceholder()
+        @forgetPlaceholder()
+        @range = range
 
   end: (data) ->
     @data.end = data
-    @forgetPlaceholder()
 
-    if @canApplyToDocument()
-      @setInputSummary(preferDocument: true)
-      @delegate?.inputControllerWillPerformTyping()
-      @responder?.setSelectedRange(@range)
-      @responder?.insertString(@data.end)
-      @responder?.setSelectedRange(@range[0] + @data.end.length)
+    if @isSignificant()
+      @forgetPlaceholder()
 
-    else if @data.start? or @data.update?
-      @requestReparse()
+      if @canApplyToDocument()
+        @setInputSummary(preferDocument: true, didInput: false)
+        @delegate?.inputControllerWillPerformTyping()
+        @responder?.setSelectedRange(@range)
+        @responder?.insertString(@data.end)
+        @responder?.setSelectedRange(@range[0] + @data.end.length)
+
+      else if @data.start? or @data.update?
+        @requestReparse()
+        @inputController.reset()
+    else
       @inputController.reset()
 
   getEndData: ->
@@ -43,6 +51,12 @@ class Trix.CompositionInput extends Trix.BasicObject
   isEnded: ->
     @getEndData()?
 
+  isSignificant: ->
+    if browser.composesExistingText
+      @inputSummary.didInput
+    else
+      true
+
   # Private
 
   canApplyToDocument: ->

src/trix/controllers/input_controller.coffee

@@ -129,6 +129,7 @@ class Trix.InputController extends Trix.BasicObject
   events:
     keydown: (event) ->
       @resetInputSummary() unless @isComposing()
+      @inputSummary.didInput = true
 
       if keyName = @constructor.keyNames[event.keyCode]
         context = @keys
@@ -297,7 +298,11 @@ class Trix.InputController extends Trix.BasicObject
     compositionend: (event) ->
       @getCompositionInput().end(event.data)
 
+    beforeinput: (event) ->
+      @inputSummary.didInput = true
+
     input: (event) ->
+      @inputSummary.didInput = true
       event.stopPropagation()
 
   keys:

src/trix/elements/trix_editor_element.coffee

@@ -1,7 +1,7 @@
 #= require trix/elements/trix_toolbar_element
 #= require trix/controllers/editor_controller
 
-{makeElement, triggerEvent, handleEvent, handleEventOnce} = Trix
+{browser, makeElement, triggerEvent, handleEvent, handleEventOnce} = Trix
 
 {attachmentSelector} = Trix.AttachmentView
 
@@ -37,11 +37,8 @@ Trix.registerElement "trix-editor", do ->
 
   # Style
 
-  # IE 11 activates resizing handles on editable elements that have "layout"
-  browserForcesObjectResizing = /Trident.*rv:11/.test(navigator.userAgent)
-
   cursorTargetStyles = do ->
-    if browserForcesObjectResizing
+    if browser.forcesObjectResizing
       display: "inline"
       width: "auto"
     else

src/trix/index.coffee.erb

@@ -11,4 +11,11 @@
   NON_BREAKING_SPACE: "\u00A0"
   OBJECT_REPLACEMENT_CHARACTER: "\uFFFC"
 
+  browser:
+    # Android emits composition events when moving the cursor through existing text
+    # Introduced in Chrome 65: https://bugs.chromium.org/p/chromium/issues/detail?id=764439#c9
+    composesExistingText: /Android.*Chrome/.test(navigator.userAgent)
+    # IE 11 activates resizing handles on editable elements that have "layout"
+    forcesObjectResizing: /Trident.*rv:11/.test(navigator.userAgent)
+
   config: {}

test/src/system/composition_input_test.coffee

@@ -1,4 +1,5 @@
-{assert, clickToolbarButton, defer, endComposition, insertNode, pressKey, selectNode, startComposition, test, testGroup, triggerEvent, typeCharacters, updateComposition} = Trix.TestHelpers
+{assert, clickToolbarButton, defer, endComposition, insertNode, pressKey, selectNode, startComposition, test, testIf, testGroup, triggerEvent, typeCharacters, updateComposition} = Trix.TestHelpers
+{browser} = Trix
 
 testGroup "Composition input", template: "editor_empty", ->
   test "composing", (expectDocument) ->
@@ -66,9 +67,11 @@ testGroup "Composition input", template: "editor_empty", ->
 
     triggerEvent(element, "keydown", charCode: 0, keyCode: 229, which: 229)
     triggerEvent(element, "compositionupdate", data: "ca")
+    triggerEvent(element, "input")
     removeCharacters -1, ->
       triggerEvent(element, "keydown", charCode: 0, keyCode: 229, which: 229)
       triggerEvent(element, "compositionupdate", data: "c")
+      triggerEvent(element, "input")
       triggerEvent(element, "compositionend", data: "c")
       removeCharacters -1, ->
         pressKey "backspace", ->
@@ -83,9 +86,11 @@ testGroup "Composition input", template: "editor_empty", ->
     triggerEvent(element, "keydown", charCode: 0, keyCode: 229, which: 229)
     triggerEvent(element, "compositionstart", data: "cat")
     triggerEvent(element, "compositionupdate", data: "cat")
+    triggerEvent(element, "input")
     removeCharacters -1, ->
       triggerEvent(element, "keydown", charCode: 0, keyCode: 229, which: 229)
       triggerEvent(element, "compositionupdate", data: "car")
+      triggerEvent(element, "input")
       triggerEvent(element, "compositionend", data: "car")
       insertNode document.createTextNode("r"), ->
         expectDocument("car\n")
@@ -97,32 +102,58 @@ testGroup "Composition input", template: "editor_empty", ->
     triggerEvent(element, "keydown", charCode: 0, keyCode: 229, which: 229)
     triggerEvent(element, "compositionstart", data: "")
     triggerEvent(element, "compositionupdate", data: "c")
+    triggerEvent(element, "input")
     node = document.createTextNode("c")
     insertNode(node)
     defer ->
       triggerEvent(element, "keydown", charCode: 0, keyCode: 229, which: 229)
       triggerEvent(element, "compositionupdate", data: "ca")
+      triggerEvent(element, "input")
       node.data = "ca"
       defer ->
         triggerEvent(element, "compositionend", data: "")
         defer ->
           expectDocument("ca\n")
 
+  testIf browser.composesExistingText, "composition events from cursor movement are ignored", (expectDocument) ->
+    element = getEditorElement()
+    element.editor.insertString("ab ")
+
+    element.editor.setSelectedRange(0)
+    triggerEvent(element, "compositionstart", data: "")
+    triggerEvent(element, "compositionupdate", data: "ab")
+    defer ->
+      element.editor.setSelectedRange(1)
+      triggerEvent(element, "compositionupdate", data: "ab")
+      defer ->
+        element.editor.setSelectedRange(2)
+        triggerEvent(element, "compositionupdate", data: "ab")
+        defer ->
+          element.editor.setSelectedRange(3)
+          triggerEvent(element, "compositionend", data: "ab")
+          defer ->
+            expectDocument("ab \n")
+
   # Simulates compositions in Firefox where the final composition data is
   # dispatched as both compositionupdate and compositionend.
   test "composition ending with same data as last update", (expectDocument) ->
     element = getEditorElement()
 
+    triggerEvent(element, "keydown", charCode: 0, keyCode: 229, which: 229)
     triggerEvent(element, "compositionstart", data: "")
     triggerEvent(element, "compositionupdate", data: "´")
     node = document.createTextNode("´")
     insertNode(node)
     selectNode(node)
     defer ->
+      triggerEvent(element, "keydown", charCode: 0, keyCode: 229, which: 229)
       triggerEvent(element, "compositionupdate", data: "é")
+      triggerEvent(element, "input")
       node.data = "é"
       defer ->
+        triggerEvent(element, "keydown", charCode: 0, keyCode: 229, which: 229)
         triggerEvent(element, "compositionupdate", data: "éé")
+        triggerEvent(element, "input")
         node.data = "éé"
         defer ->
           triggerEvent(element, "compositionend", data: "éé")

test/src/system/custom_element_test.coffee

@@ -1,4 +1,4 @@
-{after, assert, clickElement, clickToolbarButton, createFile, defer, insertImageAttachment, moveCursor, pasteContent, skip, test, testGroup, triggerEvent, typeCharacters, typeInToolbarDialog} = Trix.TestHelpers
+{after, assert, clickElement, clickToolbarButton, createFile, defer, insertImageAttachment, moveCursor, pasteContent, skip, test, testIf, testGroup, triggerEvent, typeCharacters, typeInToolbarDialog} = Trix.TestHelpers
 
 testGroup "Custom element API", template: "editor_empty", ->
   test "files are accepted by default", ->
@@ -286,8 +286,7 @@ testGroup "Custom element API", template: "editor_empty", ->
 
   # Selenium doesn't seem to focus windows properly in some browsers (FF 47 on OS X)
   # so skip this test when unfocused pending a better solution.
-  testOrSkip = if document.hasFocus() then test else skip
-  testOrSkip "element triggers custom focus event when autofocusing", (done) ->
+  testIf document.hasFocus(), "element triggers custom focus event when autofocusing", (done) ->
     element = document.createElement("trix-editor")
     element.setAttribute("autofocus", "")
 

test/src/test_helpers/test_helpers.coffee

@@ -69,4 +69,10 @@ helpers.extend
         else
           callback(done)
 
+  testIf: (condition, args...) ->
+    if condition
+      helpers.test(args...)
+    else
+      helpers.skip(args...)
+
   skip: QUnit.skip