Bug 63040 – [meta] positionIsOffsetInAnchor should only accept text nodes

• Status: NEW
• Component: HTML Editing

[Ryosuke Niwa]

During the contributor's meeting, we have concluded that we should not have positions with offsets in non-text nodes for performance reasons and for comparison.  We should refactor the code to enforce this.

[Ryosuke Niwa]

The current plan is follows:
1. Fix the bug 63100 - Add BeforeChildren/AfterChildren anchor types.
2. Add Position(PassRefPtr<Text>, unsigned offset)
3. Remove instantes of Position(..., PositionIsOffsetInAnchor)

[Ryosuke Niwa]

There are few files in WebKit that instantiates Position directly:
Source/WebKit/chromium/src/WebViewImpl.cpp:1302:                                      Position::PositionIsOffsetInAnchor);
Source/WebKit/mac/WebView/WebFrame.mm:743:    Position start = Position(startContainer, [proposedRange startOffset], Position::PositionIsOffsetInAnchor);
Source/WebKit/mac/WebView/WebFrame.mm:744:    Position end = Position(endContainer, [proposedRange endOffset], Position::PositionIsOffsetInAnchor);
Source/WebKit/mac/WebView/WebFrame.mm:1107:    Position startPos(startContainer, [rangeToReplace startOffset], Position::PositionIsOffsetInAnchor);
Source/WebKit/mac/WebView/WebFrame.mm:1108:    Position endPos(endContainer, [rangeToReplace endOffset], Position::PositionIsOffsetInAnchor);
Source/WebKit/mac/WebView/WebTextCompletionController.mm:208:            NSRect wordRect = [frame _caretRectAtPosition:Position(core([wholeWord startContainer]), [wholeWord startOffset], Position::PositionIsOffsetInAnchor) affinity:NSSelectionAffinityDownstream];

It seems there are no alternative way to represent child node other than PositionIsOffsetInAnchor.

What do you mind to represent child node w/o PositionIsOffsetInAnchor?

[Levi Weintraub]

(In reply to comment #3)
> It seems there are no alternative way to represent child node other than
> PositionIsOffsetInAnchor.
> 
> What do you mind to represent child node w/o PositionIsOffsetInAnchor?

For non-text nodes, we want to use the other types: 

PositionIsBeforeAnchor,
PositionIsAfterAnchor,
PositionIsBeforeChildren,
PositionIsAfterChildren,

This avoids having to walk the child list.

[Ryosuke Niwa]

(In reply to comment #3)
> It seems there are no alternative way to represent child node other than
> PositionIsOffsetInAnchor.
> 
> What do you mind to represent child node w/o PositionIsOffsetInAnchor?

Performance.

>> What do you mind to represent child node w/o PositionIsOffsetInAnchor?
> Performance.

Oops, my question is unclear.
How do we represent a position at child node w/o PositionIsOffsetInAnchor?
Use LegacyEditingPosition?

[Ryosuke Niwa]

(In reply to comment #6)
> >> What do you mind to represent child node w/o PositionIsOffsetInAnchor?
> > Performance.
> 
> Oops, my question is unclear.
> How do we represent a position at child node w/o PositionIsOffsetInAnchor?
> Use LegacyEditingPosition?

Use BeforeChildren, AfterChildren, BeforeAnchor, or AfterAnchor (for the latter two, you need to change the anchor node since we're no longer anchoring at the parent)

(In reply to comment #7)
> (In reply to comment #6)
> > >> What do you mind to represent child node w/o PositionIsOffsetInAnchor?
> > > Performance.
> > 
> > Oops, my question is unclear.
> > How do we represent a position at child node w/o PositionIsOffsetInAnchor?
> > Use LegacyEditingPosition?
> 
> Use BeforeChildren, AfterChildren, BeforeAnchor, or AfterAnchor (for the
> latter two, you need to change the anchor node since we're no longer
> anchoring at the parent)

Thanks for explanation. So, you want to avoid computing node index. But, why not use RangeBoundaryPoint?
It seems AnchorType introduces complexity in Position class.

[Levi Weintraub]

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > >> What do you mind to represent child node w/o PositionIsOffsetInAnchor?
> > > > Performance.
> > > 
> > > Oops, my question is unclear.
> > > How do we represent a position at child node w/o PositionIsOffsetInAnchor?
> > > Use LegacyEditingPosition?
> > 
> > Use BeforeChildren, AfterChildren, BeforeAnchor, or AfterAnchor (for the
> > latter two, you need to change the anchor node since we're no longer
> > anchoring at the parent)
> 
> Thanks for explanation. So, you want to avoid computing node index. But, why
> not use RangeBoundaryPoint?

It's not only about *computing* the node index, it's about finding the child nodes when you just have a pointer to the parent node and index. If you have a position between two non-text nodes in a node list with 1000 siblings, just having [parent, 500] will be very expensive...

> It seems AnchorType introduces complexity in Position class.