Title: [263499] trunk
Revision
263499
Author
wenson_hs...@apple.com
Date
2020-06-24 21:42:11 -0700 (Wed, 24 Jun 2020)

Log Message

Running spellcheck on https://developer.apple.com/forums/thread/650317 hangs the web process
https://bugs.webkit.org/show_bug.cgi?id=213585
<rdar://problem/64681632>

Reviewed by Simon Fraser.

Source/WebCore:

In `TextCheckingHelper::findFirstMisspellingOrBadGrammar`, we attempt to find the first misspelled piece of text
by iterating over the current paragraph range; when we exhaust the current paragraph range, we try to move on to
the next paragraph range by setting the current paragraph range to the start and end of the next paragraph,
using `startOfNextParagraph`.

However, `startOfNextParagraph` may return the null position in some cases (as seen in this bug, and the
associated layout test). This logic isn't robust against this possibility, and will end up looping infinitely in
the while loop, since `setStart` and `setEnd` are no-ops when given a null position, and so the current
paragraph remains the same.

Make this logic for finding the next misspelled word more robust by allowing us to bail out of the while loop in
the case where setStart or setEnd failed to advance the paragraph range (e.g. due to a null position, or DOM
exception).

Test: editing/mac/spelling/advance-to-next-misspelling.html

* editing/TextCheckingHelper.cpp:
(WebCore::TextCheckingHelper::findFirstMisspellingOrBadGrammar):
* testing/Internals.cpp:
(WebCore::Internals::advanceToNextMisspelling):

Add an internal testing hook to advance to the next misspelling.

* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

Add a test that attempts to advance to the next misspelling, and verifies that the misspelled word is selected.

* editing/mac/spelling/advance-to-next-misspelling-expected.txt: Added.
* editing/mac/spelling/advance-to-next-misspelling.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (263498 => 263499)


--- trunk/LayoutTests/ChangeLog	2020-06-25 04:25:05 UTC (rev 263498)
+++ trunk/LayoutTests/ChangeLog	2020-06-25 04:42:11 UTC (rev 263499)
@@ -1,3 +1,16 @@
+2020-06-24  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Running spellcheck on https://developer.apple.com/forums/thread/650317 hangs the web process
+        https://bugs.webkit.org/show_bug.cgi?id=213585
+        <rdar://problem/64681632>
+
+        Reviewed by Simon Fraser.
+
+        Add a test that attempts to advance to the next misspelling, and verifies that the misspelled word is selected.
+
+        * editing/mac/spelling/advance-to-next-misspelling-expected.txt: Added.
+        * editing/mac/spelling/advance-to-next-misspelling.html: Added.
+
 2020-06-24  Clark Wang  <clark_w...@apple.com>
 
         Removed unrestricted keyword from attributes in PannerNode

Added: trunk/LayoutTests/editing/mac/spelling/advance-to-next-misspelling-expected.txt (0 => 263499)


--- trunk/LayoutTests/editing/mac/spelling/advance-to-next-misspelling-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/mac/spelling/advance-to-next-misspelling-expected.txt	2020-06-25 04:42:11 UTC (rev 263499)
@@ -0,0 +1,5 @@
+Advancing to the next misspelling (cmd+; on macOS) should select the word 'mispelled'.
+| <p>
+|   id="start"
+|   <br>
+|   "<#selection-anchor>mispelled<#selection-focus>"

Added: trunk/LayoutTests/editing/mac/spelling/advance-to-next-misspelling.html (0 => 263499)


--- trunk/LayoutTests/editing/mac/spelling/advance-to-next-misspelling.html	                        (rev 0)
+++ trunk/LayoutTests/editing/mac/spelling/advance-to-next-misspelling.html	2020-06-25 04:42:11 UTC (rev 263499)
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<head>
+<script src=""
+</head>
+<body>
+<br>
+<div contenteditable id="editor"><p id="start"><br>mispelled</p></div>
+<script>
+Markup.description("Advancing to the next misspelling (cmd+; on macOS) should select the word 'mispelled'.");
+getSelection().setPosition(start);
+internals.advanceToNextMisspelling();
+Markup.dump(editor);
+</script>
+</body>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (263498 => 263499)


--- trunk/Source/WebCore/ChangeLog	2020-06-25 04:25:05 UTC (rev 263498)
+++ trunk/Source/WebCore/ChangeLog	2020-06-25 04:42:11 UTC (rev 263499)
@@ -1,3 +1,37 @@
+2020-06-24  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Running spellcheck on https://developer.apple.com/forums/thread/650317 hangs the web process
+        https://bugs.webkit.org/show_bug.cgi?id=213585
+        <rdar://problem/64681632>
+
+        Reviewed by Simon Fraser.
+
+        In `TextCheckingHelper::findFirstMisspellingOrBadGrammar`, we attempt to find the first misspelled piece of text
+        by iterating over the current paragraph range; when we exhaust the current paragraph range, we try to move on to
+        the next paragraph range by setting the current paragraph range to the start and end of the next paragraph,
+        using `startOfNextParagraph`.
+
+        However, `startOfNextParagraph` may return the null position in some cases (as seen in this bug, and the
+        associated layout test). This logic isn't robust against this possibility, and will end up looping infinitely in
+        the while loop, since `setStart` and `setEnd` are no-ops when given a null position, and so the current
+        paragraph remains the same.
+
+        Make this logic for finding the next misspelled word more robust by allowing us to bail out of the while loop in
+        the case where setStart or setEnd failed to advance the paragraph range (e.g. due to a null position, or DOM
+        exception).
+
+        Test: editing/mac/spelling/advance-to-next-misspelling.html
+
+        * editing/TextCheckingHelper.cpp:
+        (WebCore::TextCheckingHelper::findFirstMisspellingOrBadGrammar):
+        * testing/Internals.cpp:
+        (WebCore::Internals::advanceToNextMisspelling):
+
+        Add an internal testing hook to advance to the next misspelling.
+
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2020-06-24  David Kilzer  <ddkil...@apple.com>
 
         Use ObjectIdentifier<>instead of WebCore::nextPlaybackTargetClientContextId() in Document.cpp

Modified: trunk/Source/WebCore/editing/TextCheckingHelper.cpp (263498 => 263499)


--- trunk/Source/WebCore/editing/TextCheckingHelper.cpp	2020-06-25 04:25:05 UTC (rev 263498)
+++ trunk/Source/WebCore/editing/TextCheckingHelper.cpp	2020-06-25 04:42:11 UTC (rev 263499)
@@ -407,9 +407,14 @@
         }
         if (lastIteration || totalLengthProcessed + currentLength >= totalRangeLength)
             break;
+
         VisiblePosition newParagraphStart = startOfNextParagraph(paragraphRange->endPosition());
-        setStart(paragraphRange.ptr(), newParagraphStart);
-        setEnd(paragraphRange.ptr(), endOfParagraph(newParagraphStart));
+        if (!setStart(paragraphRange.ptr(), newParagraphStart))
+            break;
+
+        if (!setEnd(paragraphRange.ptr(), endOfParagraph(newParagraphStart)))
+            break;
+
         firstIteration = false;
         totalLengthProcessed += currentLength;
     }

Modified: trunk/Source/WebCore/testing/Internals.cpp (263498 => 263499)


--- trunk/Source/WebCore/testing/Internals.cpp	2020-06-25 04:25:05 UTC (rev 263498)
+++ trunk/Source/WebCore/testing/Internals.cpp	2020-06-25 04:42:11 UTC (rev 263499)
@@ -2187,6 +2187,14 @@
     return document->editor().spellChecker().lastProcessedIdentifier().toUInt64();
 }
 
+void Internals::advanceToNextMisspelling()
+{
+#if !PLATFORM(IOS_FAMILY)
+    if (auto* document = contextDocument())
+        document->editor().advanceToNextMisspelling();
+#endif
+}
+
 Vector<String> Internals::userPreferredLanguages() const
 {
     return WTF::userPreferredLanguages();

Modified: trunk/Source/WebCore/testing/Internals.h (263498 => 263499)


--- trunk/Source/WebCore/testing/Internals.h	2020-06-25 04:25:05 UTC (rev 263498)
+++ trunk/Source/WebCore/testing/Internals.h	2020-06-25 04:42:11 UTC (rev 263499)
@@ -326,6 +326,7 @@
 
     ExceptionOr<uint64_t> lastSpellCheckRequestSequence();
     ExceptionOr<uint64_t> lastSpellCheckProcessedSequence();
+    void advanceToNextMisspelling();
 
     Vector<String> userPreferredLanguages() const;
     void setUserPreferredLanguages(const Vector<String>&);

Modified: trunk/Source/WebCore/testing/Internals.idl (263498 => 263499)


--- trunk/Source/WebCore/testing/Internals.idl	2020-06-25 04:25:05 UTC (rev 263498)
+++ trunk/Source/WebCore/testing/Internals.idl	2020-06-25 04:42:11 UTC (rev 263499)
@@ -380,6 +380,7 @@
 
     [MayThrowException] unsigned long long lastSpellCheckRequestSequence();
     [MayThrowException] unsigned long long lastSpellCheckProcessedSequence();
+    void advanceToNextMisspelling();
 
     sequence<DOMString> userPreferredLanguages();
     void setUserPreferredLanguages(sequence<DOMString> languages);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to