From 4d151722fff14f43792fbbe8d7f7ef5a5d5398b8 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 24 Sep 2019 15:32:08 +0200 Subject: [PATCH 1/6] delay DOM modification after compositionend --- src/components/views/rooms/BasicMessageComposer.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/BasicMessageComposer.js b/src/components/views/rooms/BasicMessageComposer.js index 110df355fe..14c850b9a3 100644 --- a/src/components/views/rooms/BasicMessageComposer.js +++ b/src/components/views/rooms/BasicMessageComposer.js @@ -170,8 +170,13 @@ export default class BasicMessageEditor extends React.Component { _onCompositionEnd = (event) => { this._isIMEComposing = false; // some browsers (chromium) don't fire an input event after ending a composition - // so trigger a model update after the composition is done by calling the input handler - this._onInput({inputType: "insertCompositionText"}); + // so trigger a model update after the composition is done by calling the input handler. + // do this async though, as modifying the DOM from the compositionend event might confuse the composition. + setTimeout(() => { + this._onInput({inputType: "insertCompositionText"}); + }, 0); + } + } _onPaste = (event) => { From 9f47fad3057c1a3b591e59bc86c814b3482833cc Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 24 Sep 2019 15:32:30 +0200 Subject: [PATCH 2/6] ignore keydown events while doing IME composition --- src/components/views/rooms/BasicMessageComposer.js | 2 ++ src/components/views/rooms/EditMessageComposer.js | 3 +++ src/components/views/rooms/SendMessageComposer.js | 3 +++ 3 files changed, 8 insertions(+) diff --git a/src/components/views/rooms/BasicMessageComposer.js b/src/components/views/rooms/BasicMessageComposer.js index 14c850b9a3..40142d4436 100644 --- a/src/components/views/rooms/BasicMessageComposer.js +++ b/src/components/views/rooms/BasicMessageComposer.js @@ -177,6 +177,8 @@ export default class BasicMessageEditor extends React.Component { }, 0); } + shouldIgnoreKeyDownEvents() { + return this._isIMEComposing; } _onPaste = (event) => { diff --git a/src/components/views/rooms/EditMessageComposer.js b/src/components/views/rooms/EditMessageComposer.js index a1d6fa618f..c744711f61 100644 --- a/src/components/views/rooms/EditMessageComposer.js +++ b/src/components/views/rooms/EditMessageComposer.js @@ -127,6 +127,9 @@ export default class EditMessageComposer extends React.Component { } _onKeyDown = (event) => { + if (this._editorRef.shouldIgnoreKeyDownEvents()) { + return; + } if (event.metaKey || event.altKey || event.shiftKey) { return; } diff --git a/src/components/views/rooms/SendMessageComposer.js b/src/components/views/rooms/SendMessageComposer.js index f6e5830329..d98a1a55cb 100644 --- a/src/components/views/rooms/SendMessageComposer.js +++ b/src/components/views/rooms/SendMessageComposer.js @@ -104,6 +104,9 @@ export default class SendMessageComposer extends React.Component { }; _onKeyDown = (event) => { + if (this._editorRef.shouldIgnoreKeyDownEvents()) { + return; + } const hasModifier = event.altKey || event.ctrlKey || event.metaKey || event.shiftKey; if (event.key === "Enter" && !hasModifier) { this._sendMessage(); From 7bda1c58ebf6a4306abbe24dffbef5028a954c82 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Tue, 24 Sep 2019 15:36:46 +0200 Subject: [PATCH 3/6] better naming --- src/components/views/rooms/BasicMessageComposer.js | 6 +++--- src/components/views/rooms/EditMessageComposer.js | 2 +- src/components/views/rooms/SendMessageComposer.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/views/rooms/BasicMessageComposer.js b/src/components/views/rooms/BasicMessageComposer.js index 40142d4436..ac528f07eb 100644 --- a/src/components/views/rooms/BasicMessageComposer.js +++ b/src/components/views/rooms/BasicMessageComposer.js @@ -173,12 +173,12 @@ export default class BasicMessageEditor extends React.Component { // so trigger a model update after the composition is done by calling the input handler. // do this async though, as modifying the DOM from the compositionend event might confuse the composition. setTimeout(() => { - this._onInput({inputType: "insertCompositionText"}); + this._onInput({inputType: "insertCompositionText"}, true); }, 0); } - shouldIgnoreKeyDownEvents() { - return this._isIMEComposing; + isComposing(event) { + return !!(this._isIMEComposing || event.isComposing); } _onPaste = (event) => { diff --git a/src/components/views/rooms/EditMessageComposer.js b/src/components/views/rooms/EditMessageComposer.js index c744711f61..1a7abb45fb 100644 --- a/src/components/views/rooms/EditMessageComposer.js +++ b/src/components/views/rooms/EditMessageComposer.js @@ -127,7 +127,7 @@ export default class EditMessageComposer extends React.Component { } _onKeyDown = (event) => { - if (this._editorRef.shouldIgnoreKeyDownEvents()) { + if (this._editorRef.isComposing(event)) { return; } if (event.metaKey || event.altKey || event.shiftKey) { diff --git a/src/components/views/rooms/SendMessageComposer.js b/src/components/views/rooms/SendMessageComposer.js index d98a1a55cb..6fc53492d3 100644 --- a/src/components/views/rooms/SendMessageComposer.js +++ b/src/components/views/rooms/SendMessageComposer.js @@ -104,7 +104,7 @@ export default class SendMessageComposer extends React.Component { }; _onKeyDown = (event) => { - if (this._editorRef.shouldIgnoreKeyDownEvents()) { + if (this._editorRef.isComposing(event)) { return; } const hasModifier = event.altKey || event.ctrlKey || event.metaKey || event.shiftKey; From ffe34ee8a1ea2f643b6160603057f89de9cdcb4c Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 25 Sep 2019 10:33:52 +0200 Subject: [PATCH 4/6] try to see if this fixes safari back on of the 2 changes (updating dom async from compositionend, or ignoring keydown while composing) here has, while fixing chrome, broken safari. Don't do the async dom updating for safari if that was it. --- .../views/rooms/BasicMessageComposer.js | 24 +++++++++++++++---- .../views/rooms/EditMessageComposer.js | 1 + .../views/rooms/SendMessageComposer.js | 1 + 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/components/views/rooms/BasicMessageComposer.js b/src/components/views/rooms/BasicMessageComposer.js index ac528f07eb..2e0bc5f395 100644 --- a/src/components/views/rooms/BasicMessageComposer.js +++ b/src/components/views/rooms/BasicMessageComposer.js @@ -169,15 +169,31 @@ export default class BasicMessageEditor extends React.Component { _onCompositionEnd = (event) => { this._isIMEComposing = false; - // some browsers (chromium) don't fire an input event after ending a composition + // some browsers (Chrome) don't fire an input event after ending a composition, // so trigger a model update after the composition is done by calling the input handler. - // do this async though, as modifying the DOM from the compositionend event might confuse the composition. - setTimeout(() => { + + // however, modifying the DOM (caused by the editor model update) from the compositionend handler seems + // to confuse the IME in Chrome, likely causing https://github.com/vector-im/riot-web/issues/10913 , + // so we do it async + + // however, doing this async seems to break things in Safari for some reason, so browser sniff. + + const ua = navigator.userAgent.toLowerCase(); + const isSafari = ua.includes('safari/') && !ua.includes('chrome/'); + + if (isSafari) { this._onInput({inputType: "insertCompositionText"}, true); - }, 0); + } else { + setTimeout(() => { + this._onInput({inputType: "insertCompositionText"}, true); + }, 0); + } } isComposing(event) { + // checking the event.isComposing flag just in case any browser out there + // emits events related to the composition after compositionend + // has been fired return !!(this._isIMEComposing || event.isComposing); } diff --git a/src/components/views/rooms/EditMessageComposer.js b/src/components/views/rooms/EditMessageComposer.js index 1a7abb45fb..3430e793ac 100644 --- a/src/components/views/rooms/EditMessageComposer.js +++ b/src/components/views/rooms/EditMessageComposer.js @@ -127,6 +127,7 @@ export default class EditMessageComposer extends React.Component { } _onKeyDown = (event) => { + // ignore any keypress while doing IME compositions if (this._editorRef.isComposing(event)) { return; } diff --git a/src/components/views/rooms/SendMessageComposer.js b/src/components/views/rooms/SendMessageComposer.js index 6fc53492d3..c8f8ab1293 100644 --- a/src/components/views/rooms/SendMessageComposer.js +++ b/src/components/views/rooms/SendMessageComposer.js @@ -104,6 +104,7 @@ export default class SendMessageComposer extends React.Component { }; _onKeyDown = (event) => { + // ignore any keypress while doing IME compositions if (this._editorRef.isComposing(event)) { return; } From c8af1a62561d0771b6e7d353840c40429b005782 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 25 Sep 2019 14:59:27 +0200 Subject: [PATCH 5/6] fixup: remove flag --- src/components/views/rooms/BasicMessageComposer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/rooms/BasicMessageComposer.js b/src/components/views/rooms/BasicMessageComposer.js index 2e0bc5f395..0601b9b84f 100644 --- a/src/components/views/rooms/BasicMessageComposer.js +++ b/src/components/views/rooms/BasicMessageComposer.js @@ -182,10 +182,10 @@ export default class BasicMessageEditor extends React.Component { const isSafari = ua.includes('safari/') && !ua.includes('chrome/'); if (isSafari) { - this._onInput({inputType: "insertCompositionText"}, true); + this._onInput({inputType: "insertCompositionText"}); } else { setTimeout(() => { - this._onInput({inputType: "insertCompositionText"}, true); + this._onInput({inputType: "insertCompositionText"}); }, 0); } } From 30af9a9056c1be479d0048c8af61b874ee9febb6 Mon Sep 17 00:00:00 2001 From: Bruno Windels Date: Wed, 25 Sep 2019 16:11:37 +0200 Subject: [PATCH 6/6] need to check isComposing on native event --- src/components/views/rooms/BasicMessageComposer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/BasicMessageComposer.js b/src/components/views/rooms/BasicMessageComposer.js index 0601b9b84f..c229f64d1a 100644 --- a/src/components/views/rooms/BasicMessageComposer.js +++ b/src/components/views/rooms/BasicMessageComposer.js @@ -194,7 +194,7 @@ export default class BasicMessageEditor extends React.Component { // checking the event.isComposing flag just in case any browser out there // emits events related to the composition after compositionend // has been fired - return !!(this._isIMEComposing || event.isComposing); + return !!(this._isIMEComposing || (event.nativeEvent && event.nativeEvent.isComposing)); } _onPaste = (event) => {