ckeditor5
ckeditor5 copied to clipboard
[beforeInput] Consider updating text node values instead of replacing entire text nodes
It was proposed as a part of #12058 but the change may need to be wider (cover all browsers).
- First question to answer: Why is this needed on non-Android browsers?
- Assuming, the answer indicates that we should make that change for all browsers, extract that change from https://github.com/ckeditor/ckeditor5/pull/12446/files and propose a complete PR in this ticket.
- First question to answer: Why is this needed on non-Android browsers?
We figured out that we were triaging many tickets, also non-Android ones, on the Android PoC which includes this change applied to all browsers. By reverting it for non-Android browsers we might actually regress something compared to a state that we know.
This indicates that it may make more sense to keep this change.
The flow of typing a single letter (not IME, desktop):
-
beforeInput:insertText
native event- default prevented - no text will be inserted to DOM by the browser itself
- translated to
insertText
event - translated to
insertText
command call - text node inserted to model tree
- model changes downcasted to view ->
DowncastWriter.insert()
-
Element._insertChild()
is called- fires
change:children
event - handled as
Renderer#markToSync( 'children', ... )
- fires
-
DowncastWriter.mergeAttributes()
->mergeTextNodes()
-
- view changes triggered DOM rendering
- paragraph (parent of text) is updated since it was marked-to-sync
- the old text node is removed
- a new text node is inserted
In the above scenario, the whole text node is replaced.
The flow of typing a single letter (IME, desktop):
-
compositionStart
native event- renderer is disabled
-
beforeInput:insertCompositionText
native event- can't be default prevented - text will be inserted to DOM by the browser itself
-
MutationObserver
detected changes-
Renderer#markToSync( 'text', ... )
- Rendering is still disabled, nothing changes
-
-
compositionEnd
native event- renderer is enabled
- updates text node marked by
MutationObserver
(reverts changes made by the browser)
- updates text node marked by
- triggers
insertText
- translated to
insertText
command call - text node inserted to model tree
- model changes downcasted to view ->
DowncastWriter.insert()
-
Element._insertChild()
is called- fires
change:children
event - handled as
Renderer#markToSync( 'children', ... )
- fires
-
DowncastWriter.mergeAttributes()
->mergeTextNodes()
-
- translated to
- renderer is enabled
- view changes triggered DOM rendering
- paragraph (parent of text) is updated since it was marked-to-sync
- the old text node is removed
- a new text node is inserted
In the above scenario:
- Browser modifies DOM
- Editor reverts that changes (updates DOM node)
- Editor replaces the text node with a new one
Renderer-related problems:
- It's too complex. The implementation was never reviewed and cleaned up.
- We perform superfluous DOM changes in the above mentioned workflows.
- We have no control over which part of rendering are blocked (no way to tune this for IME and RTC-related scenarios).
- Disabling rendering leads today to problems with mapping – the editor starts crashing. This should be more robust and safer.
- And moar. Inline filler? Performance?
Decision for now: It does not make sense to dig deeper into this specific problem right now. The DOM<->View space requires a more thorough analysis and potentially a full redesign.