From e5ca869239ef1d021892f0517f18497c35c3a9b1 Mon Sep 17 00:00:00 2001 From: Javan Makhmali Date: Mon, 26 Feb 2018 15:15:56 -0800 Subject: [PATCH 1/6] =?UTF-8?q?Android:=20Ignore=20Chrome=2065=E2=80=99s?= =?UTF-8?q?=20new=20composition=20events=20during=20cursor=20movement?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../input/composition_input.coffee | 45 +++++++++++-------- src/trix/controllers/input_controller.coffee | 5 +++ test/src/system/composition_input_test.coffee | 34 ++++++++++++++ 3 files changed, 66 insertions(+), 18 deletions(-) diff --git a/src/trix/controllers/input/composition_input.coffee b/src/trix/controllers/input/composition_input.coffee index 0d12ecdea..034a50be9 100644 --- a/src/trix/controllers/input/composition_input.coffee +++ b/src/trix/controllers/input/composition_input.coffee @@ -6,35 +6,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() + + if @canApplyToDocument() + @setInputSummary(preferDocument: true) + @delegate?.inputControllerWillPerformTyping() + @responder?.setSelectedRange(@range) + @responder?.insertString(@data.end) + @responder?.setSelectedRange(@range[0] + @data.end.length) - else if @data.start? or @data.update? - @requestReparse() + else if @data.start? or @data.update? + @requestReparse() + @inputController.reset() + else @inputController.reset() getEndData: -> @@ -43,6 +49,9 @@ class Trix.CompositionInput extends Trix.BasicObject isEnded: -> @getEndData()? + isSignificant: -> + @inputSummary.didInput + # Private canApplyToDocument: -> diff --git a/src/trix/controllers/input_controller.coffee b/src/trix/controllers/input_controller.coffee index b06823f5e..d3278bf25 100644 --- a/src/trix/controllers/input_controller.coffee +++ b/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: diff --git a/test/src/system/composition_input_test.coffee b/test/src/system/composition_input_test.coffee index fc79abb0d..4af3373be 100644 --- a/test/src/system/composition_input_test.coffee +++ b/test/src/system/composition_input_test.coffee @@ -66,9 +66,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 +85,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 +101,62 @@ 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") + # Simulates the sequence of events when moving the cursor through a word on Android + # Introduced in Chrome 65: + # - https://bugs.chromium.org/p/chromium/issues/detail?id=812674 + # - https://bugs.chromium.org/p/chromium/issues/detail?id=764439#c9 + test "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: "éé") From 89c954485f2f6c16a71c2364443f344e7e7c99eb Mon Sep 17 00:00:00 2001 From: Javan Makhmali Date: Thu, 1 Mar 2018 11:08:39 -0800 Subject: [PATCH 2/6] Fix handling compositions in Firefox on Linux Fixes #489 --- src/trix/controllers/input_controller.coffee | 3 +++ test/src/system/composition_input_test.coffee | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/trix/controllers/input_controller.coffee b/src/trix/controllers/input_controller.coffee index d3278bf25..865495311 100644 --- a/src/trix/controllers/input_controller.coffee +++ b/src/trix/controllers/input_controller.coffee @@ -161,6 +161,9 @@ class Trix.InputController extends Trix.BasicObject @responder?.insertString(string) @setInputSummary(textAdded: string, didDelete: @selectionIsExpanded()) + keyup: (event) -> + @inputSummary.didInput = true + textInput: (event) -> # Handle autocapitalization {data} = event diff --git a/test/src/system/composition_input_test.coffee b/test/src/system/composition_input_test.coffee index 4af3373be..373d2204d 100644 --- a/test/src/system/composition_input_test.coffee +++ b/test/src/system/composition_input_test.coffee @@ -164,6 +164,26 @@ testGroup "Composition input", template: "editor_empty", -> assert.locationRange(index: 0, offset: 2) expectDocument("éé\n") + # Simulates compositions in Firefox on Linux, which aren't preceded by + # keydown and don't dispatch input after compositionupdate. + test "composition with limited contextual keyboard and input events", (expectDocument) -> + element = getEditorElement() + element.editor.insertString("i") + element.editor.setSelectedRange(1) + defer -> + triggerEvent(element, "keyup", charCode: 0, keyCode: 0, which: 0) + triggerEvent(element, "compositionstart", data: "") + triggerEvent(element, "compositionupdate", data: "ó") + node = document.createTextNode("ó") + insertNode(node) + selectNode(node) + defer -> + triggerEvent(element, "compositionend", data: "ó") + triggerEvent(element, "input") + typeCharacters "n", -> + assert.locationRange(index: 0, offset: 3) + expectDocument("ión\n") + removeCharacters = (direction, callback) -> selection = rangy.getSelection() range = selection.getRangeAt(0) From acff0449ab04ba1e14d4b5d8389bbabc6022ca85 Mon Sep 17 00:00:00 2001 From: Javan Makhmali Date: Thu, 1 Mar 2018 12:14:30 -0800 Subject: [PATCH 3/6] Revert "Fix handling compositions in Firefox on Linux" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 89c954485f2f6c16a71c2364443f344e7e7c99eb. Handling “keyup” reintroduces the Android issue after the next cursor movement --- src/trix/controllers/input_controller.coffee | 3 --- test/src/system/composition_input_test.coffee | 20 ------------------- 2 files changed, 23 deletions(-) diff --git a/src/trix/controllers/input_controller.coffee b/src/trix/controllers/input_controller.coffee index 865495311..d3278bf25 100644 --- a/src/trix/controllers/input_controller.coffee +++ b/src/trix/controllers/input_controller.coffee @@ -161,9 +161,6 @@ class Trix.InputController extends Trix.BasicObject @responder?.insertString(string) @setInputSummary(textAdded: string, didDelete: @selectionIsExpanded()) - keyup: (event) -> - @inputSummary.didInput = true - textInput: (event) -> # Handle autocapitalization {data} = event diff --git a/test/src/system/composition_input_test.coffee b/test/src/system/composition_input_test.coffee index 373d2204d..4af3373be 100644 --- a/test/src/system/composition_input_test.coffee +++ b/test/src/system/composition_input_test.coffee @@ -164,26 +164,6 @@ testGroup "Composition input", template: "editor_empty", -> assert.locationRange(index: 0, offset: 2) expectDocument("éé\n") - # Simulates compositions in Firefox on Linux, which aren't preceded by - # keydown and don't dispatch input after compositionupdate. - test "composition with limited contextual keyboard and input events", (expectDocument) -> - element = getEditorElement() - element.editor.insertString("i") - element.editor.setSelectedRange(1) - defer -> - triggerEvent(element, "keyup", charCode: 0, keyCode: 0, which: 0) - triggerEvent(element, "compositionstart", data: "") - triggerEvent(element, "compositionupdate", data: "ó") - node = document.createTextNode("ó") - insertNode(node) - selectNode(node) - defer -> - triggerEvent(element, "compositionend", data: "ó") - triggerEvent(element, "input") - typeCharacters "n", -> - assert.locationRange(index: 0, offset: 3) - expectDocument("ión\n") - removeCharacters = (direction, callback) -> selection = rangy.getSelection() range = selection.getRangeAt(0) From 0e5970b0d496b11a865f863c3bf81a1b3aca01e4 Mon Sep 17 00:00:00 2001 From: Javan Makhmali Date: Thu, 1 Mar 2018 14:40:45 -0800 Subject: [PATCH 4/6] Handle composition events conditionally on Android only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can’t rely on all other browsers to emit keyboard and/or input events during compositions. Especially not in the same order. Fixes #48 --- src/trix/controllers/input/composition_input.coffee | 7 ++++++- src/trix/index.coffee.erb | 5 +++++ test/src/system/composition_input_test.coffee | 9 +++------ test/src/system/custom_element_test.coffee | 5 ++--- test/src/test_helpers/test_helpers.coffee | 6 ++++++ 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/trix/controllers/input/composition_input.coffee b/src/trix/controllers/input/composition_input.coffee index 034a50be9..ed8987d3d 100644 --- a/src/trix/controllers/input/composition_input.coffee +++ b/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 @@ -50,7 +52,10 @@ class Trix.CompositionInput extends Trix.BasicObject @getEndData()? isSignificant: -> - @inputSummary.didInput + if browser.composesExistingText + @inputSummary.didInput + else + true # Private diff --git a/src/trix/index.coffee.erb b/src/trix/index.coffee.erb index 67ccaea84..0885cf80f 100644 --- a/src/trix/index.coffee.erb +++ b/src/trix/index.coffee.erb @@ -11,4 +11,9 @@ 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) + config: {} diff --git a/test/src/system/composition_input_test.coffee b/test/src/system/composition_input_test.coffee index 4af3373be..40546994b 100644 --- a/test/src/system/composition_input_test.coffee +++ b/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) -> @@ -114,11 +115,7 @@ testGroup "Composition input", template: "editor_empty", -> defer -> expectDocument("ca\n") - # Simulates the sequence of events when moving the cursor through a word on Android - # Introduced in Chrome 65: - # - https://bugs.chromium.org/p/chromium/issues/detail?id=812674 - # - https://bugs.chromium.org/p/chromium/issues/detail?id=764439#c9 - test "composition events from cursor movement are ignored", (expectDocument) -> + testIf browser.composesExistingText, "composition events from cursor movement are ignored", (expectDocument) -> element = getEditorElement() element.editor.insertString("ab ") diff --git a/test/src/system/custom_element_test.coffee b/test/src/system/custom_element_test.coffee index c53a5dc69..152e1af20 100644 --- a/test/src/system/custom_element_test.coffee +++ b/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", "") diff --git a/test/src/test_helpers/test_helpers.coffee b/test/src/test_helpers/test_helpers.coffee index 83fc25153..729b97b37 100644 --- a/test/src/test_helpers/test_helpers.coffee +++ b/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 From 6ef9bbeeb0ff2df8ff1fb4b85180ad11c65d12ed Mon Sep 17 00:00:00 2001 From: Javan Makhmali Date: Thu, 1 Mar 2018 14:44:33 -0800 Subject: [PATCH 5/6] Consolidate feature / irritant tests --- src/trix/elements/trix_editor_element.coffee | 7 ++----- src/trix/index.coffee.erb | 2 ++ 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/trix/elements/trix_editor_element.coffee b/src/trix/elements/trix_editor_element.coffee index 4803ed9c6..0262dcce6 100644 --- a/src/trix/elements/trix_editor_element.coffee +++ b/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 diff --git a/src/trix/index.coffee.erb b/src/trix/index.coffee.erb index 0885cf80f..4953a6815 100644 --- a/src/trix/index.coffee.erb +++ b/src/trix/index.coffee.erb @@ -15,5 +15,7 @@ # 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: {} From 0754994239db9277774c7462bb7d8088fd0c3702 Mon Sep 17 00:00:00 2001 From: Javan Makhmali Date: Thu, 1 Mar 2018 15:04:55 -0800 Subject: [PATCH 6/6] Android: Fix ending a composition by moving the cursor to existing text The next compositionstart can come before the mutation so `didInput` needs to be flipped back to avoid handling it. --- src/trix/controllers/input/composition_input.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/trix/controllers/input/composition_input.coffee b/src/trix/controllers/input/composition_input.coffee index ed8987d3d..5dbd642b2 100644 --- a/src/trix/controllers/input/composition_input.coffee +++ b/src/trix/controllers/input/composition_input.coffee @@ -33,7 +33,7 @@ class Trix.CompositionInput extends Trix.BasicObject @forgetPlaceholder() if @canApplyToDocument() - @setInputSummary(preferDocument: true) + @setInputSummary(preferDocument: true, didInput: false) @delegate?.inputControllerWillPerformTyping() @responder?.setSelectedRange(@range) @responder?.insertString(@data.end)