Bug 23793 – WebKit needs separate VisibleSelection and Selection data storage

• Component: HTML Editing
• Status: RESOLVED CONFIGURATION CHANGED

[Eric Seidel (no email)]

WebKit changes the Selection on setting with addRange, FF does not

This is sorta a dupe of bug 6350 and bug 17697, certainly fixing this more general bug would fix both of those.

Basically Selection::validate() is flawed, at least from the DOM's perspective.  JS should be able to set the selection to anything and get it back w/o having it be mutated in the process.  The Editing code wants to be able to get at the "validated" selection.  We may just want to store things double, not sure yet.

[Eric Seidel (no email)]

This bug was prompted by http://code.google.com/p/chromium/issues/detail?id=2874, I'm about to attach a modified test case from chromium's 2874

[Eric Seidel (no email)]

Created attachment 27404 [details]
modified test case from chromium bug

[Eric Seidel (no email)]

I think the "validation" invariant which Selection currently tries to hold, is a wrong one.  In at least I don't think Selection should assume that both of it's endpoints are valid visible positions.  Maybe we need a separate VisibleSelection class which holds that invariant, similar to how we have VisiblePosition and Position split classes.

[Eric Seidel (no email)]

In the above proposal, Selection would just be an array of ranges.  VisibleSelection would probably still support only one Range (like it does today), slowly editing commands could be transitioned off of VisibleSelection and onto Selection?  Maybe adding multiple-range selection support in the process?

[Justin Garcia]

(In reply to comment #3)
> Maybe we need a separate VisibleSelection class which holds that invariant, 
> similar to how we have VisiblePosition and Position split classes.

Yea, that seems like the way to go.

[Eric Seidel (no email)]

Ok, so SelectionController will need a VisibleSelection member (the current m_selection), and a new Selection member.

What the user sees and interacts with is the VisibleSelection.  What JavaScript and (eventually) the editing commands interact with will be Selection (a Vector of Range objects).

VisibleSelection and Selection will both be copyable, datastorage types.  Editing, and any other section of the code which wants to keep around multiple copies can do so.  SelectionController maintain the official copy. :)

In order to handle keeping the stored Selection and VisibleSelection in sync, SelectionController will need to change to return a copy of a VisibleSelection via visibleSelection() instead of a reference like it does now.  There will be a new setVisibleSelection method which can take a VisibleSelection and knows to set m_selection from VisibleSelection::toSelection() or similar.

Likewise there will need to be a setSelection which takes a Selection object and knows to create a VisibleSelection from the Selection and store it in m_visibleSelection.

Initially all of our code will use VisibleSelection.  Over time, we will move more and more code to use Selection instead.

To add multi-range selection (which we'll eventually need for table editing), Selection will just store a Vector of Range objects.  VisibleSelection can hold whatever data it needs to represent the selection, but it will probably be a Vector of VisiblePosition pairs (or maybe VisibleRanges?).

Thoughts on this proposal are most welcome!

[Ahmad Saleem]

Attached test case also passes in Safari Technology Preview 164 after Live Range Selection API also matching Chrome Canary 112 and Firefox Nightly 112.

Marking this as "RESOLVED CONFIGURATION CHANGED". Thanks!