Changeset 249074 in webkit

Timestamp:

Aug 23, 2019, 4:00:38 PM (7 years ago)

Author:

Wenson Hsieh

Message:

[iOS] [WebKit2] Tapping on the “I’m” text suggestion after typing “i’” does nothing
​https://bugs.webkit.org/show_bug.cgi?id=201085
<rdar://problem/53056118>

Reviewed by Tim Horton.

Source/WebCore:

Exposes an existing quote folding function as a helper on TextIterator, and also adjusts foldQuoteMarks to take
a const String& rather than a String. See WebKit ChangeLog for more details.

• editing/TextIterator.cpp:

(WebCore::foldQuoteMarks):
(WebCore::SearchBuffer::SearchBuffer):

• editing/TextIterator.h:

Source/WebKit:

Currently, logic in applyAutocorrectionInternal only selects the range to autocorrect if the text of the range
matches the string to replace (delivered to us from UIKit). In the case of changing "I’" to "I’m", the string to
replace is "I'" (with a straight quote rather than an apostrophe), even though the DOM contains an apostrophe.

This is because kbd believes that the document context contains straight quotes (rather than apostrophes). For
native text views, this works out because UIKit uses relative UITextPositions to determine the replacement
range rather than by checking against the contents of the document. However, WKWebView does not have the ability
to synchronously compute and reason about arbitrary UITextPositions relative to the selection, so we instead
search for the string near the current selection when applying autocorrections.

Of course, this doesn't work in this scenario because the replacement string contains a straight quote, yet the
text node contains an apostrophe, so we bail and don't end up replacing any text. To address this, we repurpose
TextIterator helpers currently used to allow find-in-page to match straight quotes against apostrophes; instead
of matching the replacement string exactly, we instead match the quote-folded versions of these strings when
finding the range to replace.

Test: fast/events/ios/autocorrect-with-apostrophe.html

• WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::applyAutocorrectionInternal):

LayoutTests:

Add a new layout test to verify that "I’" can be autocorrected to "I’m".

• fast/events/ios/autocorrect-with-apostrophe-expected.txt: Added.
• fast/events/ios/autocorrect-with-apostrophe.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/ios/autocorrect-with-apostrophe-expected.txt (added)
• LayoutTests/fast/events/ios/autocorrect-with-apostrophe.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/TextIterator.cpp (modified) (3 diffs)
• Source/WebCore/editing/TextIterator.h (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-08-23  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] [WebKit2] Tapping on the “I’m” text suggestion after typing “i’” does nothing
+        https://bugs.webkit.org/show_bug.cgi?id=201085
+        <rdar://problem/53056118>
+
+        Reviewed by Tim Horton.
+
+        Add a new layout test to verify that "I’" can be autocorrected to "I’m".
+
+        * fast/events/ios/autocorrect-with-apostrophe-expected.txt: Added.
+        * fast/events/ios/autocorrect-with-apostrophe.html: Added.
+
 2019-08-23  Tim Horton  <timothy_horton@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-08-23  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] [WebKit2] Tapping on the “I’m” text suggestion after typing “i’” does nothing
+        https://bugs.webkit.org/show_bug.cgi?id=201085
+        <rdar://problem/53056118>
+
+        Reviewed by Tim Horton.
+
+        Exposes an existing quote folding function as a helper on TextIterator, and also adjusts foldQuoteMarks to take
+        a const String& rather than a String. See WebKit ChangeLog for more details.
+
+        * editing/TextIterator.cpp:
+        (WebCore::foldQuoteMarks):
+        (WebCore::SearchBuffer::SearchBuffer):
+        * editing/TextIterator.h:
+
 2019-08-23  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/editing/TextIterator.cpp

@@ -1783 +1783 @@
 // of doing it in a separate replacement pass here, but ICU doesn't offer a way
 // to add tailoring on top of the locale-specific tailoring as of this writing.
-static inline String foldQuoteMarks(String string)
-{
-    string.replace(hebrewPunctuationGeresh, '\'');
-    string.replace(hebrewPunctuationGershayim, '"');
-    string.replace(leftDoubleQuotationMark, '"');
-    string.replace(leftLowDoubleQuotationMark, '"');
-    string.replace(leftSingleQuotationMark, '\'');
-    string.replace(leftLowSingleQuotationMark, '\'');
-    string.replace(rightDoubleQuotationMark, '"');
-    string.replace(rightSingleQuotationMark, '\'');
-
-    return string;
+String foldQuoteMarks(const String& stringToFold)
+{
+    String result(stringToFold);
+    result.replace(hebrewPunctuationGeresh, '\'');
+    result.replace(hebrewPunctuationGershayim, '"');
+    result.replace(leftDoubleQuotationMark, '"');
+    result.replace(leftLowDoubleQuotationMark, '"');
+    result.replace(leftSingleQuotationMark, '\'');
+    result.replace(leftLowSingleQuotationMark, '\'');
+    result.replace(rightDoubleQuotationMark, '"');
+    result.replace(rightSingleQuotationMark, '\'');
+
+    return result;
 }
 
@@ -2411 +2412 @@
 
 inline SearchBuffer::SearchBuffer(const String& target, FindOptions options)
-    : m_target(options & CaseInsensitive ? target.foldCase() : target)
+    : m_target(foldQuoteMarks(options & CaseInsensitive ? target.foldCase() : target))
     , m_options(options)
     , m_buffer(m_target.length())
@@ -2420 +2421 @@
     ASSERT(!m_target.isEmpty());
     m_target.replace(noBreakSpace, ' ');
-    foldQuoteMarks(m_target);
 }
 

trunk/Source/WebCore/editing/TextIterator.h

@@ -55 +55 @@
 WEBCORE_EXPORT bool hasAnyPlainText(const Range&, TextIteratorBehavior = TextIteratorDefaultBehavior);
 bool findPlainText(const String& document, const String&, FindOptions); // Lets us use the search algorithm on a string.
+WEBCORE_EXPORT String foldQuoteMarks(const String&);
 
 // FIXME: Move this somewhere else in the editing directory. It doesn't belong here.

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-08-23  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] [WebKit2] Tapping on the “I’m” text suggestion after typing “i’” does nothing
+        https://bugs.webkit.org/show_bug.cgi?id=201085
+        <rdar://problem/53056118>
+
+        Reviewed by Tim Horton.
+
+        Currently, logic in applyAutocorrectionInternal only selects the range to autocorrect if the text of the range
+        matches the string to replace (delivered to us from UIKit). In the case of changing "I’" to "I’m", the string to
+        replace is "I'" (with a straight quote rather than an apostrophe), even though the DOM contains an apostrophe.
+
+        This is because kbd believes that the document context contains straight quotes (rather than apostrophes). For
+        native text views, this works out because UIKit uses relative UITextPositions to determine the replacement
+        range rather than by checking against the contents of the document. However, WKWebView does not have the ability
+        to synchronously compute and reason about arbitrary UITextPositions relative to the selection, so we instead
+        search for the string near the current selection when applying autocorrections.
+
+        Of course, this doesn't work in this scenario because the replacement string contains a straight quote, yet the
+        text node contains an apostrophe, so we bail and don't end up replacing any text. To address this, we repurpose
+        TextIterator helpers currently used to allow find-in-page to match straight quotes against apostrophes; instead
+        of matching the replacement string exactly, we instead match the quote-folded versions of these strings when
+        finding the range to replace.
+
+        Test: fast/events/ios/autocorrect-with-apostrophe.html
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::applyAutocorrectionInternal):
+
 2019-08-23  Jiewen Tan  <jiewen_tan@apple.com>
 

trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

@@ -2335 +2335 @@
     RefPtr<Range> range;
     String textForRange;
+    auto originalTextWithFoldedQuoteMarks = foldQuoteMarks(originalText);
 
     if (frame.selection().isCaret()) {
@@ -2340 +2341 @@
         range = wordRangeFromPosition(position);
         textForRange = plainTextReplacingNoBreakSpace(range.get());
-        if (textForRange != originalText) {
+        if (foldQuoteMarks(textForRange) != originalTextWithFoldedQuoteMarks) {
             // Search for the original text before the selection caret.
             for (size_t i = 0; i < originalText.length(); ++i)
@@ -2370 +2371 @@
     }
 
-    if (textForRange != originalText)
+    if (foldQuoteMarks(textForRange) != originalTextWithFoldedQuoteMarks)
         return false;