[webkit-dev] Proposal to Reorganize Position Classes

[Ryosuke Niwa]

On Fri, Feb 4, 2011 at 10:30 AM, Darin Adler <darin at apple.com> wrote:

> On Feb 1, 2011, at 6:51 PM, Ryosuke Niwa wrote:
>
> >       • 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.
>
> I’ve been hoping to turn the Position class into this for some time. I like
> to keep the “physics” (nodes, offsets, that sort of thing) separate from the
> “chemistry” (editing smarts, things that rely on rendering, style, and
> layout).
>
> I don’t like the name “SimplePosition”. Maybe a good first step would be to
> rename the old class to DeprecatedPosition and this new class can be named
> Position.
>

That'll be an option.  Or we can keep Position as is and move some of
fancier methods to other classes like RenderedPosition or
Visible/EditingPosition.  But all of that needs to wait until the bug
52099<https://bugs.webkit.org/show_bug.cgi?id=52099>is resolved or
almost resolved which will take months if not years.

Would this store a container node and offset using range-style technology,
> or would it have the features that the current Position class has that let
> it express positions before/after a node?
>

That'll be a debating point.  Right now, I'm intending to keep all 3 anchor
types to do the conversions from RenderedPosition/EditingPosition to
Position in O(1).

To me this just sounds like today’s Position minus all the high level
> functions that don’t belong as member functions.
>

Yes, that's a way to put it.

>       • 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.
>
> I don’t understand what state this would store besides the “DOMPosition”
> and thus I am unclear on why it’s a class rather than a set of functions. I
> do agree that this is an important category of functions that needs to
> exist. I can’t understand the proposal until you say more about what state
> this would store.
>

RenderedPosition is aware of upstream and downstream unlike Position and
therefore has more expressive power than a pure DOM position.  However, it's
unaware of editing boundary so it doesn't call isContentEditable crazy like
VisiblePosition (see the bug
53649<https://bugs.webkit.org/show_bug.cgi?id=53649> for
its performance impact).  I think the entire reason we have VisiblePosition
instantiated in rendering code is to differentiate the end of a line and the
start of the following line, and almost never to care about editing
boundary.  In other words, RenderedPosition will be a safe efficient
alternative to VisiblePosition in rendering code.

Your suggestion that we make the functions in visible_units.cpp into member
> functions doesn’t sound like an improvement to me. I would prefer a design
> that keeps the number of member functions small.
>

Maybe.  But many functions in visible_units.cpp seem to work exclusively
with the existing VisiblePosition and yet some of them delivery ignore
editing boundary or have bugs with mixed editability.  So I don't see why
those can't be methods of RenderedPosition / VisiblePosition.  One benefit
of making them member functions is that it'll pollute global namespace
less. But then keeping interface small is also a good idea so we can
probably talk about this later when we're actually implementing
RenderedPosition.

>       • 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.
> > 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.
>
> Sounds OK to add a root editable element to VisiblePosition. Also seems
> fine to rename it. But I think there is more room for improvement in
> VisiblePosition than this, so I suspect this proposal does not go far
> enough.
>

What kind of suggestions do you have?  I'd love to hear.

One thing I'd really like to make happen is to make canonicalization
optional.  This would help resolving a number of bugs that are caused by the
forced canonicalization.  I'd also like to make SelectionController store
real Range so that when script manually sets selection, it can get back the
same range back.  But I don't think they'll necessarily dictate position
classes' hierarchy.

Thanks for tackling this. I’m having trouble understanding your plans, so
> I’d like to hear more concrete details at some point.


I haven't made up concrete interface yet.  This was just to give you
heads-up and some big picture.  My colleagues and I are researching about
what each existing class and function does and how it interacts with the
rest of editing code.  Once we have that data (probably share date with
you), we should be able to give you more concrete design / interface.

- Ryosuke
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20110204/5ca24395/attachment.html>