Crash at WebCore::TextIterator::handleTextBox
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Jun 2012 23:34:21 +0000 (23:34 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Jun 2012 23:34:21 +0000 (23:34 +0000)
https://bugs.webkit.org/show_bug.cgi?id=89526
<rdar://problem/10305315>

Patch by Alice Cheng <alice_cheng@apple.com> on 2012-06-26
Reviewed by Darin Adler.

Source/WebCore:

The range used for marking becomes invalid after SpellingCorrectionCommand, due to changes in the DOM made by ReplaceSelectionCommand.
This invalid range caused marking to be incorrect, and Mail.app to crash when iterating through the invalid range.  To fix this,
recalculate the range for marking after SpellingCorrectionCommand.

Test: platform/mac/editing/spelling/autocorrection-blockquote-crash.html

* editing/AlternativeTextController.cpp:
(WebCore::AlternativeTextController::applyAlternativeTextToRange):
* editing/Editor.cpp:  (WebCore::Editor::markAndReplaceFor):
* testing/Internals.cpp:
(WebCore):
(WebCore::Internals::hasAutocorrectedMarker):
* testing/Internals.h: (Internals):
* testing/Internals.idl:

LayoutTests:

* platform/mac/editing/spelling/autocorrection-blockquote-crash-expected.txt: Added.
* platform/mac/editing/spelling/autocorrection-blockquote-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/mac/editing/spelling/autocorrection-blockquote-crash-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/editing/spelling/autocorrection-blockquote-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/AlternativeTextController.cpp
Source/WebCore/editing/Editor.cpp
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index 01a60f1..3421b6e 100644 (file)
@@ -1,3 +1,14 @@
+2012-06-26  Alice Cheng  <alice_cheng@apple.com>
+
+        Crash at WebCore::TextIterator::handleTextBox
+        https://bugs.webkit.org/show_bug.cgi?id=89526
+        <rdar://problem/10305315>
+
+        Reviewed by Darin Adler.
+
+        * platform/mac/editing/spelling/autocorrection-blockquote-crash-expected.txt: Added.
+        * platform/mac/editing/spelling/autocorrection-blockquote-crash.html: Added.
+
 2012-06-26  Ryosuke Niwa  <rniwa@webkit.org>
 
         Convert editing/inserting/font-size-clears-from-typing-style.html to a dump-as-markup test
diff --git a/LayoutTests/platform/mac/editing/spelling/autocorrection-blockquote-crash-expected.txt b/LayoutTests/platform/mac/editing/spelling/autocorrection-blockquote-crash-expected.txt
new file mode 100644 (file)
index 0000000..ad1cf1b
--- /dev/null
@@ -0,0 +1,12 @@
+This test checks that markers are correct when auto correcting in the blockquote. If you type "n" and " ", there should be blue dots under information, but is off by one.
+
+PASS internals.hasAutocorrectedMarker(document, 0, 1) is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
+would this 
+testinformation 
+make a difference?
+
+
diff --git a/LayoutTests/platform/mac/editing/spelling/autocorrection-blockquote-crash.html b/LayoutTests/platform/mac/editing/spelling/autocorrection-blockquote-crash.html
new file mode 100644 (file)
index 0000000..6270fef
--- /dev/null
@@ -0,0 +1,24 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+    <script src="../../../../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description">This test checks that markers are correct when auto correcting in the blockquote. If you type "n" and " ", there should be blue dots under information, but is off by one. <br> Note, this test can fail due to user specific spell checking data. If the user has previously dismissed 'notational' as the correct spelling of 'notationl' several times, the spell checker will not provide 'information' as a suggestion anymore. To fix this, remove all files in ~/Library/Spelling.</p>
+<div id="console"></div>
+
+<div id = "test" contentEditable="true" spellCheck="true"><blockquote type="cite"><font style = "font-family:Arial"><br> would this <b id = "bold"><br></b><b><br></b>make a difference?<span><br></span><span><br></span></font></blockquote></div>
+
+<script language="javascript">
+    // Insert some text with a typographical error in it, so autocorrection occurs.
+    window.getSelection().setPosition(document.getElementById("bold"), 1);
+    document.execCommand("InsertText", false, "test infomatio");
+    eventSender.keyDown('n');
+    eventSender.keyDown(' ');
+    
+    if(window.internals)
+        shouldBeTrue('internals.hasAutocorrectedMarker(document, 0, 1)');
+</script>
+<script src="../../../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index 11f789a..eaea708 100755 (executable)
@@ -1,3 +1,26 @@
+2012-06-26  Alice Cheng  <alice_cheng@apple.com>
+
+        Crash at WebCore::TextIterator::handleTextBox
+        https://bugs.webkit.org/show_bug.cgi?id=89526
+        <rdar://problem/10305315>
+
+        Reviewed by Darin Adler.
+
+        The range used for marking becomes invalid after SpellingCorrectionCommand, due to changes in the DOM made by ReplaceSelectionCommand. 
+        This invalid range caused marking to be incorrect, and Mail.app to crash when iterating through the invalid range.  To fix this,
+        recalculate the range for marking after SpellingCorrectionCommand.
+
+        Test: platform/mac/editing/spelling/autocorrection-blockquote-crash.html
+
+        * editing/AlternativeTextController.cpp:
+        (WebCore::AlternativeTextController::applyAlternativeTextToRange):
+        * editing/Editor.cpp:  (WebCore::Editor::markAndReplaceFor):
+        * testing/Internals.cpp:
+        (WebCore):
+        (WebCore::Internals::hasAutocorrectedMarker):
+        * testing/Internals.h: (Internals):
+        * testing/Internals.idl:
+
 2012-06-26  Tom Sepez  <tsepez@chromium.org>
 
         [chromium] HTML5 audio/video tags - loading http content from https page doesn't trigger warning.
index b199af1..d4a0e43 100644 (file)
@@ -273,7 +273,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());
     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());
+    
     setEnd(paragraphRangeContainingCorrection.get(), m_frame->selection()->selection().start());
     RefPtr<Range> replacementRange = TextIterator::subrange(paragraphRangeContainingCorrection.get(), correctionStartOffsetInParagraph, alternative.length());
     String newText = plainText(replacementRange.get());
index 44e6a0d..9018a89 100644 (file)
@@ -2119,8 +2119,13 @@ void Editor::markAndReplaceFor(PassRefPtr<SpellCheckRequest> request, const Vect
                 if (canEditRichly())
                     applyCommand(CreateLinkCommand::create(m_frame->document(), result->replacement));
             } else if (canEdit() && shouldInsertText(result->replacement, rangeToReplace.get(), EditorInsertActionTyped)) {
+                int paragraphStartIndex = TextIterator::rangeLength(Range::create(m_frame->document(), m_frame->document(), 0, paragraph.paragraphRange()->startContainer(), paragraph.paragraphRange()->startOffset()).get());
+                int paragraphLength = TextIterator::rangeLength(paragraph.paragraphRange().get());
                 applyCommand(SpellingCorrectionCommand::create(rangeToReplace, result->replacement));
-
+                // Recalculate newParagraphRange, since SpellingCorrectionCommand modifies the DOM, such that the original paragraph range is no longer valid. Radar: 10305315 Bugzilla: 89526
+                RefPtr<Range> newParagraphRange = TextIterator::rangeFromLocationAndLength(m_frame->document(), paragraphStartIndex, paragraphLength+replacementLength-resultLength);
+                paragraph = TextCheckingParagraph(TextIterator::subrange(newParagraphRange.get(), resultLocation, replacementLength), newParagraphRange);
+                
                 if (AXObjectCache::accessibilityEnabled()) {
                     if (Element* root = m_frame->selection()->selection().rootEditableElement())
                         m_frame->document()->axObjectCache()->postNotification(root->renderer(), AXObjectCache::AXAutocorrectionOccured, true);
index 44ef4d5..f8ca0c6 100644 (file)
@@ -1028,6 +1028,14 @@ bool Internals::hasSpellingMarker(Document* document, int from, int length, Exce
 
     return document->frame()->editor()->selectionStartHasMarkerFor(DocumentMarker::Spelling, from, length);
 }
+    
+bool Internals::hasAutocorrectedMarker(Document* document, int from, int length, ExceptionCode&)
+{
+    if (!document || !document->frame())
+        return 0;
+    
+    return document->frame()->editor()->selectionStartHasMarkerFor(DocumentMarker::Autocorrected, from, length);
+}
 
 #if ENABLE(INSPECTOR)
 unsigned Internals::numberOfLiveNodes() const
index cac206b..160e001 100644 (file)
@@ -158,6 +158,7 @@ public:
 
     bool hasSpellingMarker(Document*, int from, int length, ExceptionCode&);
     bool hasGrammarMarker(Document*, int from, int length, ExceptionCode&);
+    bool hasAutocorrectedMarker(Document*, int from, int length, ExceptionCode&);
 
     unsigned numberOfScrollableAreas(Document*, ExceptionCode&);
 
index b3a249f..230984c 100644 (file)
@@ -132,6 +132,7 @@ module window {
 
         boolean hasSpellingMarker(in Document document, in long from, in long length) raises (DOMException);
         boolean hasGrammarMarker(in Document document, in long from, in long length) raises (DOMException);
+        boolean hasAutocorrectedMarker(in Document document, in long from, in long length) raises (DOMException);
 
         unsigned long numberOfScrollableAreas(in Document document) raises (DOMException);