Bug 702948 - Make Range.detach() a no-op; r=smaug
authorAryeh Gregor <ayg@aryeh.name>
Wed, 25 Apr 2012 15:03:48 +0300
changeset 92413 c1e2504f48fd97bab353188588efeb5ff4dc3326
parent 92412 ba1c9beaf75f3bd8bb9d04001e6a645a96989d88
child 92414 956287d79825a84f2a5900c7a30e4a355fc85f39
push id22531
push userkhuey@mozilla.com
push dateThu, 26 Apr 2012 03:23:06 +0000
treeherdermozilla-central@b893f852fe7b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs702948
milestone15.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 702948 - Make Range.detach() a no-op; r=smaug
content/base/src/nsRange.cpp
content/base/src/nsRange.h
content/test/unit/test_range.js
layout/generic/test/test_bug496275.html
--- a/content/base/src/nsRange.cpp
+++ b/content/base/src/nsRange.cpp
@@ -78,19 +78,16 @@ nsresult NS_NewContentSubtreeIterator(ns
 #define VALIDATE_ACCESS(node_)                                                     \
   PR_BEGIN_MACRO                                                                   \
     if (!node_) {                                                                  \
       return NS_ERROR_DOM_NOT_OBJECT_ERR;                                          \
     }                                                                              \
     if (!nsContentUtils::CanCallerAccess(node_)) {                                 \
       return NS_ERROR_DOM_SECURITY_ERR;                                            \
     }                                                                              \
-    if (mIsDetached) {                                                             \
-      return NS_ERROR_DOM_INVALID_STATE_ERR;                                       \
-    }                                                                              \
   PR_END_MACRO
 
 static void InvalidateAllFrames(nsINode* aNode)
 {
   NS_PRECONDITION(aNode, "bad arg");
 
   nsIFrame* frame = nsnull;
   switch (aNode->NodeType()) {
@@ -648,19 +645,16 @@ nsRange::IsPointInRange(nsIDOMNode* aPar
   return rv;
 }
   
 // returns -1 if point is before range, 0 if point is in range,
 // 1 if point is after range.
 NS_IMETHODIMP
 nsRange::ComparePoint(nsIDOMNode* aParent, PRInt32 aOffset, PRInt16* aResult)
 {
-  if (mIsDetached)
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
-
   // our range is in a good state?
   if (!mIsPositioned) 
     return NS_ERROR_NOT_INITIALIZED;
 
   nsCOMPtr<nsINode> parent = do_QueryInterface(aParent);
   NS_ENSURE_TRUE(parent, NS_ERROR_DOM_HIERARCHY_REQUEST_ERR);
 
   if (!nsContentUtils::ContentIsDescendantOf(parent, mRoot)) {
@@ -750,18 +744,17 @@ nsRange::DoSetRange(nsINode* aStartN, PR
     nsINode* newCommonAncestor = GetCommonAncestor();
     if (newCommonAncestor != oldCommonAncestor) {
       if (oldCommonAncestor) {
         UnregisterCommonAncestor(oldCommonAncestor);
       }
       if (newCommonAncestor) {
         RegisterCommonAncestor(newCommonAncestor);
       } else {
-        NS_ASSERTION(mIsDetached || !mIsPositioned,
-                     "unexpected disconnected nodes");
+        NS_ASSERTION(!mIsPositioned, "unexpected disconnected nodes");
         mInSelection = false;
       }
     }
   }
 
   // This needs to be the last thing this function does.  See comment
   // in ParentChainChanged.
   mRoot = aRoot;
@@ -839,32 +832,28 @@ nsRange::GetEndOffset(PRInt32* aEndOffse
   *aEndOffset = mEndOffset;
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsRange::GetCollapsed(bool* aIsCollapsed)
 {
-  if(mIsDetached)
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
   if (!mIsPositioned)
     return NS_ERROR_NOT_INITIALIZED;
 
   *aIsCollapsed = Collapsed();
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsRange::GetCommonAncestorContainer(nsIDOMNode** aCommonParent)
 {
   *aCommonParent = nsnull;
-  if(mIsDetached)
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
   if (!mIsPositioned)
     return NS_ERROR_NOT_INITIALIZED;
 
   nsINode* container = nsContentUtils::GetCommonAncestor(mStartParent, mEndParent);
   if (container) {
     return CallQueryInterface(container, aCommonParent);
   }
 
@@ -1044,18 +1033,16 @@ nsRange::SetEndAfter(nsIDOMNode* aSiblin
   }
 
   return SetEnd(nParent, IndexOf(aSibling) + 1);
 }
 
 NS_IMETHODIMP
 nsRange::Collapse(bool aToStart)
 {
-  if(mIsDetached)
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
   if (!mIsPositioned)
     return NS_ERROR_NOT_INITIALIZED;
 
   AutoInvalidateSelection atEndOfBlock(this);
   if (aToStart)
     DoSetRange(mStartParent, mStartOffset, mStartParent, mStartOffset, mRoot);
   else
     DoSetRange(mEndParent, mEndOffset, mEndParent, mEndOffset, mRoot);
@@ -1497,19 +1484,16 @@ PrependChild(nsIDOMNode* aParent, nsIDOM
 }
 
 nsresult nsRange::CutContents(nsIDOMDocumentFragment** aFragment)
 { 
   if (aFragment) {
     *aFragment = nsnull;
   }
 
-  if (IsDetached())
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
-
   nsresult rv;
 
   nsCOMPtr<nsIDocument> doc = mStartParent->OwnerDoc();
 
   nsCOMPtr<nsIDOMNode> commonAncestor;
   rv = GetCommonAncestorContainer(getter_AddRefs(commonAncestor));
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -1777,18 +1761,16 @@ nsRange::ExtractContents(nsIDOMDocumentF
 
 NS_IMETHODIMP
 nsRange::CompareBoundaryPoints(PRUint16 aHow, nsIDOMRange* aOtherRange,
                                PRInt16* aCmpRet)
 {
   nsRange* otherRange = static_cast<nsRange*>(aOtherRange);
   NS_ENSURE_TRUE(otherRange, NS_ERROR_NULL_POINTER);
 
-  if(mIsDetached || otherRange->IsDetached())
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
   if (!mIsPositioned || !otherRange->IsPositioned())
     return NS_ERROR_NOT_INITIALIZED;
 
   nsINode *ourNode, *otherNode;
   PRInt32 ourOffset, otherOffset;
 
   switch (aHow) {
     case nsIDOMRange::START_TO_START:
@@ -1878,19 +1860,16 @@ nsRange::CloneParentsBetween(nsIDOMNode 
   NS_IF_ADDREF(*aFarthestAncestor);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsRange::CloneContents(nsIDOMDocumentFragment** aReturn)
 {
-  if (IsDetached())
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
-
   nsresult res;
   nsCOMPtr<nsIDOMNode> commonAncestor;
   res = GetCommonAncestorContainer(getter_AddRefs(commonAncestor));
   if (NS_FAILED(res)) return res;
 
   nsCOMPtr<nsIDOMDocument> document =
     do_QueryInterface(mStartParent->OwnerDoc());
   NS_ASSERTION(document, "CloneContents needs a document to continue.");
@@ -2073,19 +2052,16 @@ nsRange::CloneContents(nsIDOMDocumentFra
   NS_IF_ADDREF(*aReturn);
 
   return NS_OK;
 }
 
 nsresult
 nsRange::CloneRange(nsRange** aReturn) const
 {
-  if(mIsDetached)
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
-
   if (aReturn == 0)
     return NS_ERROR_NULL_POINTER;
 
   nsRefPtr<nsRange> range = new nsRange();
 
   range->SetMaySpanAnonymousSubtrees(mMaySpanAnonymousSubtrees);
 
   range->DoSetRange(mStartParent, mStartOffset, mEndParent, mEndOffset, mRoot);
@@ -2259,19 +2235,16 @@ nsRange::SurroundContents(nsIDOMNode* aN
   // Select aNewParent, and its contents.
 
   return SelectNode(aNewParent);
 }
 
 NS_IMETHODIMP
 nsRange::ToString(nsAString& aReturn)
 { 
-  if(mIsDetached)
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
-
   // clear the string
   aReturn.Truncate();
   
   // If we're unpositioned, return the empty string
   if (!mIsPositioned) {
     return NS_OK;
   }
 
@@ -2353,27 +2326,18 @@ nsRange::ToString(nsAString& aReturn)
   return NS_OK;
 }
 
 
 
 NS_IMETHODIMP
 nsRange::Detach()
 {
-  if(mIsDetached)
-    return NS_ERROR_DOM_INVALID_STATE_ERR;
-
-  if (IsInSelection()) {
-    ::InvalidateAllFrames(GetRegisteredCommonAncestor());
-  }
-
+  // No-op, but still set mIsDetached for telemetry (bug 702948)
   mIsDetached = true;
-
-  DoSetRange(nsnull, 0, nsnull, 0, nsnull);
-  
   return NS_OK;
 }
 
 NS_IMETHODIMP    
 nsRange::CreateContextualFragment(const nsAString& aFragment,
                                   nsIDOMDocumentFragment** aReturn)
 {
   if (mIsPositioned) {
--- a/content/base/src/nsRange.h
+++ b/content/base/src/nsRange.h
@@ -103,21 +103,16 @@ public:
     return mEndOffset;
   }
   
   bool IsPositioned() const
   {
     return mIsPositioned;
   }
 
-  bool IsDetached() const
-  {
-    return mIsDetached;
-  }
-  
   bool Collapsed() const
   {
     return mIsPositioned && mStartParent == mEndParent &&
            mStartOffset == mEndOffset;
   }
 
   void SetMaySpanAnonymousSubtrees(bool aMaySpanAnonymousSubtrees)
   {
@@ -133,17 +128,17 @@ public:
     return mInSelection;
   }
 
   /**
    * Called when the range is added/removed from a Selection.
    */
   void SetInSelection(bool aInSelection)
   {
-    if (mInSelection == aInSelection || mIsDetached) {
+    if (mInSelection == aInSelection) {
       return;
     }
     mInSelection = aInSelection;
     nsINode* commonAncestor = GetCommonAncestor();
     NS_ASSERTION(commonAncestor, "unexpected disconnected nodes");
     if (mInSelection) {
       RegisterCommonAncestor(commonAncestor);
     } else {
@@ -240,17 +235,16 @@ protected:
     {
 #ifdef DEBUG
       mWasInSelection = mRange->IsInSelection();
 #endif
       if (!mRange->IsInSelection() || mIsNested) {
         return;
       }
       mIsNested = true;
-      NS_ASSERTION(!mRange->IsDetached(), "detached range in selection");
       mCommonAncestor = mRange->GetRegisteredCommonAncestor();
     }
     ~AutoInvalidateSelection();
     nsRange* mRange;
     nsRefPtr<nsINode> mCommonAncestor;
 #ifdef DEBUG
     bool mWasInSelection;
 #endif
--- a/content/test/unit/test_range.js
+++ b/content/test/unit/test_range.js
@@ -364,43 +364,16 @@ function run_extract_test() {
         foundEnd = true;
         break;
       }
     } while (walker.nextNode())
     do_check_true(foundEnd);
 
     // Clean up after ourselves.
     walker = null;
-
-    /* Whenever a DOM range is detached, we cannot use any methods on
-       it - including extracting its contents or deleting its contents.  It
-       should throw a NS_ERROR_DOM_INVALID_STATE_ERR exception.
-    */
-    dump("Detached range test\n");
-    var compareFrag = getFragment(baseSource);
-    baseFrag = getFragment(baseSource);
-    baseRange = getRange(baseSource, baseFrag);
-    baseRange.detach();
-    try {
-      var cutFragment = baseRange.extractContents();
-      do_throw("Should have thrown INVALID_STATE_ERR!");
-    } catch (e if (e instanceof C_i.nsIException &&
-                   e.result == INVALID_STATE_ERR)) {
-      // do nothing
-    }
-    do_check_true(compareFrag.isEqualNode(baseFrag));
-
-    try {
-      baseRange.deleteContents();
-      do_throw("Should have thrown INVALID_STATE_ERR!");
-    } catch (e if (e instanceof C_i.nsIException &&
-                   e.result == INVALID_STATE_ERR)) {
-      // do nothing
-    }
-    do_check_true(compareFrag.isEqualNode(baseFrag));
   }
 }
 
 /**
  * Miscellaneous tests not covered above.
  */
 function run_miscellaneous_tests() {
   var filePath = "test_delete_range.xml";
--- a/layout/generic/test/test_bug496275.html
+++ b/layout/generic/test/test_bug496275.html
@@ -73,22 +73,16 @@ function isAt(anchorNode, anchorOffset, 
 
 function run() {
   var sel = window.getSelection();
 
   // If nothing is focused, selection.modify() should silently fail.
   sel.removeAllRanges();
   sel.modify("move", "forward", "character");
 
-  // Similarly, we should silently fail if the selection has been detached.
-  $("c").focus();
-  sel.collapse($("p1"), 0);
-  sel.getRangeAt(0).detach();
-  sel.modify("move", "forward", "character");
-
   // Now focus our first div and put the cursor at the beginning of p1.
   $("c").focus();
   sel.collapse($("p1"), 0);
 
   // We're intentionally inconsistent with the capitalization of "move",
   // "extend", etc. below to test the case-insensitivity of modify().
 
   // If we move backward, selection.modify() shouldn't do anything, since we're