Just for context: Maciej and others had some feedback in the similar "Rich Text Editing Questions, Refactoring of Position Class" thread from a year ago (4/2/10). As an aside, I'm wondering if the general thoughts and reservations about supporting positions inside :before/:after-generated content and multiple ranges has changed in the meantime (?).
A few more comments (questions, rather) inline: On Wed, Feb 2, 2011 at 11:51 AM, Ryosuke Niwa <rn...@webkit.org> wrote: > Hi all, > * > * > *While I'm not intending to write a patch for the following proposal > anytime soon, I'd really like to get your feedback.* > * > * > As you might all know, Position classes in WebCore need refactoring and > cleanup. Part of it is due to legacy editing positions that do many > "magics", and we're currently in the process of removing them (see the bug > 52099 <http://webkit.org/b/52099>). However, even if we didn't have > legacy editing positions, we still have rather clattered code base - there > are many global functions in visible_units.cpp and VisiblePosition is used > all over in WebCore/rendering - and reorganization of Position-related > classes is needed (see the bug 52098 <http://webkit.org/b/52098>). > > I spent the last couple months to figure out what's the right solution and > realized that there are generally two use cases of VisiblePosition: > > - To differentiate upstream/downstream to express caret positions > - To iterate through editable regions to implement editing commands > > For the former use case, triggering layout or respecting editing boundary > is not needed at all, and the use of VisiblePosition seems an overkill. I > concluded that these two use cases should be addressed by two different > classes. > > So I propose to implement the following 3 classes in lieu of the existing > Position and VisiblePosition: > > - *DOMPosition* or *SimplePosition* - This class solely works on DOM, > and is agnostic of renderers and other parts of WebCore. It's meant to be > used in utility functions and classes or passed along between renderers and > WebCore/dom. It doesn't know anything about upstream or downstream. > - *RenderedPosition* - This positions is an enhanced version of the > current Position. It represents every possible visible position and or DOM > position and communicates vocabularies between WebCore/rendering and > WebCore/editing. It knows about line boundary and upstream/downstream but > it doesn't trigger a layout, doesn't canonicalize, and doesn't know > anything > about editing boundary. Its life-time is tied to layout cycle, and needs > to > be re-created every time DOM mutation or style change occurs. Many > functions in visible_units.cpp can belong to this class as member > functions. > Furthermore, PositionIterator could be merged into this class because > RenderedPosition can cache pointers and other invariants as needed since > RenderedPosition's lifetime is tied to layout cycle. It can also share > some > code with TextIterator as well. > > How does RenderedPosition know line boundaries and upstream/downstream if it doesn't trigger layout? > - *EditingPosition* or *VisiblePosition* - This class is almost > identical to the existing VisiblePosition. It knows everything DOMPosition > and RenderedPosition knows, and respects editing boundary. A significant > difference with VisiblePosition is that this class remembers the editable > root element to which it belongs. So when crossing editing boundary, it > can > figure out whether or not we're still inside the same root editable root or > not. It also knows how to canonicalize itself so editing commands can > canonicalize positions as needed although canonicalization could be > optional. I'm also not sure if this class should trigger a layout inside > its constructor like VisiblePosition does or not yet. > > "A significant difference with VisiblePosition..." - do you mean vs. RenderedPosition or vs. the current VisiblePosition? > > > The introduction of RenderedPosition is particularly useful in rendering > engine because it allows to express any caret/insertion point position with > a guarantee that it doesn't trigger a layout. > Not triggering layout is certainly a good point and something that should bring quite a bit of performance. However, I wonder some of this functionality shouldn't be factored out in a separate non-Position class (say "SelectionManager"). Cheers, - Roland
_______________________________________________ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev