Bug 25500 – Remove Range::compareBoundaryPoints(Position, Position) per Darin's suggestion in bug 25056

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Eric Seidel (no email)]

Remove Range::compareBoundaryPoints(Position, Position) per Darin's suggestion in bug 25056

Darin indicated the Range should deal only with primitive DOM node/offset pairs, and that Position (which is a more robust editing construct) should have its own comparison functions and that Range.h should not mention Position at all.

Turns out that Position already has a comparePositions() function (which knows how to additionally handled positions in shadow trees).  So I've just changed all callers of compareBoundaryPoints(Position, Position) to use the existing comparePositions() function.  I've also added a comparePositions which takes VisiblePositions for convenience.

[Eric Seidel (no email)]

Created attachment 29930 [details]
Remove Range::compareBoundaryPoints(Position, Position) per Darin's suggestion in bug 25056

 11 files changed, 77 insertions(+), 34 deletions(-)

[Eric Seidel (no email)]

Created attachment 29931 [details]
Add a compareBoundaryPoints which takes RangeBoundaryPoints

 3 files changed, 32 insertions(+), 11 deletions(-)

[Maciej Stachowiak]

Comment on attachment 29930 [details]
Remove Range::compareBoundaryPoints(Position, Position) per Darin's suggestion in bug 25056

Is the VisiblePosition version of comparePositions ever used? I couldn't tell from the diff. If not, I suggest removing it.

[Maciej Stachowiak]

Comment on attachment 29931 [details]
Add a compareBoundaryPoints which takes RangeBoundaryPoints

r=me

[Maciej Stachowiak]

Comment on attachment 29930 [details]
Remove Range::compareBoundaryPoints(Position, Position) per Darin's suggestion in bug 25056

Mostly looks ok. But please remove this function if it's not actually used. Would rather not add dead code.

+int comparePositions(const VisiblePosition& a, const VisiblePosition& b)
+{
+    return comparePositions(a.deepEquivalent(), b.deepEquivalent());
+}
+

[Eric Seidel (no email)]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/dom/Range.cpp
	M	WebCore/dom/Range.h
	M	WebCore/editing/ApplyStyleCommand.cpp
	M	WebCore/editing/CompositeEditCommand.cpp
	M	WebCore/editing/DeleteSelectionCommand.cpp
	M	WebCore/editing/SelectionController.cpp
	M	WebCore/editing/VisibleSelection.cpp
	M	WebCore/editing/htmlediting.cpp
	M	WebCore/editing/htmlediting.h
	M	WebCore/page/EventHandler.cpp
Committed r44367
	M	WebCore/ChangeLog
	M	WebCore/dom/Range.cpp
	M	WebCore/dom/Range.h
Committed r44368