[webkit-dev] Rich Text Editing Questions, Refactoring of Position Classes

[Maciej Stachowiak]

On Apr 7, 2010, at 1:59 AM, Roland Steiner wrote:

> On Wed, Apr 7, 2010 at 4:36 PM, Maciej Stachowiak <mjs at apple.com>  
> wrote:
>
> It's not clear to me how "PositionIterator" is the same concept as  
> "EditingPosition". The latter implies that it would only ever  
> represent a position where you can edit. The former implies that it  
> produces a sequence of positions (perhaps retaining additional state  
> to be able to step forward/back efficiently). It also seems to me  
> that it is useful to have a concept of a Position that is primarily  
> optimized iteration.
>
> Perhaps "EditingPosition" is not the best of names. The basic idea  
> is that we shouldn't need 2 different classes Position and  
> PositionIterator and convert back-and-forth between them.

I'm not sure I understand that goal. It's pretty normal for an  
iterator type to be distinct from the type of thing it iterates over.  
vector<T>::iterator is not the same as T, even though it iterates over  
Ts. It is also not the same as vector<T> itself. I'm not familiar with  
the exact state of the editing code, but that's what I think of when I  
hear a suggestion to combine PositionIterator and Position.

I'm also not clear on where the name Editing comes from. What is it  
about this kind of position that's related to editing? Is it only  
allowed to represent positions where editing can happen? If so, then  
it can't be a full replacement for either Position or  
PositionIterator, since those are used to represent non-editable  
locations. If it can represent even non-editable locations, then I  
wonder how it differs from your proposed DOMPosition - would it be  
just the support for fast iteration?

>
> I'm also still not clear on the proposed relation between  
> EditingPosition and VisiblePosition. Does every EditingPosition have  
> an associated VisiblePosition? How about vice versa? Is the mapping  
> one-to-one?
>
> That is a good question - it depends how we want to treat stuff  
> like :before/:after content, list items, and such. A renderer-based  
> VisualPosition could technically select/iterate/identify such  
> content, but wouldn't necessarily have a corresponding Editing/ 
> DOMPosition. OTOH, an EditingPosition probably would always have an  
> associated VisiblePosition (At least off the top of my head I can't  
> think of a case where the user can edit stuff that isn't rendered).  
> An EditingPosition with reflection or somesuch could in theory have  
> more than one VisiblePosition, but I think that's getting awfully  
> fanciful.

Could a VisiblePosition have more than one associated EditingPosition?

>
> However, the immediate idea here is more one of code separation: to  
> put the code that operates on renderers into its separate class.

I think to correctly iterate over positions where editing can happen,  
you need to be aware of rendering. For one thing, you want to skip  
stuff that is not rendered. Are you imagining that EditingPosition  
would be purely DOM based? Would EditingPosition or VisiblePosition be  
the type of object that represents a place where you can put the caret?

I understand and sympathize with your attempt to clean up editing  
concepts, but I am not getting a sense of clean separation here.

>
>> On Wed, Apr 7, 2010 at 3:53 AM, Justin Garcia <justin.garcia at apple.com 
>> > wrote:
>>
>> For example:
>>
>> <div><img style="display:block"><img style="display:block"></div>
>>
>> [img1, 1] and [img2, 0] are different visually but would both be  
>> normalized to the same position under the above proposal.
>>
>> I think this is a great example and shows that normalizing [img, 0]  
>> to [parent-of-img, index-of-img] can probably only happen once  
>> styles and renderers are taken into account, which doesn't strictly  
>> contradict Darin's point though (?).
>
> I'm not sure how it implies that. Would you assign a real DOM  
> position differently between the two styles?
>
> I think the real difference here is in affinity, i.e. whether the  
> caret would be at the end of one line or the start of the next.  
> However this doesn't affect the DOM parent and offset.
>
>
> Yes, that's what I meant - the DOM Position itself should obviously  
> not be affected by styles/CSS. This however leads me to a  
> clarification question of my own:
>
> The [img1, 1] in Justin's example would be an invalid position, no?

Indeed; I believe he was trying to give an example of how such invalid  
positions might serve a meaningful purpose.

> Do you mean that (the internal representation of) [img/block, 0] and  
> [img/block, 1] should render the caret at different positions (i.e.,  
> the start/left side and end/right side of the block line,  
> respectively)?

That is what happens currently, but I don't think that was Justin's  
main point. I think his point was that [img1, 1] renders at the end of  
the first line, and [img2, 0] renders at the start of the first line,  
even though both should normalize to a true DOM position of [div, 1]  
(between the two images).

> Or did you mean the case of 2 images (i.e., without a real internal  
> visual position) on different lines?

That's exactly what Justin's example does; making the images  
display:block puts them on separate lines, without any intervening  
elements or text content. Right after the first is logically the same  
position as right before the second, but would be visually different  
in this case.

However, I think we should handle such issues through the separate  
concept of affinity, rather than trying to encode it through funny use  
of node/offset pairs. The same basic issue arises at a soft line break  
- the caret can be on the end of one line or the start of the second -  
but in that case, there is nothing you can do to get different (node,  
offset) pairs for the two visual positions.

Regards,
Maciej

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20100407/dcfebb59/attachment-0001.html>