[Webkit/Blink] New strategy for Filling Character handling to avoid changes in DOM

[Olek Nowodziński]

At the moment…

Basic concepts

1. FillingChar (FC) is a ZWS text node.
2. selection.selectRanges() creates FC in Webkits to put selection in tricky places.
3. FC is not welcome in data returned from the editor and Undo Manager snapshots.
4. There may be a single FC in the editor at the time. There could be many other ZWSs there as well, as a regular user content.

Lifespan of FC

1. FC is removed from DOM as soon as possible, when no longer necessary.
2. FC is removed from DOM on #selectionChange.
   1. FC always survives first #selectionChange.
   2. It is removed on the second event because the first one is always fired by selectRanges() (so it isn't nuked right after created).
3. FC is removed from DOM before (on) certain #keydown events (<kbd>ENTER</kbd>, <kbd>LEFT/RIGHT ARROW</kbd>, <kbd>BACKSPACE</kbd>, etc.) to not to disrupt the commands.

FC and data processing

1. On #beforeGetData and #beforeUndoImage, if the selection is in the FC text node, a "native bookmark" is created and saved because changes in DOM done in the next point will break the selection.
2. On #beforeGetData and #beforeUndoImage, the FC's content (if present) is saved and then replaced in DOM with
   1. nothing (FC becomes an empty text node)
   2.  , if the white–space is right after it
3. Data is retrieved from editor DOM (editor.getData()), or an undo snapshot is created (CKEDITOR.instances.editor.undoManager.snapshots.map( s => s.contents )). Both data and snapshots are FC–free.
4. On #getData and #afterUndoImage FC's text in DOM is reverted back to the state saved on #beforeGetData or #beforeUndoImage.
5. On #getData and #afterUndoImage the native selection, devastated on #beforeGetData or #beforeUndoImage by changes to FC text content, is finally restored.

Problems

1. Because FC is removed from DOM on various data operations:
   1. A native #selectionchange is fired by document. It creates an infinite loop (#13593) if such listener is to be used in the editor.
   2. IME (Composition) gets broken when Undo Manager snapshot is taken. This is because FC is removed from DOM and the native selection changes. Try setInterval( function() { var i = new CKEDITOR.plugins.undo.Image( CKEDITOR.instances.editor ); console.log( i.contents ) }, 2000 ); and start composition.

Related tickets

• #12727
• #13377
• #13284
• #13389
• #13307
• #13801

---

A new approach

Basic concepts

1. When retrieving data or creating Undo Manager snapshots, remove FC from HTML strings, but not from DOM.
   1. How to tell which ZWSs are user content in HTML string and which belong to FC then?!
      1. The trick is to use a number of ZWS for FCSeq. Like 7, or 8. Or 42.
      2. It makes finding FCSeq in the contents much easier. If there are always 7 ZWS for FCSeq, any group of 7 ZWS can be removed and, if there are like 10 ZWS, the algorithm will know that 3 ZWS belong to the user content (still 7 to left to purge).

getData

1. Do noting on #beforeGetData.
2. Remove a pre–defined number of ZWS from HTML string (don't touch the DOM).

getSnapshot

1. Remove a pre–defined number of ZWS from HTML string (don't touch the DOM).

SelectionChange, keyboard events

1. FCSeq is removed from DOM on selectionChange and on keyboard events, just like in the "old approach".

Problem with bookmarks

1. createBookmark2( true ) is used to create undo snapshots. It normalizes snapshot content so adjacent text nodes are considered a single text node.
2. Because FCSeq is removed from HTML string while a snapshot is being taken (previously removed from DOM before the operation), addresses and offsets in the bookmark may be incorrect.
   1. The bookmarking algorithm must be smart to anticipate that the FCSeq visible in DOM will be gone in the snapshot data.
   2. Bookmark addresses and offsets must be corrected so when the bookmark is restored, the selection is correctly anchored.
   3. It is fun!

[Olek Nowodziński]

Pushed ​branch:t/13816 (previously "zws") with the new approach.

[Piotrek Koszuliński]

cc

[Piotrek Koszuliński]

I rebased and reviewed the branch. It's great – AFAICS it solves all the linked issues (although I'll leave final checks for you). The only issue I've found was the createBookmark2 method which had problems with couple of cases (related to FCS and other cases that weren't previously handled). I decided to jump into that and found out that big part of the algorithm must be rewritten due to cases that could not be handled otherwise. It seems that I succeeded although I'm pretty sure there's still number of cases which are not covered. This is nightmarishly complex algorithm, so... they will appear with time :).

Anyway, all is OK, but since I've done quite a lot of changes, I'm putting this branch on a final review.

[Olek Nowodziński]

@Reinmar: I'm cool with the changes you did to my branch. I checked related tickets and posted the summary of what's going on below. There's a number of tickets I'm unable to reproduce so either they were caused by some regression in the web browser which is now long gone or fixed by another ticket, either accidentally or on purpose. Please, confirm that they're no longer reproducible and then I'm OK to merge ​branch:t/13816 into master and finally switch to #13593, which was the reason we started this ticket.

---

Fixed in this ticket

• ​https://dev.ckeditor.com/ticket/12727
• ​https://dev.ckeditor.com/ticket/13377
• ​https://dev.ckeditor.com/ticket/13513

Green light for development of

• ​https://dev.ckeditor.com/ticket/13593

Unable to reproduce on master

• ​https://dev.ckeditor.com/ticket/13389
• ​https://dev.ckeditor.com/ticket/13307

[Marek Lewandowski]

It also fixes #13513, updated list in the comment above.

[Marek Lewandowski]

This is a major code refactor and it might be hard to catch all the things that might go wrong with it. We'll put it to the next milestone and merge it at the beginning - this will give us more time to see potential problems. Along with this issue we'll move all the reltaed tickets.