63040 2011-06-20 20:15:25 -0700 [meta] positionIsOffsetInAnchor should only accept text nodes 2017-07-18 08:29:58 -0700 1 1 1 Unclassified WebKit HTML Editing 528+ (Nightly build) Unspecified Unspecified NEW P2 Normal --- 63037 63100 63181 63384 52098 1 rniwa webkit-unassigned ap darin enrica eric leviw sullivan yosin oldest_to_newest 424213 0 rniwa 2011-06-20 20:15:25 -0700 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. 425418 1 rniwa 2011-06-22 11:33:56 -0700 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) 427409 2 rniwa 2011-06-25 12:53:19 -0700 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]; 1105332 3 yosin 2015-06-28 19:13:50 -0700 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? 1105333 4 leviw 2015-06-28 19:34:22 -0700 (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. 1105334 5 rniwa 2015-06-28 19:34:49 -0700 (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. 1105337 6 yosin 2015-06-28 20:50:18 -0700 >> 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? 1105338 7 rniwa 2015-06-28 21:52:56 -0700 (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) 1105346 8 yosin 2015-06-28 22:52:57 -0700 (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. 1105355 9 leviw 2015-06-28 23:46:23 -0700 (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.