Bug 827426 - Added checks for external mutations to text fragments managed by an UndoManager. r=ehsan
authorWilliam Chen <wchen@mozilla.com>
Tue, 08 Jan 2013 15:45:10 -0800
changeset 118849 485daaddbbb2c0b6ed7770b05c7c8394ea46f398
parent 118848 829470a09f03395c6e5dbeed9cb4b225af7f6a95
child 118850 861fb57b93d83f8ce5916ad16a61ebc37e6669d9
push id24180
push useremorley@mozilla.com
push dateTue, 15 Jan 2013 22:58:27 +0000
treeherdermozilla-central@72e34ce7fd92 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs827426
milestone21.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 827426 - Added checks for external mutations to text fragments managed by an UndoManager. r=ehsan
content/html/content/src/UndoManager.cpp
content/html/content/test/Makefile.in
content/html/content/test/test_bug827426.html
--- a/content/html/content/src/UndoManager.cpp
+++ b/content/html/content/src/UndoManager.cpp
@@ -247,21 +247,29 @@ UndoTextChanged::RedoTransaction()
   nsAutoString text;
   mContent->AppendTextTo(text);
 
   if (text.Length() < mChange.mChangeStart) {
     return NS_OK;
   }
 
   if (mChange.mAppend) {
-    mContent->AppendText(mRedoValue.get(), mRedoValue.Length(), true);
+    // Text length should match the change start unless there was a
+    // mutation exterior to the UndoManager in which case we do nothing.
+    if (text.Length() == mChange.mChangeStart) {
+      mContent->AppendText(mRedoValue.get(), mRedoValue.Length(), true);
+    }
   } else {
     int32_t numReplaced = mChange.mChangeEnd - mChange.mChangeStart;
-    text.Replace(mChange.mChangeStart, numReplaced, mRedoValue);
-    mContent->SetText(text, true);
+    // The length of the text should be at least as long as the replacement
+    // offset + replaced length, otherwise there was an external mutation.
+    if (mChange.mChangeStart + numReplaced <= text.Length()) {
+      text.Replace(mChange.mChangeStart, numReplaced, mRedoValue);
+      mContent->SetText(text, true);
+    }
   }
 
   return NS_OK;
 }
 
 nsresult
 UndoTextChanged::UndoTransaction()
 {
@@ -270,30 +278,43 @@ UndoTextChanged::UndoTransaction()
   nsAutoString text;
   mContent->AppendTextTo(text);
 
   if (text.Length() < mChange.mChangeStart) {
     return NS_OK;
   }
 
   if (mChange.mAppend) {
-    text.Truncate(text.Length() - mRedoValue.Length());
+    // The text should at least as long as the redo value in the case
+    // of an append, otherwise there was an external mutation.
+    if (mRedoValue.Length() <= text.Length()) {
+      text.Truncate(text.Length() - mRedoValue.Length());
+    }
   } else {
-    text.Replace(mChange.mChangeStart, mRedoValue.Length(), mUndoValue);
+    // The length of the text should be at least as long as the replacement
+    // offset + replacement length, otherwise there was an external mutation.
+    if (mChange.mChangeStart + mChange.mReplaceLength <= text.Length()) {
+      text.Replace(mChange.mChangeStart, mChange.mReplaceLength, mUndoValue);
+    }
   }
   mContent->SetText(text, true);
 
   return NS_OK;
 }
 
 void
 UndoTextChanged::SaveRedoState()
 {
   const nsTextFragment* text = mContent->GetText();
-  text->AppendTo(mRedoValue, mChange.mChangeStart, mChange.mReplaceLength);
+  mRedoValue.Truncate();
+  // The length of the text should be at least as long as the replacement
+  // offset + replacement length, otherwise there was an external mutation.
+  if (mChange.mChangeStart + mChange.mReplaceLength <= text->GetLength()) {
+    text->AppendTo(mRedoValue, mChange.mChangeStart, mChange.mReplaceLength);
+  }
 }
 
 /////////////////////////////////////////////////
 // UndoContentAppend
 /////////////////////////////////////////////////
 
 /**
  * Transaction to handle appending content to a nsIContent.
--- a/content/html/content/test/Makefile.in
+++ b/content/html/content/test/Makefile.in
@@ -267,16 +267,17 @@ MOCHITEST_FILES = \
 		test_input_file_picker.html \
 		test_bug763626.html \
 		test_bug780993.html \
 		test_bug786564.html \
 		test_bug797113.html \
 		test_bug787134.html \
 		test_bug803677.html \
 		test_bug827126.html \
+		test_bug827426.html \
 		test_iframe_sandbox_inheritance.html \
 		file_iframe_sandbox_a_if1.html \
 		file_iframe_sandbox_a_if2.html \
 		file_iframe_sandbox_a_if3.html \
 		file_iframe_sandbox_a_if4.html \
 		file_iframe_sandbox_a_if5.html \
 		file_iframe_sandbox_a_if6.html \
 		file_iframe_sandbox_a_if7.html \
new file mode 100644
--- /dev/null
+++ b/content/html/content/test/test_bug827426.html
@@ -0,0 +1,48 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=827426
+-->
+<head>
+  <title>Test for Bug 827426</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <script type="text/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=827426">Mozilla Bug 827426</a>
+<p id="display"></p>
+<div id="content">foo</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Bug 827426 **/
+
+var elem = document.getElementById("content");
+
+var undoManager = document.undoManager;
+var transaction = {
+  executeAutomatic: function() {
+    elem.childNodes[0].appendData("bar");
+  },
+};
+
+// Perform transaction
+undoManager.transact(transaction, false);
+
+// Make mutation external to UndoManager.
+elem.childNodes[0].deleteData(0, 6);
+
+// Call undo/redo a few times and make sure nothing bad happens.
+undoManager.undo();
+undoManager.redo();
+undoManager.undo();
+undoManager.redo();
+undoManager.undo();
+
+ok(true, "Nothing bad happened with external mutation.");
+
+</script>
+</pre>
+</body>
+</html>