Bug 25500 - Remove Range::compareBoundaryPoints(Position, Position) per Darin's suggestion in bug 25056
Summary: Remove Range::compareBoundaryPoints(Position, Position) per Darin's suggestio...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Eric Seidel (no email)
URL:
Keywords:
Depends on:
Blocks: 25056
  Show dependency treegraph
 
Reported: 2009-04-30 16:58 PDT by Eric Seidel (no email)
Modified: 2009-06-02 14:31 PDT (History)
0 users

See Also:


Attachments
Remove Range::compareBoundaryPoints(Position, Position) per Darin's suggestion in bug 25056 (16.90 KB, patch)
2009-04-30 17:01 PDT, Eric Seidel (no email)
mjs: review+
Details | Formatted Diff | Diff
Add a compareBoundaryPoints which takes RangeBoundaryPoints (4.45 KB, patch)
2009-04-30 17:14 PDT, Eric Seidel (no email)
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2009-04-30 16:58:42 PDT
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.
Comment 1 Eric Seidel (no email) 2009-04-30 17:01:20 PDT
Created attachment 29930 [details]
Remove Range::compareBoundaryPoints(Position, Position) per Darin's suggestion in bug 25056

 11 files changed, 77 insertions(+), 34 deletions(-)
Comment 2 Eric Seidel (no email) 2009-04-30 17:14:14 PDT
Created attachment 29931 [details]
Add a compareBoundaryPoints which takes RangeBoundaryPoints

 3 files changed, 32 insertions(+), 11 deletions(-)
Comment 3 Maciej Stachowiak 2009-05-21 20:26:09 PDT
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.
Comment 4 Maciej Stachowiak 2009-05-22 01:06:54 PDT
Comment on attachment 29931 [details]
Add a compareBoundaryPoints which takes RangeBoundaryPoints

r=me
Comment 5 Maciej Stachowiak 2009-05-22 01:09:49 PDT
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());
+}
+
Comment 6 Eric Seidel (no email) 2009-06-02 14:31:27 PDT
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