Bug 803924 - Crash with range, splitText. r=smaug
authorMats Palmgren <matspal@gmail.com>
Tue, 20 Nov 2012 21:14:15 +0100
changeset 113811 88bad356f5a4d3dfc4888305e3670ec80e576a4c
parent 113810 da575e9f17c45bfc3aa25e0e43854214005dc57b
child 113812 6ee5de769c4ac0e067acc84456f945690d092fa0
push id23890
push userryanvm@gmail.com
push dateWed, 21 Nov 2012 02:43:32 +0000
treeherdermozilla-central@4f19e7fd8bea [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs803924
milestone20.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 803924 - Crash with range, splitText. r=smaug
content/base/src/nsRange.cpp
content/base/src/nsRange.h
content/base/test/Makefile.in
content/base/test/test_textnode_split_in_selection.html
dom/imptests/failures/webapps/DOMCore/tests/approved/Makefile.in
dom/imptests/failures/webapps/DOMCore/tests/approved/test_Range-mutations.html.json
--- a/content/base/src/nsRange.cpp
+++ b/content/base/src/nsRange.cpp
@@ -370,24 +370,55 @@ nsRange::UnregisterCommonAncestor(nsINod
  * nsIMutationObserver implementation
  ******************************************************/
 
 void
 nsRange::CharacterDataChanged(nsIDocument* aDocument,
                               nsIContent* aContent,
                               CharacterDataChangeInfo* aInfo)
 {
+  MOZ_ASSERT(mAssertNextInsertOrAppendIndex == -1,
+             "splitText failed to notify insert/append?");
   NS_ASSERTION(mIsPositioned, "shouldn't be notified if not positioned");
 
   nsINode* newRoot = nullptr;
   nsINode* newStartNode = nullptr;
   nsINode* newEndNode = nullptr;
   uint32_t newStartOffset = 0;
   uint32_t newEndOffset = 0;
 
+  if (aInfo->mDetails &&
+      aInfo->mDetails->mType == CharacterDataChangeInfo::Details::eSplit) {
+    // If the splitted text node is immediately before a range boundary point
+    // that refers to a child index (i.e. its parent is the boundary container)
+    // then we need to increment the corresponding offset to account for the new
+    // text node that will be inserted.  If so, we need to prevent the next
+    // ContentInserted or ContentAppended for this range from incrementing it
+    // again (when the new text node is notified).
+    nsINode* parentNode = aContent->GetParentNode();
+    int32_t index = -1;
+    if (parentNode == mEndParent && mEndOffset > 0 &&
+        (index = parentNode->IndexOf(aContent)) + 1 == mEndOffset) {
+      ++mEndOffset;
+      mEndOffsetWasIncremented = true;
+    }
+    if (parentNode == mStartParent && mStartOffset > 0 &&
+        (index != -1 ? index : parentNode->IndexOf(aContent)) + 1 == mStartOffset) {
+      ++mStartOffset;
+      mStartOffsetWasIncremented = true;
+    }
+#ifdef DEBUG
+    if (mStartOffsetWasIncremented || mEndOffsetWasIncremented) {
+      mAssertNextInsertOrAppendIndex =
+        (mStartOffsetWasIncremented ? mStartOffset : mEndOffset) - 1;
+      mAssertNextInsertOrAppendNode = aInfo->mDetails->mNextSibling;
+    }
+#endif
+  }
+
   // If the changed node contains our start boundary and the change starts
   // before the boundary we'll need to adjust the offset.
   if (aContent == mStartParent &&
       aInfo->mChangeStart < static_cast<uint32_t>(mStartOffset)) {
     if (aInfo->mDetails) {
       // splitText(), aInfo->mDetails->mNextSibling is the new text node
       NS_ASSERTION(aInfo->mDetails->mType ==
                    CharacterDataChangeInfo::Details::eSplit,
@@ -499,50 +530,77 @@ nsRange::ContentAppended(nsIDocument* aD
     while (child) {
       if (!child->IsDescendantOfCommonAncestorForRangeInSelection()) {
         MarkDescendants(child);
         child->SetDescendantOfCommonAncestorForRangeInSelection();
       }
       child = child->GetNextSibling();
     }
   }
+
+  if (mStartOffsetWasIncremented || mEndOffsetWasIncremented) {
+    MOZ_ASSERT(mAssertNextInsertOrAppendIndex == aNewIndexInContainer);
+    MOZ_ASSERT(mAssertNextInsertOrAppendNode == aFirstNewContent);
+    MOZ_ASSERT(aFirstNewContent->IsNodeOfType(nsINode::eTEXT));
+    mStartOffsetWasIncremented = mEndOffsetWasIncremented = false;
+#ifdef DEBUG
+    mAssertNextInsertOrAppendIndex = -1;
+    mAssertNextInsertOrAppendNode = nullptr;
+#endif
+  }
 }
 
 void
 nsRange::ContentInserted(nsIDocument* aDocument,
                          nsIContent* aContainer,
                          nsIContent* aChild,
                          int32_t aIndexInContainer)
 {
   NS_ASSERTION(mIsPositioned, "shouldn't be notified if not positioned");
 
   nsINode* container = NODE_FROM(aContainer, aDocument);
 
   // Adjust position if a sibling was inserted.
-  if (container == mStartParent && aIndexInContainer < mStartOffset) {
+  if (container == mStartParent && aIndexInContainer < mStartOffset &&
+      !mStartOffsetWasIncremented) {
     ++mStartOffset;
   }
-  if (container == mEndParent && aIndexInContainer < mEndOffset) {
+  if (container == mEndParent && aIndexInContainer < mEndOffset &&
+      !mEndOffsetWasIncremented) {
     ++mEndOffset;
   }
   if (container->IsSelectionDescendant() &&
       !aChild->IsDescendantOfCommonAncestorForRangeInSelection()) {
     MarkDescendants(aChild);
     aChild->SetDescendantOfCommonAncestorForRangeInSelection();
   }
+
+  if (mStartOffsetWasIncremented || mEndOffsetWasIncremented) {
+    MOZ_ASSERT(mAssertNextInsertOrAppendIndex == aIndexInContainer);
+    MOZ_ASSERT(mAssertNextInsertOrAppendNode == aChild);
+    MOZ_ASSERT(aChild->IsNodeOfType(nsINode::eTEXT));
+    mStartOffsetWasIncremented = mEndOffsetWasIncremented = false;
+#ifdef DEBUG
+    mAssertNextInsertOrAppendIndex = -1;
+    mAssertNextInsertOrAppendNode = nullptr;
+#endif
+  }
 }
 
 void
 nsRange::ContentRemoved(nsIDocument* aDocument,
                         nsIContent* aContainer,
                         nsIContent* aChild,
                         int32_t aIndexInContainer,
                         nsIContent* aPreviousSibling)
 {
   NS_ASSERTION(mIsPositioned, "shouldn't be notified if not positioned");
+  MOZ_ASSERT(!mStartOffsetWasIncremented && !mEndOffsetWasIncremented &&
+             mAssertNextInsertOrAppendIndex == -1,
+             "splitText failed to notify insert/append?");
 
   nsINode* container = NODE_FROM(aContainer, aDocument);
   bool gravitateStart = false;
   bool gravitateEnd = false;
 
   // Adjust position if a sibling was removed...
   if (container == mStartParent) {
     if (aIndexInContainer < mStartOffset) {
@@ -576,16 +634,19 @@ nsRange::ContentRemoved(nsIDocument* aDo
     aChild->ClearDescendantOfCommonAncestorForRangeInSelection();
     UnmarkDescendants(aChild);
   }
 }
 
 void
 nsRange::ParentChainChanged(nsIContent *aContent)
 {
+  MOZ_ASSERT(!mStartOffsetWasIncremented && !mEndOffsetWasIncremented &&
+             mAssertNextInsertOrAppendIndex == -1,
+             "splitText failed to notify insert/append?");
   NS_ASSERTION(mRoot == aContent, "Wrong ParentChainChanged notification?");
   nsINode* newRoot = IsValidBoundary(mStartParent);
   NS_ASSERTION(newRoot, "No valid boundary or root found!");
   NS_ASSERTION(newRoot == IsValidBoundary(mEndParent),
                "Start parent and end parent give different root!");
   // This is safe without holding a strong ref to self as long as the change
   // of mRoot is the last thing in DoSetRange.
   DoSetRange(mStartParent, mStartOffset, mEndParent, mEndOffset, newRoot);
--- a/content/base/src/nsRange.h
+++ b/content/base/src/nsRange.h
@@ -25,16 +25,22 @@ public:
   nsRange()
     : mRoot(nullptr)
     , mStartOffset(0)
     , mEndOffset(0)
     , mIsPositioned(false)
     , mIsDetached(false)
     , mMaySpanAnonymousSubtrees(false)
     , mInSelection(false)
+    , mStartOffsetWasIncremented(false)
+    , mEndOffsetWasIncremented(false)
+#ifdef DEBUG
+    , mAssertNextInsertOrAppendIndex(-1)
+    , mAssertNextInsertOrAppendNode(nullptr)
+#endif
   {}
   virtual ~nsRange();
 
   static nsresult CreateRange(nsIDOMNode* aStartParent, int32_t aStartOffset,
                               nsIDOMNode* aEndParent, int32_t aEndOffset,
                               nsRange** aRange);
   static nsresult CreateRange(nsIDOMNode* aStartParent, int32_t aStartOffset,
                               nsIDOMNode* aEndParent, int32_t aEndOffset,
@@ -224,11 +230,17 @@ protected:
   nsCOMPtr<nsINode> mEndParent;
   int32_t mStartOffset;
   int32_t mEndOffset;
 
   bool mIsPositioned;
   bool mIsDetached;
   bool mMaySpanAnonymousSubtrees;
   bool mInSelection;
+  bool mStartOffsetWasIncremented;
+  bool mEndOffsetWasIncremented;
+#ifdef DEBUG
+  int32_t  mAssertNextInsertOrAppendIndex;
+  nsINode* mAssertNextInsertOrAppendNode;
+#endif
 };
 
 #endif /* nsRange_h___ */
--- a/content/base/test/Makefile.in
+++ b/content/base/test/Makefile.in
@@ -588,16 +588,17 @@ MOCHITEST_FILES_B = \
     test_mixed_content_blocker_bug803225.html \
     file_mixed_content_main_bug803225.html \
     file_mixed_content_main_bug803225_websocket_wsh.py \
     bug803225_test_mailto.html \
 		test_bug789856.html \
 		file_bug804395.jar \
 		test_bug804395.html \
 		test_bug809003.html \
+		test_textnode_split_in_selection.html \
 		$(NULL)
 
 # OOP tests don't work on Windows (bug 763081) or native-fennec
 # (see Bug 774939)
 ifneq ($(OS_ARCH),WINNT)
 ifndef MOZ_ANDROID_OMTC
 MOCHITEST_FILES_B += \
 		test_messagemanager_assertpermission.html \
new file mode 100644
--- /dev/null
+++ b/content/base/test/test_textnode_split_in_selection.html
@@ -0,0 +1,221 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=803924
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 803924</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=803924">Mozilla Bug 803924</a>
+<p id="display"></p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 803924 **/
+
+var sel = document.getSelection();
+var flush = true;
+var dry = true;
+var run = "";
+var empty_range;
+var empty_first_text_range;
+var empty_last_text_range;
+var full_range;
+
+function check(range, expected, test)
+{
+  is(""+range, expected, test);
+  is(""+empty_range, "", "empty range test after: "+test);
+  is(""+empty_first_text_range, "", "empty first text range test after: "+test);
+  if (empty_last_text_range) is(""+empty_last_text_range, "", "empty last text range test after: "+test);
+  is(""+full_range, full_range.startContainer.textContent, "full range test after: "+test);
+}
+
+function newDiv()
+{
+  var div = document.createElement('div');
+  for (var i = 0; i < arguments.length; ++i) {
+    div.appendChild(document.createTextNode(arguments[i]));
+  }
+  document.body.appendChild(div)
+  empty_range = document.createRange();
+  empty_range.setStart(div,0);
+  empty_range.setEnd(div,0);
+  var firstTextNode = div.childNodes[0];
+  var lastTextNode = div.childNodes[div.childNodes.length - 1];
+  empty_first_text_range = document.createRange();
+  empty_first_text_range.setStart(firstTextNode,0);
+  empty_first_text_range.setEnd(firstTextNode,0);
+  empty_last_text_range = null;
+  if (firstTextNode != lastTextNode) {
+    empty_last_text_range = document.createRange();
+    empty_last_text_range.setStart(lastTextNode,0);
+    empty_last_text_range.setEnd(lastTextNode,0);
+  }
+  full_range = document.createRange();
+  full_range.setStart(div,0);
+  full_range.setEnd(div,div.childNodes.length);
+  return div;
+}
+
+function selEnd(div,child,index,split,s)
+{
+  var start = div.childNodes[child];
+  var r = document.createRange();
+  sel.addRange(r);
+  r.setStart(start, index);
+  r.setEnd(div, div.childNodes.length);
+  if (!dry) start.splitText(split);
+  check(r,s,run+" selEnd "+child+","+index+","+split);
+}
+
+function selStart(div,child,index,split,s)
+{
+  if (flush) document.body.getClientRects();
+  var start = div.childNodes[child];
+  var r = document.createRange();
+  sel.addRange(r);
+  r.setStart(div, 0);
+  r.setEnd(start, index);
+  if (!dry) start.splitText(split);
+  check(r,s,run+" selStart "+child+","+index+","+split);
+}
+
+function selMiddleStart(div,child,index,split,s)
+{
+  if (flush) document.body.getClientRects();
+  var start = div.childNodes[child];
+  var r = document.createRange();
+  sel.addRange(r);
+  r.setStart(div, 1);
+  r.setEnd(start, index);
+  if (!dry) start.splitText(split);
+  check(r,s,run+" selMiddleStart "+child+","+index+","+split);
+}
+
+function selMiddleEnd(div,child,index,split,s)
+{
+  if (flush) document.body.getClientRects();
+  var start = div.childNodes[child];
+  var r = document.createRange();
+  sel.addRange(r);
+  r.setStart(start, index);
+  r.setEnd(div, 2);
+  if (!dry) start.splitText(split);
+  check(r,s,run+" selMiddleEnd "+child+","+index+","+split);
+}
+
+function splitBefore(div,child,index,split,s)
+{
+  if (flush) document.body.getClientRects();
+  var start = div.childNodes[child];
+  var r = document.createRange();
+  sel.addRange(r);
+  r.setStart(div, 1);
+  r.setEnd(start, index);
+  if (!dry) div.childNodes[0].splitText(split);
+  check(r,s,run+" splitBefore "+child+","+index+","+split);
+}
+
+function runTests(s)
+{
+  run = s+":";
+  selEnd(newDiv('111'), 0,0,0,'111');
+  selEnd(newDiv('111'), 0,0,1,'111');
+  selEnd(newDiv('111'), 0,0,3,'111');
+  selEnd(newDiv(''), 0,0,0,'');
+  selEnd(newDiv('111'), 0,1,0,'11');
+  selEnd(newDiv('111'), 0,2,1,'1');
+  selEnd(newDiv('111'), 0,1,3,'11');
+  selEnd(newDiv('111','222'), 0,1,0,'11222');
+  selEnd(newDiv('111','222'), 0,2,1,'1222');
+  selEnd(newDiv('111','222'), 0,1,3,'11222');
+  selEnd(newDiv('111','222'), 1,1,0,'22');
+  selEnd(newDiv('111','222'), 1,2,1,'2');
+  selEnd(newDiv('','222'), 1,1,1,'22');
+  selEnd(newDiv('','222'), 0,0,0,'222');
+  selEnd(newDiv('111',''), 0,1,0,'11');
+  selEnd(newDiv('111','222'), 1,1,3,'22');
+  selEnd(newDiv('111','222','333'), 1,1,0,'22333');
+  selEnd(newDiv('111','222','333'), 1,2,1,'2333');
+  selEnd(newDiv('111','222','333'), 1,1,3,'22333');
+  selEnd(newDiv('111','222',''), 1,1,3,'22');
+  selEnd(newDiv('111','','333'), 0,1,3,'11333');
+
+  selStart(newDiv('111'), 0,0,0,'');
+  selStart(newDiv('111'), 0,0,1,'');
+  selStart(newDiv('111'), 0,0,3,'');
+  selStart(newDiv('111'), 0,1,0,'1');
+  selStart(newDiv('111'), 0,2,1,'11');
+  selStart(newDiv('111'), 0,1,3,'1');
+  selStart(newDiv(''), 0,0,0,'');
+  selStart(newDiv('111','222'), 0,1,0,'1');
+  selStart(newDiv('111','222'), 0,2,1,'11');
+  selStart(newDiv('111','222'), 0,1,3,'1');
+  selStart(newDiv('111','222'), 1,1,0,'1112');
+  selStart(newDiv('111','222'), 1,2,1,'11122');
+  selStart(newDiv('111','222'), 1,1,3,'1112');
+  selStart(newDiv('','222'), 1,1,2,'2');
+  selStart(newDiv('','222'), 0,0,0,'');
+  selStart(newDiv('111',''), 1,0,0,'111');
+  selStart(newDiv('111','222','333'), 1,1,0,'1112');
+  selStart(newDiv('111','222','333'), 1,2,1,'11122');
+  selStart(newDiv('111','222','333'), 1,1,3,'1112');
+  selStart(newDiv('111','','333'), 1,0,0,'111');
+  selStart(newDiv('111','222',''), 1,1,3,'1112');
+
+  selMiddleStart(newDiv('111','222','333'), 1,1,0,'2');
+  selMiddleStart(newDiv('111','222','333'), 1,2,1,'22');
+  selMiddleStart(newDiv('111','222','333'), 1,1,3,'2');
+  selMiddleStart(newDiv('111','222','333'), 2,1,0,'2223');
+  selMiddleStart(newDiv('111','222','333'), 2,2,1,'22233');
+  selMiddleStart(newDiv('111','222','333'), 2,1,3,'2223');
+  selMiddleStart(newDiv('111','','333'), 2,1,2,'3');
+  selMiddleStart(newDiv('111','','333'), 1,0,0,'');
+
+  selMiddleEnd(newDiv('111','222','333'), 0,1,0,'11222');
+  selMiddleEnd(newDiv('111','222','333'), 0,2,1,'1222');
+  selMiddleEnd(newDiv('111','222','333'), 0,1,3,'11222');
+  selMiddleEnd(newDiv('111','222','333'), 1,1,0,'22');
+  selMiddleEnd(newDiv('111','222','333'), 1,2,1,'2');
+  selMiddleEnd(newDiv('111','222','333'), 1,1,3,'22');
+  selMiddleEnd(newDiv('111','','333'), 0,1,2,'11');
+  selMiddleEnd(newDiv('111','','333'), 0,1,3,'11');
+  selMiddleEnd(newDiv('111','','333'), 1,0,0,'');
+
+  splitBefore(newDiv('111','222','333'), 1,1,0,'2');
+  splitBefore(newDiv('111','222','333'), 1,2,1,'22');
+  splitBefore(newDiv('111','222','333'), 1,1,3,'2');
+  splitBefore(newDiv('111','222','333'), 2,1,0,'2223');
+  splitBefore(newDiv('111','222','333'), 2,2,1,'22233');
+  splitBefore(newDiv('111','222','333'), 2,1,3,'2223');
+  splitBefore(newDiv('','222','333'), 1,1,0,'2');
+  splitBefore(newDiv('','','333'), 1,0,0,'');
+  splitBefore(newDiv('','222',''), 2,0,0,'222');
+  splitBefore(newDiv('111','','333'), 2,1,2,'3');
+}
+
+function boom()
+{
+  runTests("dry run");  // this is to verify the result strings without splitText()
+  dry = false;
+  flush = false;
+  runTests("no flush");
+  flush = true;
+  runTests("flush");
+}
+
+boom();
+
+
+</script>
+</pre>
+</body>
+</html>
--- a/dom/imptests/failures/webapps/DOMCore/tests/approved/Makefile.in
+++ b/dom/imptests/failures/webapps/DOMCore/tests/approved/Makefile.in
@@ -18,13 +18,14 @@ MOCHITEST_FILES := \
   test_Range-collapse.html.json \
   test_Range-commonAncestorContainer.html.json \
   test_Range-compareBoundaryPoints.html.json \
   test_Range-comparePoint.html.json \
   test_Range-deleteContents.html.json \
   test_Range-extractContents.html.json \
   test_Range-intersectsNode.html.json \
   test_Range-isPointInRange.html.json \
+  test_Range-mutations.html.json \
   test_Range-set.html.json \
   test_interfaces.html.json \
   $(NULL)
 
 include $(topsrcdir)/config/rules.mk
new file mode 100644
--- /dev/null
+++ b/dom/imptests/failures/webapps/DOMCore/tests/approved/test_Range-mutations.html.json
@@ -0,0 +1,12 @@
+{
+  "paras[0].firstChild.splitText(1), with unselected range on paras[0] from 0 to 1": true,
+  "paras[0].firstChild.splitText(1), with selected range on paras[0] from 0 to 1": true,
+  "paras[0].firstChild.splitText(1), with unselected range collapsed at (paras[0], 1)": true,
+  "paras[0].firstChild.splitText(1), with selected range collapsed at (paras[0], 1)": true,
+  "paras[0].firstChild.splitText(1), with unselected range from (paras[0].firstChild, 1) to (paras[0], 1)": true,
+  "paras[0].firstChild.splitText(1), with selected range from (paras[0].firstChild, 1) to (paras[0], 1)": true,
+  "paras[0].firstChild.splitText(2), with unselected range from (paras[0].firstChild, 1) to (paras[0], 1)": true,
+  "paras[0].firstChild.splitText(2), with selected range from (paras[0].firstChild, 1) to (paras[0], 1)": true,
+  "paras[0].firstChild.splitText(3), with unselected range from (paras[0].firstChild, 1) to (paras[0], 1)": true,
+  "paras[0].firstChild.splitText(3), with selected range from (paras[0].firstChild, 1) to (paras[0], 1)": true
+}