Autocorrection causes ASSERT when replacing alternative string
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Apr 2014 21:35:17 +0000 (21:35 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Apr 2014 21:35:17 +0000 (21:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=131475

Reviewed by Ryosuke Niwa.

In AlternativeTextController::applyAlternativeTextToRange(), we attempt to create
a Range that crosses from outside of a shadow root to inside of one. Instead,
we should keep the Range entirely within the shadow root.

Test: ManualTests/autocorrection/autocorrection-accept-crash.html

* editing/AlternativeTextController.cpp:
(WebCore::AlternativeTextController::applyAlternativeTextToRange):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@167151 268f45cc-cd09-0410-ab3c-d52691b4dbfc

ManualTests/autocorrection/autocorrection-accept-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/AlternativeTextController.cpp

diff --git a/ManualTests/autocorrection/autocorrection-accept-crash.html b/ManualTests/autocorrection/autocorrection-accept-crash.html
new file mode 100644 (file)
index 0000000..28268f5
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body>
+<ol>
+<li>Type "word loadp" in the first box.</li>
+<li>When the suggestion popup appears, click in the second box.</li>
+</ol>
+<p>
+This test triggers a codepath when we are trying to determine a range around
+the new correction word. We were remembering the location for the new word
+by creating a Range from the beginning of the document to the beginning of
+the new word. However, since the word is inside a shadow root, this Range
+would collapse and be meaningless. Further processing using the range would
+trigger ASSERTs and crash.
+</p>
+<p>
+The test is successful if there is no crash.
+</p>
+<input id="t" type="text" spellCheck="true">
+<textarea id="a"></textarea>
+</body>
+</html>
index 8470ef5..4f52439 100644 (file)
@@ -1,3 +1,19 @@
+2014-04-09  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Autocorrection causes ASSERT when replacing alternative string
+        https://bugs.webkit.org/show_bug.cgi?id=131475
+
+        Reviewed by Ryosuke Niwa.
+
+        In AlternativeTextController::applyAlternativeTextToRange(), we attempt to create
+        a Range that crosses from outside of a shadow root to inside of one. Instead,
+        we should keep the Range entirely within the shadow root.
+
+        Test: ManualTests/autocorrection/autocorrection-accept-crash.html
+
+        * editing/AlternativeTextController.cpp:
+        (WebCore::AlternativeTextController::applyAlternativeTextToRange):
+
 2014-04-11  Hans Muller  <hmuller@adobe.com>
 
         [CSS Shapes] shape-outside from image doesn't load properly
index 165c149..4334ba8 100644 (file)
@@ -260,8 +260,8 @@ void AlternativeTextController::applyAlternativeTextToRange(const Range* range,
     if (ec)
         return;
 
-    Position startPositionOfrangeWithAlternative = range->startPosition();
-    correctionStartOffsetInParagraphAsRange->setEnd(startPositionOfrangeWithAlternative.containerNode(), startPositionOfrangeWithAlternative.computeOffsetInContainerNode(), ec);
+    Position startPositionOfRangeWithAlternative = range->startPosition();
+    correctionStartOffsetInParagraphAsRange->setEnd(startPositionOfRangeWithAlternative.containerNode(), startPositionOfRangeWithAlternative.computeOffsetInContainerNode(), ec);
     if (ec)
         return;
 
@@ -270,11 +270,12 @@ void AlternativeTextController::applyAlternativeTextToRange(const Range* range,
 
     // Clone the range, since the caller of this method may want to keep the original range around.
     RefPtr<Range> rangeWithAlternative = range->cloneRange(ec);
-    
-    int paragraphStartIndex = TextIterator::rangeLength(Range::create(*m_frame.document(), m_frame.document(), 0, paragraphRangeContainingCorrection.get()->startContainer(), paragraphRangeContainingCorrection.get()->startOffset()).get());
+
+    ContainerNode& rootNode = paragraphRangeContainingCorrection.get()->startContainer()->treeScope().rootNode();
+    int paragraphStartIndex = TextIterator::rangeLength(Range::create(*rootNode.document(), &rootNode, 0, paragraphRangeContainingCorrection.get()->startContainer(), paragraphRangeContainingCorrection.get()->startOffset()).get());
     applyCommand(SpellingCorrectionCommand::create(rangeWithAlternative, alternative));
     // Recalculate pragraphRangeContainingCorrection, since SpellingCorrectionCommand modified the DOM, such that the original paragraphRangeContainingCorrection is no longer valid. Radar: 10305315 Bugzilla: 89526
-    paragraphRangeContainingCorrection = TextIterator::rangeFromLocationAndLength(m_frame.document(), paragraphStartIndex, correctionStartOffsetInParagraph + alternative.length());
+    paragraphRangeContainingCorrection = TextIterator::rangeFromLocationAndLength(&rootNode, paragraphStartIndex, correctionStartOffsetInParagraph + alternative.length());
     
     setEnd(paragraphRangeContainingCorrection.get(), m_frame.selection().selection().start());
     RefPtr<Range> replacementRange = TextIterator::subrange(paragraphRangeContainingCorrection.get(), correctionStartOffsetInParagraph, alternative.length());