IME/typing bug-fixing #16289
IME/typing bug-fixing #16289
Conversation
Fix (typing, engine): Predictive text should not get doubled while typing. Closes #16106.
Fix (engine): The reverse typing effect should not happen after the focus change. Closes #14702.
…ges, not as a normal flow in IME. Composed text on Android should be inserted to the model only after it is available in the DOM (so on beforeinput is too early). So no mutations other by those made by IME.
…er avoiding programmatic DOM changes while composing.
…osition mutations on composition end.
This comment was marked as outdated.
This comment was marked as outdated.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
… of mutations vs composition elements.
…the model to avoid reverting DOM changes and the re-applying them.
| document.on<ViewDocumentFocusEvent>( 'focus', () => this._handleFocus() ); | ||
| document.on<ViewDocumentBlurEvent>( 'blur', ( evt, data ) => this._handleBlur( data ) ); | ||
|
|
||
| document.on<ViewDocumentInputEvent>( 'beforeinput', () => { |
There was a problem hiding this comment.
Comments needed. Why we need to handle focus on every beforeinput? It was confusing initially that this actually SHOULD happen only once. Because it relies on the assumption that beforeinput is fired only for focused editors. Or not? If not, what happens then?
There was a problem hiding this comment.
Q: What happens when beforeinput is fired for a blurred editable?
There was a problem hiding this comment.
What happens when beforeinput is fired for a blurred editable is described in this issue: #14702.
| // @if CK_DEBUG_TYPING // } | ||
|
|
||
| insertAt( domElement as DomElement, i, expectedDomChildren[ i ] ); | ||
| i++; | ||
| } | ||
| // Update the existing text node data. Note that replace action is generated only for Android for now. |
There was a problem hiding this comment.
This comment was outdated. That's why we removed the 2nd sentence.
| return; | ||
| } | ||
|
|
||
| if ( env.isAndroid && this.isComposing && actualText.replace( /\u00A0/g, ' ' ) == expectedText.replace( /\u00A0/g, ' ' ) ) { |
| this._compositionQueue.flush( 'composition end' ); | ||
| } ); | ||
|
|
||
| // Trigger mutations check after the composition completes to fix all DOM changes that got ignored during composition. |
There was a problem hiding this comment.
Add some examples when this is necessary to happen. It's sort of a backup. Some changes are handled "at their selection" and some generalized to the model element's scope.
| // @if CK_DEBUG_TYPING // ); | ||
| // @if CK_DEBUG_TYPING // } | ||
|
|
||
| view.document.fire<ViewDocumentMutationsEvent>( 'mutations', { mutations: [] } ); |
There was a problem hiding this comment.
Explain that this will trigger the renderer to check all the nodes previously marked but ignored because the document was in the composition state (which blocks the renderer).
Also explain an example case when this is needed.
|
@niegowski What's the ETA on when this will be released, what version of CKEditor would it be in? |
|
We are starting preparation for the release that is scheduled for the next week. Most likely it will be 43.0.0. |
Suggested merge commit message (convention)
Fix (typing, engine): Predictive text should not get doubled while typing. Closes #16106.
Fix (engine): The reverse typing effect should not happen after the focus change. Closes #14702. Thanks, @urbanspr1nter!
Fix (engine, typing): Typing on Android should avoid modifying DOM while composing. Closes #13994. Closes #14707. Closes #13850. Closes #13693. Closes #14567. Closes: #11569.
Additional information
For example – encountered issues, assumptions you had to make, other affected tickets, etc.