[webkit-dev] Proposal to Reorganize Position Classes

[Roland Steiner]

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 <rniwa at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20110202/454aa8b0/attachment.html>