Comment on attachment 556304 [details] [diff] [review] Part 1: Merge nsIDOMRange and nsIDOMNSRange --- a/accessible/src/html/nsHyperTextAccessible.cpp +++ b/accessible/src/html/nsHyperTextAccessible.cpp @@ -2357,21 +2356,20 @@ nsHyperTextAccessible::GetSpellTextAttri for (PRInt32 index = 0; index < rangeCount; index++) { nsCOMPtr<nsIDOMRange> range = ranges[index]; - nsCOMPtr<nsIDOMNSRange> nsrange(do_QueryInterface(range)); - NS_ENSURE_STATE(nsrange); + NS_ENSURE_STATE(range); You can probably assert that. Fine either way. --- a/content/base/src/nsRange.cpp +++ b/content/base/src/nsRange.cpp /******************************************************** - * Utilities for comparing points: API from nsIDOMNSRange + * Utilities for comparing points: API from nsIDOMRange ********************************************************/ Please make the line lengths match again. --- a/dom/interfaces/range/Makefile.in +++ b/dom/interfaces/range/Makefile.in -XPIDLSRCS = \ - nsIDOMNSRange.idl \ +XPIDLSRCS = \ I'd prefer just one space between = and \. --- a/dom/interfaces/range/nsIDOMRange.idl +++ b/dom/interfaces/range/nsIDOMRange.idl + + nsIDOMDocumentFragment createContextualFragment(in DOMString fragment); + + // This returns true if parent+offset equals either + // of the boundary points or is between them. + boolean isPointInRange(in nsIDOMNode parent, + in long offset); + + // comparePoint returns + // -1 if point is before the start boundary point, + // 0 if point is either of the boundary points or between them, + // 1 if point is after the end boundary point. + // Sort of a strcmp for ranges. + short comparePoint(in nsIDOMNode parent, in long offset); + + nsIDOMClientRectList getClientRects(); + nsIDOMClientRect getBoundingClientRect(); Care to add spec links while you're here? <http://dev.w3.org/csswg/cssom-view/#extensions-to-the-range-interface> for these two, and <http://html5.org/specs/dom-parsing.html#extensions-to-the-range-interface> for createContextualFragment. --- a/editor/libeditor/html/nsHTMLEditor.cpp +++ b/editor/libeditor/html/nsHTMLEditor.cpp @@ -1733,20 +1732,18 @@ nsHTMLEditor::ReplaceHeadContentsWithHTM - res = nsrange->CreateContextualFragment(inputString, + res = range->CreateContextualFragment(inputString, getter_AddRefs(docfrag)); Fix indentation. --- a/editor/txtsvc/src/nsFilteredContentIterator.cpp +++ b/editor/txtsvc/src/nsFilteredContentIterator.cpp @@ -273,17 +273,17 @@ ContentIsInTraversalRange(nsIContent *aC rv = nsTextServicesDocument::ComparePoints(aEndNode, aEndOffset, parentNode, indx, &endRes); NS_ENSURE_SUCCESS(rv, PR_FALSE); return (startRes <= 0) && (endRes >= 0); } static PRBool -ContentIsInTraversalRange(nsIDOMNSRange *aRange, nsIDOMNode* aNextNode, PRBool aIsPreMode) +ContentIsInTraversalRange(nsIDOMRange *aRange, nsIDOMNode* aNextNode, PRBool aIsPreMode) { nsCOMPtr<nsIContent> content(do_QueryInterface(aNextNode)); nsCOMPtr<nsIDOMRange> range(do_QueryInterface(aRange)); I don't think you need to do this anymore. --- a/extensions/spellcheck/src/mozInlineSpellChecker.cpp +++ b/extensions/spellcheck/src/mozInlineSpellChecker.cpp @@ -163,20 +162,18 @@ mozInlineSpellStatus::InitForEditorChang // ...we need to put the start and end in the correct order - nsCOMPtr<nsIDOMNSRange> nsrange = do_QueryInterface(mAnchorRange, &rv); - NS_ENSURE_SUCCESS(rv, rv); PRInt16 cmpResult; - rv = nsrange->ComparePoint(aPreviousNode, aPreviousOffset, &cmpResult); + rv = mAnchorRange->ComparePoint(aPreviousNode, aPreviousOffset, &cmpResult); Please check if mAnchorRange can be null here. if (aStartNode && aEndNode) { - nsrange = do_QueryInterface(mRange, &rv); - NS_ENSURE_SUCCESS(rv, rv); - - rv = nsrange->ComparePoint(aStartNode, aStartOffset, &cmpResult); + rv = mRange->ComparePoint(aStartNode, aStartOffset, &cmpResult); And mRange here @@ -379,31 +373,28 @@ mozInlineSpellStatus::FinishNavigationEv - nsCOMPtr<nsIDOMNSRange> oldWordNS = do_QueryInterface(oldWord, &rv); - NS_ENSURE_SUCCESS(rv, rv); And oldWord here @@ -1337,18 +1321,18 @@ nsresult mozInlineSpellChecker::DoSpellC + if (aStatus->mCreatedRange) + aStatus->mCreatedRange->IsPointInRange(beginNode, beginOffset, &inCreatedRange); - if (noCheckRange) { + if (aStatus->mNoCheckRange) { PRBool inExclusion = PR_FALSE; - noCheckRange->IsPointInRange(beginNode, beginOffset, &inExclusion); + aStatus->mNoCheckRange->IsPointInRange(beginNode, beginOffset, &inExclusion); Wrap under 80 columns please. Looks good to me, but I'd like a peer to review it as well.

Comment on attachment 556305 [details] [diff] [review] Part 2: Remove nsIRange --- a/accessible/src/html/nsHyperTextAccessible.cpp +++ b/accessible/src/html/nsHyperTextAccessible.cpp NS_IMETHODIMP nsHyperTextAccessible::GetSelectionCount(PRInt32 *aSelectionCount) { nsCOMPtr<nsISelection> domSel; - nsCOMArray<nsIDOMRange> ranges; + nsTArray<nsRange*> ranges; nsresult rv = GetSelections(nsISelectionController::SELECTION_NORMAL, nsnull, nsnull, &ranges); NS_ENSURE_SUCCESS(rv, rv); - *aSelectionCount = ranges.Count(); + *aSelectionCount = ranges.Length(); Could you add a PRInt32 cast here, and file a followup to return an unsigned long? NS_IMETHODIMP nsHyperTextAccessible::GetSelectionBounds(PRInt32 aSelectionNum, PRInt32 *aStartOffset, PRInt32 *aEndOffset) { - PRInt32 rangeCount = ranges.Count(); + PRInt32 rangeCount = ranges.Length(); PRUint32 - nsCOMPtr<nsIDOMRange> range = ranges[aSelectionNum]; + nsRange* range = ranges[aSelectionNum]; Please check if you need to hold a ref here. nsHyperTextAccessible::GetSpellTextAttribute(nsIDOMNode *aNode, PRInt32 aNodeOffset, PRInt32 *aHTStartOffset, PRInt32 *aHTEndOffset, nsIPersistentProperties *aAttributes) { + for (PRUint32 index = 0; index < rangeCount; index++) { + nsRange* range = ranges[index]; And here --- a/content/base/src/nsContentIterator.cpp +++ b/content/base/src/nsContentIterator.cpp nsresult -nsContentIterator::Init(nsIDOMRange* aRange) +nsContentIterator::Init(nsIDOMRange* aDOMRange) { - nsCOMPtr<nsIRange> range = do_QueryInterface(aRange); - return Init(range); - -} - -nsresult -nsContentIterator::Init(nsIRange* aRange) -{ - NS_ENSURE_ARG_POINTER(aRange); + NS_ENSURE_ARG_POINTER(aDOMRange); + nsRange* aRange = static_cast<nsRange*>(aDOMRange); s/aRange/range/, and you need to add builtinclass to nsIDOMRange to make this safe. Also, I'd slightly prefer a static nsRange* nsRange::AsRange(nsIDOMRange* aDOMRange) { return static_cast<nsRange*>(aDOMRange); } helper to avoid static_casting all over the place. --- a/content/base/src/nsDocumentEncoder.cpp +++ b/content/base/src/nsDocumentEncoder.cpp @@ -1066,17 +1066,17 @@ nsDocumentEncoder::EncodeToString(nsAStr if (content && content->Tag() == nsGkAtoms::tr) { Want to file a bug for this not checking the namespace? --- a/content/base/src/nsRange.cpp +++ b/content/base/src/nsRange.cpp +nsRange::CompareNodeToRange(nsINode* aNode, nsIDOMRange* aDOMRange, PRBool *outNodeBefore, PRBool *outNodeAfter) + nsRange* aRange = static_cast<nsRange*>(aDOMRange); s/aRange/range/ + Trailing whitespace. +nsresult +nsRange::CloneRange(nsRange** aReturn) const { - *aReturn = range.forget().get(); + range.forget(aReturn); Thanks. +NS_IMETHODIMP +nsRange::CloneRange(nsIDOMRange** aReturn) { + nsRange* range = nsnull; + nsresult rv = CloneRange(&range); + *aReturn = range; Please do use a smart pointer here. --- a/content/base/src/nsRange.h +++ b/content/base/src/nsRange.h -class nsRange : public nsIRange, +class nsRange : public nsIDOMRange, public nsStubMutationObserver { public: nsRange() - { - } + : mRoot(nsnull), + mStartOffset(0), + mEndOffset(0), + mIsPositioned(PR_FALSE), + mIsDetached(PR_FALSE), + mMaySpanAnonymousSubtrees(PR_FALSE) , should go at the start of the line. NS_DECL_NSIMUTATIONOBSERVER_PARENTCHAINCHANGED - + Trailing whitespace. --- a/editor/libeditor/html/nsHTMLEditRules.cpp +++ b/editor/libeditor/html/nsHTMLEditRules.cpp + // clone aRange. + nsCOMPtr<nsIDOMRange> domRange; + res = aRange->CloneRange(getter_AddRefs(domRange)); + //mDocChangeRange = static_cast<nsRange*>(domRange.get()); ?? --- a/embedding/components/find/src/nsFind.cpp +++ b/embedding/components/find/src/nsFind.cpp @@ -1315,19 +1310,16 @@ nsFind::Find(const PRUnichar *aPatText, ResetAll(); return NS_OK; } /* static */ already_AddRefed<nsIDOMRange> nsFind::CreateRange() { - nsCOMPtr<nsIRange> range = do_CreateInstance(kRangeCID); - if (!range) { - return nsnull; - } + nsRange* range = new nsRange(); range->SetMaySpanAnonymousSubtrees(PR_TRUE); nsIDOMRange* result; - CallQueryInterface(range.get(), &result); + CallQueryInterface(range, &result); return result; nsRefPtr<nsRange> range = new nsRange(); range->SetMaySpanAnonymousSubtrees(PR_TRUE); return range.forget(); --- a/layout/base/nsPresShell.cpp +++ b/layout/base/nsPresShell.cpp PresShell::CreateRangePaintInfo(nsIDOMRange* aRange, nsRect& aSurfaceRect, PRBool aForPrimarySelection) { NS_TIME_FUNCTION_WITH_DOCURL; RangePaintInfo* info = nsnull; - nsCOMPtr<nsIRange> range = do_QueryInterface(aRange); - if (!range) - return nsnull; + nsRange* range = static_cast<nsRange*>(aRange); I think you still need the null check. --- a/layout/generic/nsSelection.cpp +++ b/layout/generic/nsSelection.cpp +nsresult +nsTypedSelection::GetTableSelectionType(nsIDOMRange* aDOMRange, PRInt32* aTableSelectionType) { + nsRange* aRange = static_cast<nsRange*>(aDOMRange); s/aRange/range/ nsTypedSelection::GetRangesForInterval(nsIDOMNode* aBeginNode, PRInt32 aBeginOffset, nsIDOMNode* aEndNode, PRInt32 aEndOffset, PRBool aAllowAdjacent, PRUint32 *aResultCount, nsIDOMRange ***aResults) - if (results.Count() == 0) + Trailing whitespace + *aResultCount = results.Length(); + if (*aResultCount == 0) return NS_OK; {} +nsTypedSelection::AddRange(nsIDOMRange* aDOMRange) +{ + if (!aDOMRange) return NS_ERROR_NULL_POINTER; {} +nsTypedSelection::RemoveRange(nsIDOMRange* aDOMRange) +{ + if (!aDOMRange) return NS_ERROR_INVALID_ARG; + nsRefPtr<nsRange> aRange = static_cast<nsRange*>(aDOMRange); As usual + And here nsTypedSelection::CollapseToEnd() { // Get the last range - nsIRange* lastRange = mRanges[cnt-1].mRange; + nsRange* lastRange = mRanges[cnt-1].mRange; 'cnt - 1' @@ -5129,18 +5074,18 @@ nsTypedSelection::Extend(nsINode* aParen - nsCOMPtr<nsIRange> difRange = new nsRange(); - nsCOMPtr<nsIRange> range; + nsRefPtr<nsRange> difRange = new nsRange(); + nsRefPtr<nsRange> range; Want to declare these closer to their use? Looks good to me, except the thing in nsHTMLEditRules.cpp.

Comment on attachment 556305 [details] [diff] [review] Part 2: Remove nsIRange >--- a/content/base/public/nsISelectionPrivate.idl >+++ b/content/base/public/nsISelectionPrivate.idl >@@ -152,19 +153,19 @@ interface nsISelectionPrivate : nsISelec > */ > void GetRangesForInterval( > in nsIDOMNode beginNode, in PRInt32 beginOffset, > in nsIDOMNode endNode, in PRInt32 endOffset, > in PRBool allowAdjacent, > out PRUint32 resultCount, > [retval, array, size_is(resultCount)] out nsIDOMRange results); > >- [noscript] void GetRangesForIntervalCOMArray( >- in nsIDOMNode beginNode, in PRInt32 beginOffset, >- in nsIDOMNode endNode, in PRInt32 endOffset, >+ [noscript] void GetRangesForIntervalArray( >+ in nsINode beginNode, in PRInt32 beginOffset, >+ in nsINode endNode, in PRInt32 endOffset, > in PRBool allowAdjacent, > in RangeArray results); And please do s/PRBool/boolean/ while you're here.

Nvm, mwu already did that.

Comment on attachment 556478 [details] [diff] [review] Part 2: Remove nsIRange Oops, Part 2 was missing some pieces.

I'm going to add builtinclass to nsIDOMRange for the final version.

Comment on attachment 556729 [details] [diff] [review] Part 2: Remove nsIRange I won't have time to review this anytime soon.

Comment on attachment 556730 [details] [diff] [review] Part 3: Remove ns(I)RangeUtils I won't have time to look at this

Bent?

Oh, weird. I know nothing about nsRange... Olli, can you review the changes here?

I could. Someone should update the patches. They certainly don't apply cleanly to trunk.

And merged better: https://tbpl.mozilla.org/?tree=Try&rev=c78bbae01a21

Bitrot from bz in bug 684638...

Comment on attachment 570580 [details] [diff] [review] Part 3: Remove ns(I)RangeUtils Do we know if some binary addons uses this interface?

unbitrot

unbitrot

Comment on attachment 584665 [details] [diff] [review] Part 1: Merge nsIDOMRange and nsIDOMNSRange I wouldn't mind making nsIDOMRange a builtinclass if possible.

(In reply to Olli Pettay [:smaug] from comment #28) > I wouldn't mind making nsIDOMRange a builtinclass if possible. That's done in part 2: https://bugzilla.mozilla.org/attachment.cgi?id=584666&action=diff#a/dom/interfaces/range/nsIDOMRange.idl_sec1 Do we need to bump the uuid for that?

No.

The only thing I'm worried here is that are we removing some useful-to-binary-addons features

Part 1 may lose null checks in nsHTMLEditor::ReplaceHeadContentsWithHTM nsHTMLEditor::RebuildDocumentFromSource ContentIsInTraversalRange mozInlineSpellStatus::InitForEditorChang mozInlineSpellStatus::FinishNavigationEvent

Comment on attachment 584666 [details] [diff] [review] Part 2: Remove nsIRange > nsEditor::CreateRange(nsIDOMNode *aStartParent, PRInt32 aStartOffset, > nsIDOMNode *aEndParent, PRInt32 aEndOffset, > nsIDOMRange **aRange) > { >- nsresult result; >- result = CallCreateInstance("@mozilla.org/content/range;1", aRange); >- NS_ENSURE_SUCCESS(result, result); >- >- NS_ENSURE_TRUE(*aRange, NS_ERROR_NULL_POINTER); >- >- result = (*aRange)->SetStart(aStartParent, aStartOffset); >+ *aRange = new nsRange(); >+ NS_ADDREF((*aRange)); NS_ADDREF(*aRange = new Range()); > nsTextServicesDocument::CreateRange(nsIDOMNode *aStartParent, PRInt32 aStartOffset, > nsIDOMNode *aEndParent, PRInt32 aEndOffset, > nsIDOMRange **aRange) > { >- nsresult result; >- >- result = CallCreateInstance("@mozilla.org/content/range;1", aRange); >- NS_ENSURE_SUCCESS(result, result); >- >- NS_ENSURE_TRUE(*aRange, NS_ERROR_NULL_POINTER); >- >- result = (*aRange)->SetStart(aStartParent, aStartOffset); >+ *aRange = new nsRange(); >+ NS_ADDREF((*aRange)); ditto

Comment on attachment 570580 [details] [diff] [review] Part 3: Remove ns(I)RangeUtils ># HG changeset patch ># Parent 215fae0d65890241596e437273f98d027b75a15c ># User David Zbarsky <dzbarsky@gmail.com> >Bug 682611 - Part 3: Remove ns(I)RangeUtils; r?smaug > >diff --git a/content/base/public/Makefile.in b/content/base/public/Makefile.in >--- a/content/base/public/Makefile.in >+++ b/content/base/public/Makefile.in >@@ -57,17 +57,16 @@ nsIDocument.h \ > nsDeprecatedOperationList.h \ > nsIDocumentObserver.h \ > nsIMutationObserver.h \ > nsIMutationObserver2.h \ > nsINameSpaceManager.h \ > nsINode.h \ > nsINodeInfo.h \ > nsINodeList.h \ >-nsIRangeUtils.h \ > nsIScriptElement.h \ > nsIStyleSheetLinkingElement.h \ > nsIContentSerializer.h \ > nsIHTMLToTextSink.h \ > nsIXPathEvaluatorInternal.h \ > mozISanitizingSerializer.h \ > nsCaseTreatment.h \ > nsContentCID.h \ >diff --git a/content/base/public/nsContentCID.h b/content/base/public/nsContentCID.h >--- a/content/base/public/nsContentCID.h >+++ b/content/base/public/nsContentCID.h >@@ -173,20 +173,16 @@ > // {1abdcc96-1dd2-11b2-b520-f8f59cdd67bc} > #define NS_XULTREEBUILDER_CID \ > { 0x1abdcc96, 0x1dd2, 0x11b2, { 0xb5, 0x20, 0xf8, 0xf5, 0x9c, 0xdd, 0x67, 0xbc } } > > // {541AFCB2-A9A3-11d2-8EC5-00805F29F370} > #define NS_XULDOCUMENT_CID \ > { 0x541afcb2, 0xa9a3, 0x11d2, { 0x8e, 0xc5, 0x0, 0x80, 0x5f, 0x29, 0xf3, 0x70 } } > >-// {a6cf9126-15b3-11d2-932e-00805f8add32} >-#define NS_RANGEUTILS_CID \ >-{ 0xa6cf9126, 0x15b3, 0x11d2, {0x93, 0x2e, 0x00, 0x80, 0x5f, 0x8a, 0xdd, 0x32 } } >- > #define NS_SVGDOCUMENT_CID \ > { /* b7f44954-1dd1-11b2-8c2e-c2feab4186bc */ \ > 0xb7f44954, 0x11d1, 0x11b2, \ > {0x8c, 0x2e, 0xc2, 0xfe, 0xab, 0x41, 0x86, 0xbc}} > > #ifdef MOZ_MEDIA > > // {d899a152-9412-46b2-b651-2e71c5c2f05f} >diff --git a/content/base/public/nsContentUtils.h b/content/base/public/nsContentUtils.h >--- a/content/base/public/nsContentUtils.h >+++ b/content/base/public/nsContentUtils.h >@@ -319,16 +319,19 @@ public: > * 0 if error or if point1 == point2. > * NOTE! If the two nodes aren't in the same connected subtree, > * the result is 1, and the optional aDisconnected parameter > * is set to true. > */ > static PRInt32 ComparePoints(nsINode* aParent1, PRInt32 aOffset1, > nsINode* aParent2, PRInt32 aOffset2, > bool* aDisconnected = nsnull); >+ static PRInt32 ComparePoints(nsIDOMNode* aParent1, PRInt32 aOffset1, >+ nsIDOMNode* aParent2, PRInt32 aOffset2, >+ bool* aDisconnected = nsnull); > > /** > * Brute-force search of the element subtree rooted at aContent for > * an element with the given id. aId must be nonempty, otherwise > * this method may return nodes even if they have no id! > */ > static Element* MatchElementId(nsIContent *aContent, const nsAString& aId); > >diff --git a/content/base/public/nsIRangeUtils.h b/content/base/public/nsIRangeUtils.h >deleted file mode 100644 >--- a/content/base/public/nsIRangeUtils.h >+++ /dev/null >@@ -1,72 +0,0 @@ >-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- >- * >- * ***** BEGIN LICENSE BLOCK ***** >- * Version: MPL 1.1/GPL 2.0/LGPL 2.1 >- * >- * The contents of this file are subject to the Mozilla Public License Version >- * 1.1 (the "License"); you may not use this file except in compliance with >- * the License. You may obtain a copy of the License at >- * http://www.mozilla.org/MPL/ >- * >- * Software distributed under the License is distributed on an "AS IS" basis, >- * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License >- * for the specific language governing rights and limitations under the >- * License. >- * >- * The Original Code is Mozilla Communicator client code, released >- * March 31, 1998. >- * >- * The Initial Developer of the Original Code is >- * Netscape Communications Corporation. >- * Portions created by the Initial Developer are Copyright (C) 1998 >- * the Initial Developer. All Rights Reserved. >- * >- * Contributor(s): >- * >- * Alternatively, the contents of this file may be used under the terms of >- * either of the GNU General Public License Version 2 or later (the "GPL"), >- * or the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), >- * in which case the provisions of the GPL or the LGPL are applicable instead >- * of those above. If you wish to allow use of your version of this file only >- * under the terms of either the GPL or the LGPL, and not to allow others to >- * use your version of this file under the terms of the MPL, indicate your >- * decision by deleting the provisions above and replace them with the notice >- * and other provisions required by the GPL or the LGPL. If you do not delete >- * the provisions above, a recipient may use your version of this file under >- * the terms of any one of the MPL, the GPL or the LGPL. >- * >- * ***** END LICENSE BLOCK ***** */ >- >-/* A class for range utilities. */ >- >-#ifndef nsIRangeUtils_h___ >-#define nsIRangeUtils_h___ >- >-#include "nsISupports.h" >- >-// Forward declarations >-class nsRange; >-class nsIDOMNode; >-class nsIContent; >- >-// IID for the nsIRangeUtils interface >-#define NS_IRANGEUTILS_IID \ >-{ 0xa6cf9127, 0x15b3, 0x11d2, {0x93, 0x2e, 0x00, 0x80, 0x5f, 0x8a, 0xdd, 0x32} } >- >-class nsIRangeUtils : public nsISupports { >-public: >- NS_DECLARE_STATIC_IID_ACCESSOR(NS_IRANGEUTILS_IID) >- >- NS_IMETHOD_(PRInt32) ComparePoints(nsIDOMNode* aParent1, PRInt32 aOffset1, >- nsIDOMNode* aParent2, PRInt32 aOffset2) = 0; >- >- NS_IMETHOD CompareNodeToRange(nsIContent* aNode, >- nsRange* aRange, >- bool *outNodeBefore, >- bool *outNodeAfter) = 0; >-}; >- >-NS_DEFINE_STATIC_IID_ACCESSOR(nsIRangeUtils, NS_IRANGEUTILS_IID) >- >-#endif /* nsIRangeUtils_h___ */ >- >diff --git a/content/base/src/nsContentUtils.cpp b/content/base/src/nsContentUtils.cpp >--- a/content/base/src/nsContentUtils.cpp >+++ b/content/base/src/nsContentUtils.cpp >@@ -1672,16 +1672,28 @@ nsContentUtils::ComparePoints(nsINode* a > nsINode* child2 = parents2.ElementAt(--pos2); > return aOffset1 <= parent->IndexOf(child2) ? -1 : 1; > } > > nsINode* child1 = parents1.ElementAt(--pos1); > return parent->IndexOf(child1) < aOffset2 ? -1 : 1; > } > >+/* static */ >+PRInt32 >+nsContentUtils::ComparePoints(nsIDOMNode* aParent1, PRInt32 aOffset1, >+ nsIDOMNode* aParent2, PRInt32 aOffset2, >+ bool* aDisconnected) >+{ >+ nsCOMPtr<nsINode> parent1 = do_QueryInterface(aParent1); >+ nsCOMPtr<nsINode> parent2 = do_QueryInterface(aParent2); >+ NS_ENSURE_TRUE(parent1 && parent2, -1); >+ return ComparePoints(parent1, aOffset1, parent2, aOffset2); >+} >+ > inline bool > IsCharInSet(const char* aSet, > const PRUnichar aChar) > { > PRUnichar ch; > while ((ch = *aSet)) { > if (aChar == PRUnichar(ch)) { > return true; >diff --git a/content/base/src/nsRange.cpp b/content/base/src/nsRange.cpp >--- a/content/base/src/nsRange.cpp >+++ b/content/base/src/nsRange.cpp >@@ -140,62 +140,16 @@ nsRange::CompareNodeToRange(nsINode* aNo > rangeEndOffset, > parent, nodeEnd, > &disconnected) < 0; > NS_ENSURE_TRUE(!disconnected, NS_ERROR_DOM_WRONG_DOCUMENT_ERR); > return NS_OK; > } > > /****************************************************** >- * non members >- ******************************************************/ >- >-nsresult >-NS_NewRangeUtils(nsIRangeUtils** aResult) >-{ >- NS_ENSURE_ARG_POINTER(aResult); >- >- nsRangeUtils* rangeUtil = new nsRangeUtils(); >- if (!rangeUtil) { >- return NS_ERROR_OUT_OF_MEMORY; >- } >- >- return CallQueryInterface(rangeUtil, aResult); >-} >- >-/****************************************************** >- * nsISupports >- ******************************************************/ >-NS_IMPL_ISUPPORTS1(nsRangeUtils, nsIRangeUtils) >- >-/****************************************************** >- * nsIRangeUtils methods >- ******************************************************/ >- >-NS_IMETHODIMP_(PRInt32) >-nsRangeUtils::ComparePoints(nsIDOMNode* aParent1, PRInt32 aOffset1, >- nsIDOMNode* aParent2, PRInt32 aOffset2) >-{ >- nsCOMPtr<nsINode> parent1 = do_QueryInterface(aParent1); >- nsCOMPtr<nsINode> parent2 = do_QueryInterface(aParent2); >- >- NS_ENSURE_TRUE(parent1 && parent2, -1); >- >- return nsContentUtils::ComparePoints(parent1, aOffset1, parent2, aOffset2); >-} >- >-NS_IMETHODIMP >-nsRangeUtils::CompareNodeToRange(nsIContent* aNode, nsRange* aRange, >- bool *outNodeBefore, bool *outNodeAfter) >-{ >- return nsRange::CompareNodeToRange(aNode, aRange, outNodeBefore, >- outNodeAfter); >-} >- >-/****************************************************** > * constructor/destructor > ******************************************************/ > > nsRange::~nsRange() > { > DoSetRange(nsnull, 0, nsnull, 0, nsnull); > // we want the side effects (releases and list removals) > } >diff --git a/content/base/src/nsRange.h b/content/base/src/nsRange.h >--- a/content/base/src/nsRange.h >+++ b/content/base/src/nsRange.h >@@ -38,17 +38,16 @@ > /* > * Implementation of the DOM nsIDOMRange object. > */ > > #ifndef nsRange_h___ > #define nsRange_h___ > > #include "nsIDOMRange.h" >-#include "nsIRangeUtils.h" > #include "nsCOMPtr.h" > #include "nsIDOMDocumentFragment.h" > #include "nsIContent.h" > #include "nsIDOMNode.h" > #include "prmon.h" > #include "nsStubMutationObserver.h" > > class nsRange : public nsIDOMRange, >@@ -182,30 +181,9 @@ protected: > PRInt32 mStartOffset; > PRInt32 mEndOffset; > > bool mIsPositioned; > bool mIsDetached; > bool mMaySpanAnonymousSubtrees; > }; > >- >-// ------------------------------------------------------------------------------- >- >-class nsRangeUtils : public nsIRangeUtils >-{ >-public: >- NS_DECL_ISUPPORTS >- >- // nsIRangeUtils interface >- NS_IMETHOD_(PRInt32) ComparePoints(nsIDOMNode* aParent1, PRInt32 aOffset1, >- nsIDOMNode* aParent2, PRInt32 aOffset2); >- >- NS_IMETHOD CompareNodeToRange(nsIContent* aNode, >- nsRange* aRange, >- bool *outNodeBefore, >- bool *outNodeAfter); >-}; >- >-// Make a new nsIRangeUtils object >-nsresult NS_NewRangeUtils(nsIRangeUtils** aInstancePtrResult); >- > #endif /* nsRange_h___ */ >diff --git a/editor/libeditor/html/nsHTMLEditRules.cpp b/editor/libeditor/html/nsHTMLEditRules.cpp >--- a/editor/libeditor/html/nsHTMLEditRules.cpp >+++ b/editor/libeditor/html/nsHTMLEditRules.cpp >@@ -55,17 +55,16 @@ > #include "nsIDOMNode.h" > #include "nsIDOMText.h" > #include "nsIDOMElement.h" > #include "nsIDOMNodeList.h" > #include "nsISelection.h" > #include "nsISelectionPrivate.h" > #include "nsISelectionController.h" > #include "nsIDOMRange.h" >-#include "nsIRangeUtils.h" > #include "nsIDOMCharacterData.h" > #include "nsIEnumerator.h" > #include "nsIDOMNamedNodeMap.h" > #include "nsRange.h" > > #include "nsEditorUtils.h" > #include "nsWSRunObject.h" > >@@ -5255,17 +5254,17 @@ nsHTMLEditRules::ExpandSelectionForDelet > nsRefPtr<nsRange> range = new nsRange(); > res = range->SetStart(selStartNode, selStartOffset); > NS_ENSURE_SUCCESS(res, res); > res = range->SetEnd(selEndNode, selEndOffset); > NS_ENSURE_SUCCESS(res, res); > > // check if block is entirely inside range > nsCOMPtr<nsIContent> brContentBlock = do_QueryInterface(brBlock); >- res = mHTMLEditor->sRangeHelper->CompareNodeToRange(brContentBlock, range, &nodeBefore, &nodeAfter); >+ res = nsRange::CompareNodeToRange(brContentBlock, range, &nodeBefore, &nodeAfter); > > // if block isn't contained, forgo grabbing the br in the expanded selection > if (nodeBefore || nodeAfter) > doEndExpansion = false; > } > if (doEndExpansion) > { > res = aSelection->Extend(selEndNode, selEndOffset); >@@ -5487,19 +5486,21 @@ nsHTMLEditRules::NormalizeSelection(nsIS > // that is not collapsed and yet does not contain any editable content, and satisfies some of the > // above conditions that cause tweaking. In this case we don't want to tweak the selection into > // a block it was never in, etc. There are a variety of strategies one might use to try to > // detect these cases, but I think the most straightforward is to see if the adjusted locations > // "cross" the old values: ie, new end before old start, or new start after old end. If so > // then just leave things alone. > > PRInt16 comp; >- comp = mHTMLEditor->sRangeHelper->ComparePoints(startNode, startOffset, newEndNode, newEndOffset); >+ comp = nsContentUtils::ComparePoints(startNode, startOffset, >+ newEndNode, newEndOffset); > if (comp == 1) return NS_OK; // new end before old start >- comp = mHTMLEditor->sRangeHelper->ComparePoints(newStartNode, newStartOffset, endNode, endOffset); >+ comp = nsContentUtils::ComparePoints(newStartNode, newStartOffset, >+ endNode, endOffset); > if (comp == 1) return NS_OK; // new start after old end > > // otherwise set selection to new values. > inSelection->Collapse(newStartNode, newStartOffset); > inSelection->Extend(newEndNode, newEndOffset); > return NS_OK; > } > >@@ -7635,17 +7636,17 @@ nsHTMLEditRules::PinSelectionToNewBlock( > nsRefPtr<nsRange> range = new nsRange(); > res = range->SetStart(selNode, selOffset); > NS_ENSURE_SUCCESS(res, res); > res = range->SetEnd(selNode, selOffset); > NS_ENSURE_SUCCESS(res, res); > nsCOMPtr<nsIContent> block (do_QueryInterface(mNewBlock)); > NS_ENSURE_TRUE(block, NS_ERROR_NO_INTERFACE); > bool nodeBefore, nodeAfter; >- res = mHTMLEditor->sRangeHelper->CompareNodeToRange(block, range, &nodeBefore, &nodeAfter); >+ res = nsRange::CompareNodeToRange(block, range, &nodeBefore, &nodeAfter); > NS_ENSURE_SUCCESS(res, res); > > if (nodeBefore && nodeAfter) > return NS_OK; // selection is inside block > else if (nodeBefore) > { > // selection is after block. put at end of block. > nsCOMPtr<nsIDOMNode> tmp = mNewBlock; >diff --git a/editor/libeditor/html/nsHTMLEditor.cpp b/editor/libeditor/html/nsHTMLEditor.cpp >--- a/editor/libeditor/html/nsHTMLEditor.cpp >+++ b/editor/libeditor/html/nsHTMLEditor.cpp >@@ -67,17 +67,16 @@ > #include "mozilla/css/Loader.h" > #include "nsCSSStyleSheet.h" > #include "nsIDOMStyleSheet.h" > > #include "nsIEnumerator.h" > #include "nsIContent.h" > #include "nsIContentIterator.h" > #include "nsIDOMRange.h" >-#include "nsIRangeUtils.h" > #include "nsISupportsArray.h" > #include "nsContentUtils.h" > #include "nsIDocumentEncoder.h" > #include "nsIDOMDocumentFragment.h" > #include "nsIPresShell.h" > #include "nsPresContext.h" > #include "SetDocTitleTxn.h" > #include "nsFocusManager.h" >@@ -100,18 +99,16 @@ > #include "nsIParserService.h" > #include "mozilla/dom/Element.h" > > // Some utilities to handle annoying overloading of "A" tag for link and named anchor > static char hrefText[] = "href"; > static char anchorTxt[] = "anchor"; > static char namedanchorText[] = "namedanchor"; > >-nsIRangeUtils* nsHTMLEditor::sRangeHelper; >- > #define IsLinkTag(s) (s.EqualsIgnoreCase(hrefText)) > #define IsNamedAnchorTag(s) (s.EqualsIgnoreCase(anchorTxt) || s.EqualsIgnoreCase(namedanchorText)) > > nsHTMLEditor::nsHTMLEditor() > : nsPlaintextEditor() > , mIgnoreSpuriousDragEvent(false) > , mCRInParagraphCreatesParagraph(false) > , mSelectedCellIndex(0) >@@ -187,23 +184,16 @@ nsHTMLEditor::HideAnonymousEditingUIs() > if (mAbsolutelyPositionedObject) > HideGrabber(); > if (mInlineEditedCell) > HideInlineTableEditingUI(); > if (mResizedObject) > HideResizers(); > } > >-/* static */ >-void >-nsHTMLEditor::Shutdown() >-{ >- NS_IF_RELEASE(sRangeHelper); >-} >- > NS_IMPL_CYCLE_COLLECTION_CLASS(nsHTMLEditor) > > NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(nsHTMLEditor, nsPlaintextEditor) > NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mTypeInState) > NS_IMPL_CYCLE_COLLECTION_UNLINK_NSCOMPTR(mTextServices) > > tmp->HideAnonymousEditingUIs(); > NS_IMPL_CYCLE_COLLECTION_UNLINK_END >@@ -262,23 +252,16 @@ nsHTMLEditor::Init(nsIDOMDocument *aDoc, > nsIContent *aRoot, > nsISelectionController *aSelCon, > PRUint32 aFlags) > { > NS_PRECONDITION(aDoc && !aSelCon, "bad arg"); > NS_ENSURE_TRUE(aDoc, NS_ERROR_NULL_POINTER); > > nsresult result = NS_OK, rulesRes = NS_OK; >- >- // make a range util object for comparing dom points >- if (!sRangeHelper) { >- result = CallGetService("@mozilla.org/content/range-utils;1", >- &sRangeHelper); >- NS_ENSURE_TRUE(sRangeHelper, result); >- } > > if (1) > { > // block to scope nsAutoEditInitRulesTrigger > nsAutoEditInitRulesTrigger rulesTrigger(static_cast<nsPlaintextEditor*>(this), rulesRes); > > // Init the plaintext editor > result = nsPlaintextEditor::Init(aDoc, aRoot, nsnull, aFlags); >diff --git a/editor/libeditor/html/nsHTMLEditor.h b/editor/libeditor/html/nsHTMLEditor.h >--- a/editor/libeditor/html/nsHTMLEditor.h >+++ b/editor/libeditor/html/nsHTMLEditor.h >@@ -74,17 +74,16 @@ > > class nsIDOMKeyEvent; > class nsITransferable; > class nsIDocumentEncoder; > class nsIClipboard; > class TypeInState; > class nsIContentFilter; > class nsIURL; >-class nsIRangeUtils; > class nsILinkHandler; > struct PropItem; > > /** > * The HTML editor implementation.<br> > * Use to edit HTML document represented as a DOM tree. > */ > class nsHTMLEditor : public nsPlaintextEditor, >@@ -793,23 +792,16 @@ protected: > nsTArray<nsRefPtr<nsCSSStyleSheet> > mStyleSheets; > > // an array for holding default style settings > nsTArray<PropItem*> mDefaultStyles; > > // for real-time spelling > nsCOMPtr<nsITextServicesDocument> mTextServices; > >- // And a static range utils service >- static nsIRangeUtils* sRangeHelper; >- >-public: >- // ... which means that we need to listen to shutdown >- static void Shutdown(); >- > protected: > > /* ANONYMOUS UTILS */ > void RemoveListenerAndDeleteRef(const nsAString& aEvent, > nsIDOMEventListener* aListener, > bool aUseCapture, > nsIDOMElement* aElement, > nsIContent* aParentContent, >diff --git a/editor/libeditor/html/nsWSRunObject.cpp b/editor/libeditor/html/nsWSRunObject.cpp >--- a/editor/libeditor/html/nsWSRunObject.cpp >+++ b/editor/libeditor/html/nsWSRunObject.cpp >@@ -38,18 +38,18 @@ > #include "nsTextFragment.h" > #include "nsWSRunObject.h" > #include "nsIDOMNode.h" > #include "nsHTMLEditor.h" > #include "nsTextEditUtils.h" > #include "nsIContent.h" > #include "nsIDOMCharacterData.h" > #include "nsCRT.h" >-#include "nsIRangeUtils.h" > #include "nsRange.h" >+#include "nsContentUtils.h" > > const PRUnichar nbsp = 160; > > static bool IsBlockNode(nsIDOMNode* node) > { > bool isBlock (false); > nsHTMLEditor::NodeIsBlockStatic(node, &isBlock); > return isBlock; >@@ -1597,17 +1597,17 @@ nsWSRunObject::DeleteChars(nsIDOMNode *a > range = new nsRange(); > res = range->SetStart(aStartNode, aStartOffset); > NS_ENSURE_SUCCESS(res, res); > res = range->SetEnd(aEndNode, aEndOffset); > NS_ENSURE_SUCCESS(res, res); > } > bool nodeBefore, nodeAfter; > nsCOMPtr<nsIContent> content (do_QueryInterface(node)); >- res = mHTMLEditor->sRangeHelper->CompareNodeToRange(content, range, &nodeBefore, &nodeAfter); >+ res = nsRange::CompareNodeToRange(content, range, &nodeBefore, &nodeAfter); > NS_ENSURE_SUCCESS(res, res); > if (nodeAfter) > { > break; > } > if (!nodeBefore) > { > res = mHTMLEditor->DeleteNode(node); >@@ -1855,31 +1855,33 @@ nsWSRunObject::FindRun(nsIDOMNode *aNode > { > // given a dompoint, find the ws run that is before or after it, as caller needs > NS_ENSURE_TRUE(aNode && outRun, NS_ERROR_NULL_POINTER); > > nsresult res = NS_OK; > WSFragment *run = mStartRun; > while (run) > { >- PRInt16 comp = mHTMLEditor->sRangeHelper->ComparePoints(aNode, aOffset, run->mStartNode, run->mStartOffset); >+ PRInt16 comp = nsContentUtils::ComparePoints(aNode, aOffset, run->mStartNode, >+ run->mStartOffset); > if (comp <= 0) > { > if (after) > { > *outRun = run; > return res; > } > else // before > { > *outRun = nsnull; > return res; > } > } >- comp = mHTMLEditor->sRangeHelper->ComparePoints(aNode, aOffset, run->mEndNode, run->mEndOffset); >+ comp = nsContentUtils::ComparePoints(aNode, aOffset, >+ run->mEndNode, run->mEndOffset); > if (comp < 0) > { > *outRun = run; > return res; > } > else if (comp == 0) > { > if (after) >@@ -1942,17 +1944,17 @@ nsWSRunObject::GetWSPointAfter(nsIDOMNod > nsCOMPtr<nsIDOMNode> curNode; > > // begin binary search > // we do this because we need to minimize calls to ComparePoints(), > // which is mongo expensive > while (curNum != lastNum) > { > curNode = mNodeArray[curNum]; >- cmp = mHTMLEditor->sRangeHelper->ComparePoints(aNode, aOffset, curNode, 0); >+ cmp = nsContentUtils::ComparePoints(aNode, aOffset, curNode, 0); > if (cmp < 0) > lastNum = curNum; > else > firstNum = curNum + 1; > curNum = (lastNum - firstNum) / 2 + firstNum; > NS_ASSERTION(firstNum <= curNum && curNum <= lastNum, "Bad binary search"); > } > >@@ -1991,17 +1993,17 @@ nsWSRunObject::GetWSPointBefore(nsIDOMNo > nsCOMPtr<nsIDOMNode> curNode; > > // begin binary search > // we do this because we need to minimize calls to ComparePoints(), > // which is mongo expensive > while (curNum != lastNum) > { > curNode = mNodeArray[curNum]; >- cmp = mHTMLEditor->sRangeHelper->ComparePoints(aNode, aOffset, curNode, 0); >+ cmp = nsContentUtils::ComparePoints(aNode, aOffset, curNode, 0); > if (cmp < 0) > lastNum = curNum; > else > firstNum = curNum + 1; > curNum = (lastNum - firstNum) / 2 + firstNum; > NS_ASSERTION(firstNum <= curNum && curNum <= lastNum, "Bad binary search"); > } > >diff --git a/editor/txtsvc/src/nsFilteredContentIterator.cpp b/editor/txtsvc/src/nsFilteredContentIterator.cpp >--- a/editor/txtsvc/src/nsFilteredContentIterator.cpp >+++ b/editor/txtsvc/src/nsFilteredContentIterator.cpp >@@ -37,17 +37,17 @@ > > #include "nsFilteredContentIterator.h" > #include "nsIContentIterator.h" > #include "nsComponentManagerUtils.h" > #include "nsIContent.h" > #include "nsString.h" > #include "nsIEnumerator.h" > >-#include "nsTextServicesDocument.h" >+#include "nsContentUtils.h" > > #include "nsIDOMNode.h" > > //------------------------------------------------------------ > nsFilteredContentIterator::nsFilteredContentIterator(nsITextServicesFilter* aFilter) : > mFilter(aFilter), > mDidSkip(false), > mIsOutOfRange(false), >@@ -249,24 +249,20 @@ ContentIsInTraversalRange(nsIContent *aC > > ContentToParentOffset(aContent, getter_AddRefs(parentNode), &indx); > > NS_ENSURE_TRUE(parentNode, false); > > if (!aIsPreMode) > ++indx; > >- PRInt32 startRes; >- PRInt32 endRes; >- nsresult rv = nsTextServicesDocument::ComparePoints(aStartNode, aStartOffset, parentNode, indx, &startRes); >- NS_ENSURE_SUCCESS(rv, false); >- >- rv = nsTextServicesDocument::ComparePoints(aEndNode, aEndOffset, parentNode, indx, &endRes); >- NS_ENSURE_SUCCESS(rv, false); >- >+ PRInt32 startRes = nsContentUtils::ComparePoints(aStartNode, aStartOffset, >+ parentNode, indx); >+ PRInt32 endRes = nsContentUtils::ComparePoints(aEndNode, aEndOffset, >+ parentNode, indx); > return (startRes <= 0) && (endRes >= 0); > } > > static bool > ContentIsInTraversalRange(nsIDOMRange *aRange, nsIDOMNode* aNextNode, bool aIsPreMode) > { > nsCOMPtr<nsIContent> content(do_QueryInterface(aNextNode)); > NS_ENSURE_TRUE(content, false); >diff --git a/editor/txtsvc/src/nsFilteredContentIterator.h b/editor/txtsvc/src/nsFilteredContentIterator.h >--- a/editor/txtsvc/src/nsFilteredContentIterator.h >+++ b/editor/txtsvc/src/nsFilteredContentIterator.h >@@ -38,17 +38,16 @@ > #ifndef nsFilteredContentIterator_h__ > #define nsFilteredContentIterator_h__ > > #include "nsIContentIterator.h" > #include "nsCOMPtr.h" > #include "nsIAtom.h" > #include "nsITextServicesFilter.h" > #include "nsRange.h" >-#include "nsIRangeUtils.h" > #include "nsCycleCollectionParticipant.h" > > /** > * > */ > class nsFilteredContentIterator : public nsIContentIterator > { > public: >diff --git a/editor/txtsvc/src/nsTextServicesDocument.cpp b/editor/txtsvc/src/nsTextServicesDocument.cpp >--- a/editor/txtsvc/src/nsTextServicesDocument.cpp >+++ b/editor/txtsvc/src/nsTextServicesDocument.cpp >@@ -44,17 +44,17 @@ > #include "nsIAtom.h" > #include "nsStaticAtom.h" > #include "nsString.h" > #include "nsIEnumerator.h" > #include "nsIContent.h" > #include "nsIContentIterator.h" > #include "nsIDOMNodeList.h" > #include "nsIDOMRange.h" >-#include "nsIRangeUtils.h" >+#include "nsContentUtils.h" > #include "nsISelection.h" > #include "nsIPlaintextEditor.h" > #include "nsTextServicesDocument.h" > #include "nsFilteredContentIterator.h" > > #include "nsIDOMElement.h" > #include "nsIDOMHTMLElement.h" > #include "nsIDOMHTMLDocument.h" >@@ -98,18 +98,16 @@ public: > bool mIsInsertedText; > bool mIsValid; > }; > > #define TS_ATOM(name_, value_) nsIAtom* nsTextServicesDocument::name_ = 0; > #include "nsTSAtomList.h" > #undef TS_ATOM > >-nsIRangeUtils* nsTextServicesDocument::sRangeHelper; >- > nsTextServicesDocument::nsTextServicesDocument() > { > mRefCnt = 0; > > mSelStartIndex = -1; > mSelStartOffset = -1; > mSelEndIndex = -1; > mSelEndOffset = -1; >@@ -134,23 +132,16 @@ nsTextServicesDocument::RegisterAtoms() > #define TS_ATOM(name_, value_) NS_STATIC_ATOM(name_##_buffer, &name_), > #include "nsTSAtomList.h" > #undef TS_ATOM > }; > > NS_RegisterStaticAtoms(ts_atoms, ArrayLength(ts_atoms)); > } > >-/* static */ >-void >-nsTextServicesDocument::Shutdown() >-{ >- NS_IF_RELEASE(sRangeHelper); >-} >- > NS_IMPL_CYCLE_COLLECTING_ADDREF(nsTextServicesDocument) > NS_IMPL_CYCLE_COLLECTING_RELEASE(nsTextServicesDocument) > > NS_INTERFACE_MAP_BEGIN(nsTextServicesDocument) > NS_INTERFACE_MAP_ENTRY(nsITextServicesDocument) > NS_INTERFACE_MAP_ENTRY(nsIEditActionListener) > NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsITextServicesDocument) > NS_INTERFACE_MAP_ENTRIES_CYCLE_COLLECTION(nsTextServicesDocument) >@@ -2710,23 +2701,20 @@ nsTextServicesDocument::GetCollapsedSele > result = range->GetStartContainer(getter_AddRefs(parent)); > > NS_ENSURE_SUCCESS(result, result); > > result = range->GetStartOffset(&offset); > > NS_ENSURE_SUCCESS(result, result); > >- result = ComparePoints(eStart->mNode, eStartOffset, parent, offset, &e1s1); >- >- NS_ENSURE_SUCCESS(result, result); >- >- result = ComparePoints(eEnd->mNode, eEndOffset, parent, offset, &e2s1); >- >- NS_ENSURE_SUCCESS(result, result); >+ e1s1 = nsContentUtils::ComparePoints(eStart->mNode, eStartOffset, >+ parent, offset); >+ e2s1 = nsContentUtils::ComparePoints(eEnd->mNode, eEndOffset, >+ parent, offset); > > if (e1s1 > 0 || e2s1 < 0) > { > // We're done if the caret is outside the > // current text block. > > return NS_OK; > } >@@ -3004,23 +2992,20 @@ nsTextServicesDocument::GetUncollapsedSe > NS_ENSURE_SUCCESS(result, result); > > result = GetRangeEndPoints(range, > getter_AddRefs(startParent), &startOffset, > getter_AddRefs(endParent), &endOffset); > > NS_ENSURE_SUCCESS(result, result); > >- result = ComparePoints(eStart->mNode, eStartOffset, endParent, endOffset, &e1s2); >- >- NS_ENSURE_SUCCESS(result, result); >- >- result = ComparePoints(eEnd->mNode, eEndOffset, startParent, startOffset, &e2s1); >- >- NS_ENSURE_SUCCESS(result, result); >+ e1s2 = nsContentUtils::ComparePoints(eStart->mNode, eStartOffset, >+ endParent, endOffset); >+ e2s1 = nsContentUtils::ComparePoints(eEnd->mNode, eEndOffset, >+ startParent, startOffset); > > // Break out of the loop if the text block intersects the current range. > > if (e1s2 <= 0 && e2s1 >= 0) > break; > } > > // We're done if we didn't find an intersecting range. >@@ -3029,23 +3014,20 @@ nsTextServicesDocument::GetUncollapsedSe > { > *aSelStatus = nsITextServicesDocument::eBlockOutside; > *aSelOffset = *aSelLength = -1; > return NS_OK; > } > > // Now that we have an intersecting range, find out more info: > >- result = ComparePoints(eStart->mNode, eStartOffset, startParent, startOffset, &e1s1); >- >- NS_ENSURE_SUCCESS(result, result); >- >- result = ComparePoints(eEnd->mNode, eEndOffset, endParent, endOffset, &e2s2); >- >- NS_ENSURE_SUCCESS(result, result); >+ e1s1 = nsContentUtils::ComparePoints(eStart->mNode, eStartOffset, >+ startParent, startOffset); >+ e2s2 = nsContentUtils::ComparePoints(eEnd->mNode, eEndOffset, >+ endParent, endOffset); > > if (rangeCount > 1) > { > // There are multiple selection ranges, we only deal > // with the first one that intersects the current, > // text block, so mark this a as a partial. > > *aSelStatus = nsITextServicesDocument::eBlockPartial; >@@ -3254,34 +3236,16 @@ nsTextServicesDocument::SelectionIsColla > > bool > nsTextServicesDocument::SelectionIsValid() > { > return(mSelStartIndex >= 0); > } > > nsresult >-nsTextServicesDocument::ComparePoints(nsIDOMNode* aParent1, PRInt32 aOffset1, >- nsIDOMNode* aParent2, PRInt32 aOffset2, >- PRInt32 *aResult) >-{ >- nsresult result; >- >- if (!sRangeHelper) { >- result = CallGetService("@mozilla.org/content/range-utils;1", >- &sRangeHelper); >- NS_ENSURE_TRUE(sRangeHelper, result); >- } >- >- *aResult = sRangeHelper->ComparePoints(aParent1, aOffset1, >- aParent2, aOffset2); >- return NS_OK; >-} >- >-nsresult > nsTextServicesDocument::GetRangeEndPoints(nsIDOMRange *aRange, > nsIDOMNode **aStartParent, PRInt32 *aStartOffset, > nsIDOMNode **aEndParent, PRInt32 *aEndOffset) > { > nsresult result; > > NS_ENSURE_TRUE(aRange && aStartParent && aStartOffset && aEndParent && aEndOffset, NS_ERROR_NULL_POINTER); > >diff --git a/editor/txtsvc/src/nsTextServicesDocument.h b/editor/txtsvc/src/nsTextServicesDocument.h >--- a/editor/txtsvc/src/nsTextServicesDocument.h >+++ b/editor/txtsvc/src/nsTextServicesDocument.h >@@ -47,17 +47,16 @@ > #include "nsIEditActionListener.h" > #include "nsITextServicesDocument.h" > #include "nsTArray.h" > #include "nsISelectionController.h" > #include "nsITextServicesFilter.h" > #include "nsWeakReference.h" > #include "nsCycleCollectionParticipant.h" > >-class nsIRangeUtils; > class OffsetEntry; > > /** implementation of a text services object. > * > */ > class nsTextServicesDocument : public nsITextServicesDocument, > public nsIEditActionListener > { >@@ -109,36 +108,30 @@ private: > PRInt32 mSelStartOffset; > PRInt32 mSelEndIndex; > PRInt32 mSelEndOffset; > > nsCOMPtr<nsIDOMRange> mExtent; > > nsCOMPtr<nsITextServicesFilter> mTxtSvcFilter; > >- static nsIRangeUtils* sRangeHelper; >- > public: > > /** The default constructor. > */ > nsTextServicesDocument(); > > /** The default destructor. > */ > virtual ~nsTextServicesDocument(); > > /** To be called at module init > */ > static void RegisterAtoms(); > >- /** To be called at module shutdown >- */ >- static void Shutdown(); >- > /* Macro for AddRef(), Release(), and QueryInterface() */ > NS_DECL_CYCLE_COLLECTING_ISUPPORTS > NS_DECL_CYCLE_COLLECTION_CLASS_AMBIGUOUS(nsTextServicesDocument, nsITextServicesDocument) > > /* nsITextServicesDocument method implementations. */ > NS_IMETHOD InitWithEditor(nsIEditor *aEditor); > NS_IMETHOD GetDocument(nsIDOMDocument **aDoc); > NS_IMETHOD SetExtent(nsIDOMRange* aDOMRange); >@@ -187,17 +180,16 @@ public: > NS_IMETHOD WillInsertText(nsIDOMCharacterData *aTextNode, PRInt32 aOffset, const nsAString &aString); > NS_IMETHOD DidInsertText(nsIDOMCharacterData *aTextNode, PRInt32 aOffset, const nsAString &aString, nsresult aResult); > NS_IMETHOD WillDeleteText(nsIDOMCharacterData *aTextNode, PRInt32 aOffset, PRInt32 aLength); > NS_IMETHOD DidDeleteText(nsIDOMCharacterData *aTextNode, PRInt32 aOffset, PRInt32 aLength, nsresult aResult); > NS_IMETHOD WillDeleteSelection(nsISelection *aSelection); > NS_IMETHOD DidDeleteSelection(nsISelection *aSelection); > > /* Helper functions */ >- static nsresult ComparePoints(nsIDOMNode *aParent1, PRInt32 aOffset1, nsIDOMNode *aParent2, PRInt32 aOffset2, PRInt32 *aResult); > static nsresult GetRangeEndPoints(nsIDOMRange *aRange, nsIDOMNode **aParent1, PRInt32 *aOffset1, nsIDOMNode **aParent2, PRInt32 *aOffset2); > static nsresult CreateRange(nsIDOMNode *aStartParent, PRInt32 aStartOffset, nsIDOMNode *aEndParent, PRInt32 aEndOffset, nsIDOMRange **aRange); > > private: > /* nsTextServicesDocument private methods. */ > > nsresult CreateContentIterator(nsIDOMRange *aRange, nsIContentIterator **aIterator); > >diff --git a/layout/build/nsLayoutModule.cpp b/layout/build/nsLayoutModule.cpp >--- a/layout/build/nsLayoutModule.cpp >+++ b/layout/build/nsLayoutModule.cpp >@@ -64,17 +64,16 @@ > #include "nsHTMLStyleSheet.h" > #include "nsIHTMLToTextSink.h" > #include "nsILayoutDebugger.h" > #include "nsINameSpaceManager.h" > #include "nsINodeInfo.h" > #include "nsIObserver.h" > #include "nsIObserverService.h" > #include "nsIPresShell.h" >-#include "nsIRangeUtils.h" > #include "nsIScriptNameSpaceManager.h" > #include "nsISelection.h" > #include "nsIXBLService.h" > #include "nsCaret.h" > #include "nsPlainTextSerializer.h" > #include "mozSanitizingSerializer.h" > #include "nsXMLContentSerializer.h" > #include "nsXHTMLContentSerializer.h" >@@ -436,17 +435,16 @@ nsresult NS_NewTreeBoxObject(nsIBoxObjec > nsresult NS_NewCanvasRenderingContext2D(nsIDOMCanvasRenderingContext2D** aResult); > nsresult NS_NewCanvasRenderingContext2DThebes(nsIDOMCanvasRenderingContext2D** aResult); > nsresult NS_NewCanvasRenderingContextWebGL(nsIDOMWebGLRenderingContext** aResult); > > nsresult NS_CreateFrameTraversal(nsIFrameTraversal** aResult); > > nsresult NS_NewDomSelection(nsISelection** aResult); > nsresult NS_NewContentViewer(nsIContentViewer** aResult); >-nsresult NS_NewRangeUtils(nsIRangeUtils** aResult); > nsresult NS_NewContentIterator(nsIContentIterator** aResult); > nsresult NS_NewPreContentIterator(nsIContentIterator** aResult); > nsresult NS_NewGenRegularIterator(nsIContentIterator** aResult); > nsresult NS_NewContentSubtreeIterator(nsIContentIterator** aResult); > nsresult NS_NewGenSubtreeIterator(nsIContentIterator** aInstancePtrResult); > nsresult NS_NewContentDocumentLoaderFactory(nsIDocumentLoaderFactory** aResult); > nsresult NS_NewHTMLCopyTextEncoder(nsIDocumentEncoder** aResult); > nsresult NS_NewTextEncoder(nsIDocumentEncoder** aResult); >@@ -504,17 +502,16 @@ NS_GENERIC_FACTORY_CONSTRUCTOR(inDOMUtil > > MAKE_CTOR(CreateNameSpaceManager, nsINameSpaceManager, NS_GetNameSpaceManager) > MAKE_CTOR(CreateContentViewer, nsIContentViewer, NS_NewContentViewer) > MAKE_CTOR(CreateHTMLDocument, nsIDocument, NS_NewHTMLDocument) > MAKE_CTOR(CreateXMLDocument, nsIDocument, NS_NewXMLDocument) > MAKE_CTOR(CreateSVGDocument, nsIDocument, NS_NewSVGDocument) > MAKE_CTOR(CreateImageDocument, nsIDocument, NS_NewImageDocument) > MAKE_CTOR(CreateDOMSelection, nsISelection, NS_NewDomSelection) >-MAKE_CTOR(CreateRangeUtils, nsIRangeUtils, NS_NewRangeUtils) > MAKE_CTOR(CreateContentIterator, nsIContentIterator, NS_NewContentIterator) > MAKE_CTOR(CreatePreContentIterator, nsIContentIterator, NS_NewPreContentIterator) > MAKE_CTOR(CreateSubtreeIterator, nsIContentIterator, NS_NewContentSubtreeIterator) > // CreateHTMLImgElement, see below > // CreateHTMLOptionElement, see below > // CreateHTMLAudioElement, see below > MAKE_CTOR(CreateTextEncoder, nsIDocumentEncoder, NS_NewTextEncoder) > MAKE_CTOR(CreateHTMLCopyTextEncoder, nsIDocumentEncoder, NS_NewHTMLCopyTextEncoder) >@@ -738,17 +735,16 @@ NS_DEFINE_NAMED_CID(IN_CSSVALUESEARCH_CI > NS_DEFINE_NAMED_CID(IN_DOMUTILS_CID); > NS_DEFINE_NAMED_CID(NS_NAMESPACEMANAGER_CID); > NS_DEFINE_NAMED_CID(NS_CONTENT_VIEWER_CID); > NS_DEFINE_NAMED_CID(NS_HTMLDOCUMENT_CID); > NS_DEFINE_NAMED_CID(NS_XMLDOCUMENT_CID); > NS_DEFINE_NAMED_CID(NS_SVGDOCUMENT_CID); > NS_DEFINE_NAMED_CID(NS_IMAGEDOCUMENT_CID); > NS_DEFINE_NAMED_CID(NS_DOMSELECTION_CID); >-NS_DEFINE_NAMED_CID(NS_RANGEUTILS_CID); > NS_DEFINE_NAMED_CID(NS_CONTENTITERATOR_CID); > NS_DEFINE_NAMED_CID(NS_PRECONTENTITERATOR_CID); > NS_DEFINE_NAMED_CID(NS_SUBTREEITERATOR_CID); > NS_DEFINE_NAMED_CID(NS_HTMLIMAGEELEMENT_CID); > NS_DEFINE_NAMED_CID(NS_HTMLOPTIONELEMENT_CID); > #ifdef MOZ_MEDIA > NS_DEFINE_NAMED_CID(NS_HTMLAUDIOELEMENT_CID); > #endif >@@ -871,17 +867,16 @@ static const mozilla::Module::CIDEntry k > { &kIN_DOMUTILS_CID, false, NULL, inDOMUtilsConstructor }, > { &kNS_NAMESPACEMANAGER_CID, false, NULL, CreateNameSpaceManager }, > { &kNS_CONTENT_VIEWER_CID, false, NULL, CreateContentViewer }, > { &kNS_HTMLDOCUMENT_CID, false, NULL, CreateHTMLDocument }, > { &kNS_XMLDOCUMENT_CID, false, NULL, CreateXMLDocument }, > { &kNS_SVGDOCUMENT_CID, false, NULL, CreateSVGDocument }, > { &kNS_IMAGEDOCUMENT_CID, false, NULL, CreateImageDocument }, > { &kNS_DOMSELECTION_CID, false, NULL, CreateDOMSelection }, >- { &kNS_RANGEUTILS_CID, false, NULL, CreateRangeUtils }, > { &kNS_CONTENTITERATOR_CID, false, NULL, CreateContentIterator }, > { &kNS_PRECONTENTITERATOR_CID, false, NULL, CreatePreContentIterator }, > { &kNS_SUBTREEITERATOR_CID, false, NULL, CreateSubtreeIterator }, > { &kNS_HTMLIMAGEELEMENT_CID, false, NULL, CreateHTMLImgElement }, > { &kNS_HTMLOPTIONELEMENT_CID, false, NULL, CreateHTMLOptionElement }, > #ifdef MOZ_MEDIA > { &kNS_HTMLAUDIOELEMENT_CID, false, NULL, CreateHTMLAudioElement }, > #endif >@@ -995,17 +990,16 @@ static const mozilla::Module::ContractID > { "@mozilla.org/inspector/deep-tree-walker;1", &kIN_DEEPTREEWALKER_CID }, > { "@mozilla.org/inspector/flasher;1", &kIN_FLASHER_CID }, > { "@mozilla.org/inspector/search;1?type=cssvalue", &kIN_CSSVALUESEARCH_CID }, > { "@mozilla.org/inspector/dom-utils;1", &kIN_DOMUTILS_CID }, > { NS_NAMESPACEMANAGER_CONTRACTID, &kNS_NAMESPACEMANAGER_CID }, > { "@mozilla.org/xml/xml-document;1", &kNS_XMLDOCUMENT_CID }, > { "@mozilla.org/svg/svg-document;1", &kNS_SVGDOCUMENT_CID }, > { "@mozilla.org/content/dom-selection;1", &kNS_DOMSELECTION_CID }, >- { "@mozilla.org/content/range-utils;1", &kNS_RANGEUTILS_CID }, > { "@mozilla.org/content/post-content-iterator;1", &kNS_CONTENTITERATOR_CID }, > { "@mozilla.org/content/pre-content-iterator;1", &kNS_PRECONTENTITERATOR_CID }, > { "@mozilla.org/content/subtree-content-iterator;1", &kNS_SUBTREEITERATOR_CID }, > { NS_HTMLIMGELEMENT_CONTRACTID, &kNS_HTMLIMAGEELEMENT_CID }, > { NS_HTMLOPTIONELEMENT_CONTRACTID, &kNS_HTMLOPTIONELEMENT_CID }, > #ifdef MOZ_MEDIA > { NS_HTMLAUDIOELEMENT_CONTRACTID, &kNS_HTMLAUDIOELEMENT_CID }, > #endif >diff --git a/layout/build/nsLayoutStatics.cpp b/layout/build/nsLayoutStatics.cpp >--- a/layout/build/nsLayoutStatics.cpp >+++ b/layout/build/nsLayoutStatics.cpp >@@ -330,19 +330,16 @@ nsLayoutStatics::Shutdown() > > nsJSRuntime::Shutdown(); > nsGlobalWindow::ShutDown(); > nsDOMClassInfo::ShutDown(); > nsListControlFrame::Shutdown(); > nsXBLWindowKeyHandler::ShutDown(); > nsAutoCopyListener::Shutdown(); > >- nsHTMLEditor::Shutdown(); >- nsTextServicesDocument::Shutdown(); >- > #ifdef MOZ_SYDNEYAUDIO > nsAudioStream::ShutdownLibrary(); > #endif > > nsCORSListenerProxy::Shutdown(); > > nsIPresShell::ReleaseStatics(); >

Someone recently added this weird looking line: http://mxr.mozilla.org/mozilla-central/source/extensions/spellcheck/src/mozInlineSpellChecker.cpp#884 I'm changing that to: - nsCOMPtr<nsIRange> range = do_QueryInterface(NULL); // Check everything - rv = status.InitForRange(range); + rv = status.InitForRange(nsnull); ok?

I'd prefer NULL myself, but yes, that's right.

(In reply to Ms2ger from comment #32) > Part 1 may lose null checks in > > nsHTMLEditor::ReplaceHeadContentsWithHTM > nsHTMLEditor::RebuildDocumentFromSource > ContentIsInTraversalRange > mozInlineSpellStatus::InitForEditorChang > mozInlineSpellStatus::FinishNavigationEvent Good catch, I have investigated them: > nsHTMLEditor::ReplaceHeadContentsWithHTM nsCOMPtr<nsIDOMRange> range; res = selection->GetRangeAt(0, getter_AddRefs(range)); NS_ENSURE_SUCCESS(res, res); - nsCOMPtr<nsIDOMNSRange> nsrange (do_QueryInterface(range)); - NS_ENSURE_TRUE(nsrange, NS_ERROR_NO_INTERFACE); Yes, the null check is lost, but a successful GetRangeAt guarranties a non-null range. > nsHTMLEditor::RebuildDocumentFromSource nsCOMPtr<nsIDOMRange> range; res = selection->GetRangeAt(0, getter_AddRefs(range)); NS_ENSURE_SUCCESS(res, res); - nsCOMPtr<nsIDOMNSRange> nsrange (do_QueryInterface(range)); - NS_ENSURE_TRUE(nsrange, NS_ERROR_NO_INTERFACE); Same thing. > ContentIsInTraversalRange -ContentIsInTraversalRange(nsIDOMNSRange *aRange, nsIDOMNode* aNextNode, bool aIsPreMode) +ContentIsInTraversalRange(nsIDOMRange *aRange, nsIDOMNode* aNextNode, bool aIsPreMode) { - nsCOMPtr<nsIContent> content(do_QueryInterface(aNextNode)); - nsCOMPtr<nsIDOMRange> range(do_QueryInterface(aRange)); - NS_ENSURE_TRUE(content && range, false); + nsCOMPtr<nsIContent> content(do_QueryInterface(aNextNode)); + NS_ENSURE_TRUE(content, false); Yes, we're losing a null-check on aRange here. I've added it back. >mozInlineSpellStatus::InitForEditorChang - nsCOMPtr<nsIDOMNSRange> nsrange = do_QueryInterface(mAnchorRange, &rv); - NS_ENSURE_SUCCESS(rv, rv); PRInt16 cmpResult; - rv = nsrange->ComparePoint(aPreviousNode, aPreviousOffset, &cmpResult); + rv = mAnchorRange->ComparePoint(aPreviousNode, aPreviousOffset, &cmpResult); Yes, we're losing a null-check on mAnchorRange here, but mAnchorRange is set further up by PositionToCollapsedRange and we check for a successful return value so mAnchorRange is guarranteed to be non-null here. and later on mRange: - nsrange = do_QueryInterface(mRange, &rv); - NS_ENSURE_SUCCESS(rv, rv); - - rv = nsrange->ComparePoint(aStartNode, aStartOffset, &cmpResult); + rv = mRange->ComparePoint(aStartNode, aStartOffset, &cmpResult); At this point mRange is the result of "new nsRange()". >mozInlineSpellStatus::FinishNavigationEvent oldWord is set by GetRangeForWord and the result is checked for success. I've checked that the GetRangeForWord out-param can't be null with a successful result.

(In reply to Ms2ger from comment #36) > I'd prefer NULL myself, but yes, that's right. It appears the replacement of nsnull is still being discussed. I'll stick with nsnull until the tree-wide replacement happens - I'd rather not have both nsnull and NULL in the same file.

The intra-diff to add back the null-check is: static bool ContentIsInTraversalRange(nsIDOMRange *aRange, nsIDOMNode* aNextNode, bool aIsPreMode) { nsCOMPtr<nsIContent> content(do_QueryInterface(aNextNode)); - NS_ENSURE_TRUE(content, false); - - + NS_ENSURE_TRUE(content && aRange, false);

Thanks for checking so carefully!

Part 1 & 2 landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/6e016e20e726 https://hg.mozilla.org/integration/mozilla-inbound/rev/3c6cfdfe7d46

https://hg.mozilla.org/mozilla-central/rev/6e016e20e726 https://hg.mozilla.org/mozilla-central/rev/3c6cfdfe7d46 Leaving open for part 3.

Ms2ger, is part 3 ready to land as is?

Do you have access to the addons MXR to check if any of the add-ons there use nsIRangeUtils?

(In reply to Ms2ger from comment #44) > Do you have access to the addons MXR to check if any of the add-ons there > use nsIRangeUtils? I don't, as far as I know. Maybe Boris has? (or know someone who has)

I do have access to it, but it's pretty out of date. That said, there are not hits for nsIRangeUtils in what I do have access to.

I think we're good to go, then. Mats, do you want to do the honours?

I agree, looks good.

Part 3: Remove ns(I)RangeUtils: https://hg.mozilla.org/integration/mozilla-inbound/rev/d78b890eae96

Mats, could this have caused hang like this http://pastebin.mozilla.org/1477334

It seems unlikely that just decomtamination would cause a hang, unless we made a mistake somewhere of course. Is it the nsContentIterator that hangs because it never ends? It looks like the top of the stack handles a QuerySelectedText event, involving some content indexes maybe? We fixed a crash bug recently in that part of the code in bug 692145. If you can reproduce it, you could try before/after that fix.

justdave got the hang, perhaps he could tell more about it.

I can reproduce at will, which aurora nightly was that? I can try before/after builds and see.

bug 692145 should affect only on Windows, so, the comment 51's stack must be caused by something of others.

Dave, please file a new bug for tracking purposes; CC me, smaug and Ms2ger. This bug first landed on m-c around 2012-01-10, with part 3 landing around 2012-01-28. (see comment 42 and comment 49 for exact revisions)

The static_cast *before* the null-check is scary, because it relies on nsRange having nsIDOMRange as its first base class. I would also be happier if someone added a version of nsContentIterator::Init that takes an nsRange* directly.

Docs updated: https://developer.mozilla.org/en/XPCOM_Interface_Reference/nsISelectionPrivate The nsIRange stuff didn't need any changes given the internal nature of this change.