Summary: | WebKit needs separate VisibleSelection and Selection data storage | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> | ||||
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED CONFIGURATION CHANGED | ||||||
Severity: | Normal | CC: | ahmad.saleem792, ayg, emacemac7, hyatt, jparent, justin.garcia, kai, ojan | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Mac | ||||||
OS: | OS X 10.5 | ||||||
Bug Depends on: | 23852 | ||||||
Bug Blocks: | 6350, 17697 | ||||||
Attachments: |
|
Description
Eric Seidel (no email)
2009-02-06 11:39:32 PST
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 Created attachment 27404 [details]
modified test case from chromium bug
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. 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? (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. 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! 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! |