| author | William Chen <wchen@mozilla.com> |
| Tue, 08 Jan 2013 15:45:10 -0800 | |
| changeset 118849 | 485daaddbbb2c0b6ed7770b05c7c8394ea46f398 |
| parent 118848 | 829470a09f03395c6e5dbeed9cb4b225af7f6a95 |
| child 118850 | 861fb57b93d83f8ce5916ad16a61ebc37e6669d9 |
| push id | 24180 |
| push user | emorley@mozilla.com |
| push date | Tue, 15 Jan 2013 22:58:27 +0000 |
| treeherder | mozilla-central@72e34ce7fd92 [default view] [failures only] |
| perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
| reviewers | ehsan |
| bugs | 827426 |
| milestone | 21.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
|
--- 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>