Closed Bug 357445 Opened 18 years ago Closed 18 years ago

Clean up ranges

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: sicking, Assigned: sicking)

References

Details

Attachments

(2 files)

Our range code is a rats nest. Comming up is a first patch to clean up parts of it. Mostly it changes the internal members to use nsINode rather than nsIDOMNode. I left large parts of nsRange.cpp untouched, simply to keep down the patch size. Additionally some parts should simply be rewritten so there's no point in cleaning it up first.

Once this one's landed I plan to change the notifications so that ranges use normal nsIMutationObserver hooks rather than having to have special notifications just for ranges.
Attached patch Patch to fixSplinter Review
So mostly this change the internal members to nsINode, and converts some of the surrounding code to use nsINode too. I also introduced an nsIRange interface which makes it possible to reach the nsINode members directly. Eventually we should add more methods here for some deCOMtamination goodness.

I also rewrote nsRange::IsIncreasing to be nsContentUtils::ComparePoints
Attachment #242933 - Flags: superreview?(jst)
Attachment #242933 - Flags: review?(jst)
Comment on attachment 242933 [details] [diff] [review]
Patch to fix

- In nsRange::CloneRange(nsIDOMRange** aReturn):

+  nsRange* range = new nsRange();
+  NS_ENSURE_TRUE(range, NS_ERROR_OUT_OF_MEMORY);
...
+  range->DoSetRange(mStartParent, mStartOffset, mEndParent, mEndOffset);
+
+  NS_ADDREF(*aReturn = range);

That should be flipped around, don't call methods on XPCOM objects with a 0 reference count if you can help it.

Other than that it looks good to me. r+sr=jst
Attachment #242933 - Flags: superreview?(jst)
Attachment #242933 - Flags: superreview+
Attachment #242933 - Flags: review?(jst)
Attachment #242933 - Flags: review+
Checked in, thanks for the quick review!
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Depends on: 357509
Depends on: 357550
This bug regressed the nsRange::IsNodeIntersectsRange function. The two argument pairs to the last nsContentUtils::ComparePoints call are inverted. This was fixed in bug 358073.
Depends on: 358660
Attachment #244527 - Flags: superreview+
Attachment #244527 - Flags: review+
Depends on: 361229
Depends on: 379280
Depends on: 414076
Depends on: 454904
Component: DOM: Traversal-Range → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.