Title: [268009] branches/safari-610-branch
Revision
268009
Author
alanc...@apple.com
Date
2020-10-05 15:38:23 -0700 (Mon, 05 Oct 2020)

Log Message

Cherry-pick r266909. rdar://problem/69101091

    Source/WebCore:
    Text replacements at the beginning of a second line are replaced too early
    https://bugs.webkit.org/show_bug.cgi?id=216327
    <rdar://problem/68170353>

    Reviewed by Darin Adler.

    In the changes in r258871, using SimpleRanges instead of Range causing some side effects
    when the replacements at the beginning of lines. The ranges that we are counting are backwards
    and the return characters are being counted instead of being ignored. There is almost
    certainly a better fix than this, but this patch restores the original logic that
    was present when Range was being used, until a better fix can be worked out.

    Test: editing/spelling/text-replacement-first-word-second-line.html

    * editing/Editor.cpp:
    (WebCore::Editor::markAndReplaceFor):
    * editing/TextCheckingHelper.cpp:
    (WebCore::TextCheckingParagraph::automaticReplacementStart const):
    (WebCore::TextCheckingParagraph::automaticReplacementLength const):
    * editing/TextCheckingHelper.h:

    LayoutTests:
    Overlapping text replacements at the beginning of a line are replaced too early
    https://bugs.webkit.org/show_bug.cgi?id=216327

    Reviewed by Darin Adler.

    * editing/spelling/text-replacement-first-word-second-line-expected.txt: Added.
    * editing/spelling/text-replacement-first-word-second-line.html: Added.

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

Modified Paths

Added Paths

Diff

Modified: branches/safari-610-branch/LayoutTests/ChangeLog (268008 => 268009)


--- branches/safari-610-branch/LayoutTests/ChangeLog	2020-10-05 22:38:11 UTC (rev 268008)
+++ branches/safari-610-branch/LayoutTests/ChangeLog	2020-10-05 22:38:23 UTC (rev 268009)
@@ -1,3 +1,50 @@
+2020-10-05  Alan Coon  <alanc...@apple.com>
+
+        Cherry-pick r266909. rdar://problem/69101091
+
+    Source/WebCore:
+    Text replacements at the beginning of a second line are replaced too early
+    https://bugs.webkit.org/show_bug.cgi?id=216327
+    <rdar://problem/68170353>
+    
+    Reviewed by Darin Adler.
+    
+    In the changes in r258871, using SimpleRanges instead of Range causing some side effects
+    when the replacements at the beginning of lines. The ranges that we are counting are backwards
+    and the return characters are being counted instead of being ignored. There is almost
+    certainly a better fix than this, but this patch restores the original logic that
+    was present when Range was being used, until a better fix can be worked out.
+    
+    Test: editing/spelling/text-replacement-first-word-second-line.html
+    
+    * editing/Editor.cpp:
+    (WebCore::Editor::markAndReplaceFor):
+    * editing/TextCheckingHelper.cpp:
+    (WebCore::TextCheckingParagraph::automaticReplacementStart const):
+    (WebCore::TextCheckingParagraph::automaticReplacementLength const):
+    * editing/TextCheckingHelper.h:
+    
+    LayoutTests:
+    Overlapping text replacements at the beginning of a line are replaced too early
+    https://bugs.webkit.org/show_bug.cgi?id=216327
+    
+    Reviewed by Darin Adler.
+    
+    * editing/spelling/text-replacement-first-word-second-line-expected.txt: Added.
+    * editing/spelling/text-replacement-first-word-second-line.html: Added.
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266909 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2020-09-10  Megan Gardner  <megan_gard...@apple.com>
+
+            Overlapping text replacements at the beginning of a line are replaced too early
+            https://bugs.webkit.org/show_bug.cgi?id=216327
+
+            Reviewed by Darin Adler.
+
+            * editing/spelling/text-replacement-first-word-second-line-expected.txt: Added.
+            * editing/spelling/text-replacement-first-word-second-line.html: Added.
+
 2020-10-02  Alan Coon  <alanc...@apple.com>
 
         Cherry-pick r266028. rdar://problem/69904377

Added: branches/safari-610-branch/LayoutTests/editing/spelling/text-replacement-first-word-second-line-expected.txt (0 => 268009)


--- branches/safari-610-branch/LayoutTests/editing/spelling/text-replacement-first-word-second-line-expected.txt	                        (rev 0)
+++ branches/safari-610-branch/LayoutTests/editing/spelling/text-replacement-first-word-second-line-expected.txt	2020-10-05 22:38:23 UTC (rev 268009)
@@ -0,0 +1,32 @@
+To manually test, add an automatic text replacement mapping from the string '**' to 'stars'. Then, on the second line (this is imporatant, it must not be the first line) of a contenteditable, type the string '**'. It should not be immediately corrected to 'stars'. Type any other character, which should then change '**' to 'stars'.
+
+After typing possible replacment:
+| "
+        "
+| <div>
+|   <br>
+| <div>
+|   "**<#selection-caret>"
+| "
+        "
+| <div>
+|   <br>
+| "
+    "
+
+After pressing enter:
+| "
+        "
+| <div>
+|   <br>
+| <div>
+|   "stars"
+| <div>
+|   <#selection-caret>
+|   <br>
+| "
+        "
+| <div>
+|   <br>
+| "
+    "

Added: branches/safari-610-branch/LayoutTests/editing/spelling/text-replacement-first-word-second-line.html (0 => 268009)


--- branches/safari-610-branch/LayoutTests/editing/spelling/text-replacement-first-word-second-line.html	                        (rev 0)
+++ branches/safari-610-branch/LayoutTests/editing/spelling/text-replacement-first-word-second-line.html	2020-10-05 22:38:23 UTC (rev 268009)
@@ -0,0 +1,63 @@
+<html>
+<head>
+<script src=""
+<script src=""
+<script src=""
+<script>
+Markup.description("To manually test, add an automatic text replacement mapping from the string '**' to 'stars'. Then, on the second line (this is imporatant, it must not be the first line) of a contenteditable, type the string '**'. It should not be immediately corrected to 'stars'. Type any other character, which should then change '**' to 'stars'.");
+Markup.waitUntilDone();
+
+async function runTest()
+{
+    if (window.internals) {
+        internals.settings.setUnifiedTextCheckerEnabled(true);
+        internals.settings.setAsynchronousSpellCheckingEnabled(false);
+        internals.setAutomaticTextReplacementEnabled(true);
+        internals.setAutomaticSpellingCorrectionEnabled(true);
+        await UIHelper.setSpellCheckerResults({
+            "**": [
+                {
+                    "replacement": "stars",
+                    "type": "replacement",
+                    "from": 0,
+                    "to": 2
+                }
+            ],
+            "**\n": [
+                {
+                    "replacement": "stars",
+                    "type": "replacement",
+                    "from": 0,
+                    "to": 2
+                }
+            ]
+        });
+    }
+
+    const editor = document.getElementById("editor");
+    editor.focus();
+    insertParagraphCommand();
+
+    for (const character of "**")
+        typeCharacterCommand(character);
+    await UIHelper.delayFor(0);
+
+    Markup.dump(editor, "After typing possible replacment");
+
+    insertParagraphCommand();
+    await UIHelper.delayFor(0);
+
+    Markup.dump(editor, "After pressing enter");
+    Markup.notifyDone();
+}
+</script>
+</head>
+
+<body _onload_="runTest()">
+    <div id="description"></div>
+    <div contenteditable style="margin-bottom: 1em; border: 1px orange dashed;" id="editor">
+        <div><br></div>
+        <div><br></div>
+    </div>
+</body>
+</html>

Modified: branches/safari-610-branch/LayoutTests/platform/gtk/TestExpectations (268008 => 268009)


--- branches/safari-610-branch/LayoutTests/platform/gtk/TestExpectations	2020-10-05 22:38:11 UTC (rev 268008)
+++ branches/safari-610-branch/LayoutTests/platform/gtk/TestExpectations	2020-10-05 22:38:23 UTC (rev 268009)
@@ -945,6 +945,7 @@
 
 # Requires support for testing text replacement
 editing/spelling/text-replacement-after-typing-to-word.html [ Skip ]
+editing/spelling/text-replacement-first-word-second-line.html [ Skip ]
 
 # MediaRecorder is not currently implemented
 http/wpt/mediarecorder [ Skip ]

Modified: branches/safari-610-branch/LayoutTests/platform/ios/TestExpectations (268008 => 268009)


--- branches/safari-610-branch/LayoutTests/platform/ios/TestExpectations	2020-10-05 22:38:11 UTC (rev 268008)
+++ branches/safari-610-branch/LayoutTests/platform/ios/TestExpectations	2020-10-05 22:38:23 UTC (rev 268009)
@@ -127,6 +127,7 @@
 
 # Requires support for testing text replacement
 editing/spelling/text-replacement-after-typing-to-word.html [ Skip ]
+editing/spelling/text-replacement-first-word-second-line.html [ Skip ]
 
 # UIKit draws selection on iOS
 fast/selectors/input-with-selection-pseudo-element.html [ WontFix ]

Modified: branches/safari-610-branch/LayoutTests/platform/wincairo/TestExpectations (268008 => 268009)


--- branches/safari-610-branch/LayoutTests/platform/wincairo/TestExpectations	2020-10-05 22:38:11 UTC (rev 268008)
+++ branches/safari-610-branch/LayoutTests/platform/wincairo/TestExpectations	2020-10-05 22:38:23 UTC (rev 268009)
@@ -701,6 +701,7 @@
 editing/spelling/spellcheck-attribute.html [ Timeout ]
 
 # UIScriptController.setContinuousSpellCheckingEnabled is not implemented.
+editing/spelling/text-replacement-first-word-second-line.html [ Skip ]
 editing/spelling/toggle-spellchecking.html [ Skip ]
 
 # LayoutTest/fast/loader tests that fail or time out.

Modified: branches/safari-610-branch/Source/WebCore/ChangeLog (268008 => 268009)


--- branches/safari-610-branch/Source/WebCore/ChangeLog	2020-10-05 22:38:11 UTC (rev 268008)
+++ branches/safari-610-branch/Source/WebCore/ChangeLog	2020-10-05 22:38:23 UTC (rev 268009)
@@ -1,5 +1,65 @@
 2020-10-05  Alan Coon  <alanc...@apple.com>
 
+        Cherry-pick r266909. rdar://problem/69101091
+
+    Source/WebCore:
+    Text replacements at the beginning of a second line are replaced too early
+    https://bugs.webkit.org/show_bug.cgi?id=216327
+    <rdar://problem/68170353>
+    
+    Reviewed by Darin Adler.
+    
+    In the changes in r258871, using SimpleRanges instead of Range causing some side effects
+    when the replacements at the beginning of lines. The ranges that we are counting are backwards
+    and the return characters are being counted instead of being ignored. There is almost
+    certainly a better fix than this, but this patch restores the original logic that
+    was present when Range was being used, until a better fix can be worked out.
+    
+    Test: editing/spelling/text-replacement-first-word-second-line.html
+    
+    * editing/Editor.cpp:
+    (WebCore::Editor::markAndReplaceFor):
+    * editing/TextCheckingHelper.cpp:
+    (WebCore::TextCheckingParagraph::automaticReplacementStart const):
+    (WebCore::TextCheckingParagraph::automaticReplacementLength const):
+    * editing/TextCheckingHelper.h:
+    
+    LayoutTests:
+    Overlapping text replacements at the beginning of a line are replaced too early
+    https://bugs.webkit.org/show_bug.cgi?id=216327
+    
+    Reviewed by Darin Adler.
+    
+    * editing/spelling/text-replacement-first-word-second-line-expected.txt: Added.
+    * editing/spelling/text-replacement-first-word-second-line.html: Added.
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@266909 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2020-09-10  Megan Gardner  <megan_gard...@apple.com>
+
+            Text replacements at the beginning of a second line are replaced too early
+            https://bugs.webkit.org/show_bug.cgi?id=216327
+            <rdar://problem/68170353>
+
+            Reviewed by Darin Adler.
+
+            In the changes in r258871, using SimpleRanges instead of Range causing some side effects
+            when the replacements at the beginning of lines. The ranges that we are counting are backwards
+            and the return characters are being counted instead of being ignored. There is almost
+            certainly a better fix than this, but this patch restores the original logic that
+            was present when Range was being used, until a better fix can be worked out.
+
+            Test: editing/spelling/text-replacement-first-word-second-line.html
+
+            * editing/Editor.cpp:
+            (WebCore::Editor::markAndReplaceFor):
+            * editing/TextCheckingHelper.cpp:
+            (WebCore::TextCheckingParagraph::automaticReplacementStart const):
+            (WebCore::TextCheckingParagraph::automaticReplacementLength const):
+            * editing/TextCheckingHelper.h:
+
+2020-10-05  Alan Coon  <alanc...@apple.com>
+
         Cherry-pick r267869. rdar://problem/69904332
 
     [iOS WK1] Crashes when using ANGLE WebGL from another thread

Modified: branches/safari-610-branch/Source/WebCore/editing/Editor.cpp (268008 => 268009)


--- branches/safari-610-branch/Source/WebCore/editing/Editor.cpp	2020-10-05 22:38:11 UTC (rev 268008)
+++ branches/safari-610-branch/Source/WebCore/editing/Editor.cpp	2020-10-05 22:38:23 UTC (rev 268009)
@@ -2802,7 +2802,8 @@
         auto resultLocation = results[i].range.location + offsetDueToReplacement;
         auto resultLength = results[i].range.length;
         auto resultEndLocation = resultLocation + resultLength;
-        auto automaticReplacementEndLocation = paragraph.automaticReplacementStart() + paragraph.automaticReplacementLength() + offsetDueToReplacement;
+        auto automaticReplacementStartLocation = paragraph.automaticReplacementStart(true);
+        auto automaticReplacementEndLocation = automaticReplacementStartLocation + paragraph.automaticReplacementLength(true) + offsetDueToReplacement;
         const String& replacement = results[i].replacement;
         bool resultEndsAtAmbiguousBoundary = useAmbiguousBoundaryOffset && selectionOffset - 1 <= resultEndLocation;
 
@@ -2827,7 +2828,7 @@
                     addMarker(badGrammarRange, DocumentMarker::Grammar, detail.userDescription);
                 }
             }
-        } else if (resultEndLocation <= automaticReplacementEndLocation && resultEndLocation >= paragraph.automaticReplacementStart()
+        } else if (automaticReplacementStartLocation <= resultEndLocation && resultEndLocation <= automaticReplacementEndLocation
             && isAutomaticTextReplacementType(resultType)) {
             // In this case the result range just has to touch the automatic replacement range, so we can handle replacing non-word text such as punctuation.
             ASSERT(resultLength > 0);

Modified: branches/safari-610-branch/Source/WebCore/editing/TextCheckingHelper.cpp (268008 => 268009)


--- branches/safari-610-branch/Source/WebCore/editing/TextCheckingHelper.cpp	2020-10-05 22:38:11 UTC (rev 268008)
+++ branches/safari-610-branch/Source/WebCore/editing/TextCheckingHelper.cpp	2020-10-05 22:38:23 UTC (rev 268009)
@@ -208,15 +208,21 @@
     return *m_checkingLength;
 }
 
-uint64_t TextCheckingParagraph::automaticReplacementStart() const
+uint64_t TextCheckingParagraph::automaticReplacementStart(bool oldBehaviour) const
 {
+    if (oldBehaviour && is_gt(documentOrder(paragraphRange().start, m_automaticReplacementRange.start)))
+        return 0;
+    
     if (!m_automaticReplacementStart)
         m_automaticReplacementStart = characterCount({ paragraphRange().start, m_automaticReplacementRange.start });
     return *m_automaticReplacementStart;
 }
 
-uint64_t TextCheckingParagraph::automaticReplacementLength() const
+uint64_t TextCheckingParagraph::automaticReplacementLength(bool oldBehaviour) const
 {
+    if (oldBehaviour && is_gt(documentOrder(paragraphRange().start, m_automaticReplacementRange.start)))
+        return characterCount({ paragraphRange().start, m_automaticReplacementRange.end });
+    
     if (!m_automaticReplacementLength)
         m_automaticReplacementLength = characterCount(m_automaticReplacementRange);
     return *m_automaticReplacementLength;

Modified: branches/safari-610-branch/Source/WebCore/editing/TextCheckingHelper.h (268008 => 268009)


--- branches/safari-610-branch/Source/WebCore/editing/TextCheckingHelper.h	2020-10-05 22:38:11 UTC (rev 268008)
+++ branches/safari-610-branch/Source/WebCore/editing/TextCheckingHelper.h	2020-10-05 22:38:23 UTC (rev 268009)
@@ -54,8 +54,8 @@
     uint64_t checkingLength() const;
     StringView checkingSubstring() const { return text().substring(checkingStart(), checkingLength()); }
 
-    uint64_t automaticReplacementStart() const;
-    uint64_t automaticReplacementLength() const;
+    uint64_t automaticReplacementStart(bool = false) const;
+    uint64_t automaticReplacementLength(bool = false) const;
 
     bool checkingRangeMatches(CharacterRange range) const { return range.location == checkingStart() && range.length == checkingLength(); }
     bool isCheckingRangeCoveredBy(CharacterRange range) const { return range.location <= checkingStart() && range.location + range.length >= checkingStart() + checkingLength(); }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to