RichText: fix isComposing for Safari #19171
Conversation
ellatrix added [Type] Bug ellatrix requested review from daniloercoli, etoledom, jorgefilipecosta and SergioEstevao as code owners
| document.removeEventListener( 'selectionchange', this.onSelectionChange ); | ||
| // Do not trigger a change if characters are being composed. Browsers | ||
| // will usually emit a final `input` event when the characters are | ||
| // composed. |
There was a problem hiding this comment.
Thanks for all the comments. Maybe expand this one to reflect the current change, e.g.
… As of December 2019, Safari doesn't honour
nativeEvent.isComposing.
Incidentally, that could make it easier — later on, in a year or ten — to know what workarounds can be eliminated in our code base.
There was a problem hiding this comment.
Yes, I should add something here. I'm also wondering if I should still use nativeEvent.isComposing and fall back to this.isComposing. Normally compositionstop should fire, but who knows, if in some rare cases it doesn't, the value would be stuck at true until composing is started again. nativeEvent.isComposing is probably more reliable. On the other hand, always using this.isComposing will more quickly surface any bugs.
There was a problem hiding this comment.
I don't oppose to listening to those two sources, but is there anything making you doubt of compositionstop's reliability? Since it's a proper SyntheticEvent, I would expect adequate browser support.
There was a problem hiding this comment.
Yes, browser support is good, but I wouldn't be surprised if it's implemented slightly differently in browsers, just like the calling order of all these event handlers, or not called at all in some situations. Let's try this and keep an eye on reports.
Description
Fixes #19161.
Apparently Safari doesn't support
isComposingon aninputevent. See https://developer.mozilla.org/en-US/docs/Web/API/InputEvent/isComposing.The solution is to set a flag on
compositionstartand remove andcompositionend.Unfortunately, we cannot add an e2e test right now. See puppeteer/puppeteer#4981.
How has this been tested?
For an American keyboard:
Cmd+B, thenAlt+`, then type "a". "à" should be bold.Screenshots
Types of changes
Bug fix
Checklist: