<div><div class="gmail_quote">On Fri, Feb 4, 2011 at 10:30 AM, Darin Adler <span dir="ltr"><<a href="mailto:darin@apple.com">darin@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">

On Feb 1, 2011, at 6:51 PM, Ryosuke Niwa wrote:<br>
<br>
>       • 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.<br>


<br>
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).<br>


<br>
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.<br></blockquote><div><br></div><div>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 <a href="https://bugs.webkit.org/show_bug.cgi?id=52099">bug 52099</a> is resolved or almost resolved which will take months if not years.</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
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?<br></blockquote><div><br></div>

<div>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).</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">


To me this just sounds like today’s Position minus all the high level functions that don’t belong as member functions.<br></blockquote><div><br></div><div>Yes, that's a way to put it.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">


>       • 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.<br>


<br>
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.<br>

</blockquote><div><br></div><div>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 <a href="https://bugs.webkit.org/show_bug.cgi?id=53649">bug 53649</a> 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.</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
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.<br></blockquote><div>

<br></div><div>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.</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
>       • 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.<br>


<div class="im">> 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.<br>


<br>
</div>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.<br>

</blockquote><div><br></div><div>What kind of suggestions do you have?  I'd love to hear.</div><div><br></div><div>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.</div>

<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Thanks for tackling this. I’m having trouble understanding your plans, so I’d like to hear more concrete details at some point.</blockquote><div><br></div><div>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.</div>

<div><br></div><div>- Ryosuke</div></div></div>