- Revision
- 249145
- Author
- alanc...@apple.com
- Date
- 2019-08-27 09:19:34 -0700 (Tue, 27 Aug 2019)
Log Message
Cherry-pick r249074. rdar://problem/54735492
[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.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249074 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-608-branch/LayoutTests/ChangeLog (249144 => 249145)
--- branches/safari-608-branch/LayoutTests/ChangeLog 2019-08-27 16:19:29 UTC (rev 249144)
+++ branches/safari-608-branch/LayoutTests/ChangeLog 2019-08-27 16:19:34 UTC (rev 249145)
@@ -1,3 +1,69 @@
+2019-08-27 Alan Coon <alanc...@apple.com>
+
+ Cherry-pick r249074. rdar://problem/54735492
+
+ [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.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249074 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-08-23 Wenson Hsieh <wenson_hs...@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 Ryan Haddad <ryanhad...@apple.com>
Cherry-pick r249042. rdar://problem/54622280
Added: branches/safari-608-branch/LayoutTests/fast/events/ios/autocorrect-with-apostrophe-expected.txt (0 => 249145)
--- branches/safari-608-branch/LayoutTests/fast/events/ios/autocorrect-with-apostrophe-expected.txt (rev 0)
+++ branches/safari-608-branch/LayoutTests/fast/events/ios/autocorrect-with-apostrophe-expected.txt 2019-08-27 16:19:34 UTC (rev 249145)
@@ -0,0 +1,12 @@
+
+This test verifies that "I'" can be autocorrected to "I’m". To manually run the test, focus the text area, place the caret at the end of the apostrophe, and select the "I’m" text candidate above the keyboard. Check that the text candidate is inserted as expected.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS Observed input event of inputType insertReplacementText
+PASS document.activeElement is editor
+PASS editor.value is "I’m"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: branches/safari-608-branch/LayoutTests/fast/events/ios/autocorrect-with-apostrophe.html (0 => 249145)
--- branches/safari-608-branch/LayoutTests/fast/events/ios/autocorrect-with-apostrophe.html (rev 0)
+++ branches/safari-608-branch/LayoutTests/fast/events/ios/autocorrect-with-apostrophe.html 2019-08-27 16:19:34 UTC (rev 249145)
@@ -0,0 +1,41 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+ <meta charset="utf8">
+ <meta name="viewport" content="initial-scale=1.0, user-scalable=no, width=device-width">
+ <script src=""
+ <script src=""
+ <style>
+ textarea {
+ font-size: 18px;
+ }
+ </style>
+ <script>
+ jsTestIsAsync = true;
+
+ addEventListener("load", async () => {
+ description(`This test verifies that "I'" can be autocorrected to "I’m". To manually run the test, focus the text area, place the caret at the end of the apostrophe, and select the "I’m" text candidate above the keyboard. Check that the text candidate is inserted as expected.`);
+
+ addEventListener("input", event => testPassed(`Observed input event of inputType ${event.inputType}`));
+
+ if (!window.testRunner || !testRunner.runUIScript)
+ return;
+
+ editor = document.getElementById("editor");
+ await UIHelper.activateElementAndWaitForInputSession(editor);
+ await UIHelper.applyAutocorrection("I’m", "I'");
+ shouldBe("document.activeElement", "editor");
+ shouldBeEqualToString("editor.value", "I’m");
+ editor.blur();
+ await UIHelper.waitForKeyboardToHide();
+ finishJSTest();
+ });
+ </script>
+</head>
+<body>
+ <textarea contenteditable id="editor">I’</textarea>
+ <div id="description"></div>
+ <div id="console"></div>
+</body>
+
+</html>
Modified: branches/safari-608-branch/Source/WebCore/ChangeLog (249144 => 249145)
--- branches/safari-608-branch/Source/WebCore/ChangeLog 2019-08-27 16:19:29 UTC (rev 249144)
+++ branches/safari-608-branch/Source/WebCore/ChangeLog 2019-08-27 16:19:34 UTC (rev 249145)
@@ -1,5 +1,74 @@
2019-08-27 Alan Coon <alanc...@apple.com>
+ Cherry-pick r249074. rdar://problem/54735492
+
+ [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.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249074 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-08-23 Wenson Hsieh <wenson_hs...@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-27 Alan Coon <alanc...@apple.com>
+
Cherry-pick r248886. rdar://problem/54365278
Source/WebCore:
Modified: branches/safari-608-branch/Source/WebCore/editing/TextIterator.cpp (249144 => 249145)
--- branches/safari-608-branch/Source/WebCore/editing/TextIterator.cpp 2019-08-27 16:19:29 UTC (rev 249144)
+++ branches/safari-608-branch/Source/WebCore/editing/TextIterator.cpp 2019-08-27 16:19:34 UTC (rev 249145)
@@ -1782,18 +1782,19 @@
// FIXME: We'd like to tailor the searcher to fold quote marks for us instead
// 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 foldQuoteMarks(const String& stringToFold)
{
- string.replace(hebrewPunctuationGeresh, '\'');
- string.replace(hebrewPunctuationGershayim, '"');
- string.replace(leftDoubleQuotationMark, '"');
- string.replace(leftLowDoubleQuotationMark, '"');
- string.replace(leftSingleQuotationMark, '\'');
- string.replace(leftLowSingleQuotationMark, '\'');
- string.replace(rightDoubleQuotationMark, '"');
- string.replace(rightSingleQuotationMark, '\'');
+ 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 string;
+ return result;
}
#if !UCONFIG_NO_COLLATION
@@ -2410,7 +2411,7 @@
#else
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())
, m_isCharacterStartBuffer(m_target.length())
@@ -2419,7 +2420,6 @@
{
ASSERT(!m_target.isEmpty());
m_target.replace(noBreakSpace, ' ');
- foldQuoteMarks(m_target);
}
inline SearchBuffer::~SearchBuffer() = default;
Modified: branches/safari-608-branch/Source/WebCore/editing/TextIterator.h (249144 => 249145)
--- branches/safari-608-branch/Source/WebCore/editing/TextIterator.h 2019-08-27 16:19:29 UTC (rev 249144)
+++ branches/safari-608-branch/Source/WebCore/editing/TextIterator.h 2019-08-27 16:19:34 UTC (rev 249145)
@@ -54,6 +54,7 @@
WEBCORE_EXPORT Ref<Range> findClosestPlainText(const Range&, const String&, FindOptions, unsigned);
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.
bool isRendererReplacedElement(RenderObject*);
Modified: branches/safari-608-branch/Source/WebKit/ChangeLog (249144 => 249145)
--- branches/safari-608-branch/Source/WebKit/ChangeLog 2019-08-27 16:19:29 UTC (rev 249144)
+++ branches/safari-608-branch/Source/WebKit/ChangeLog 2019-08-27 16:19:34 UTC (rev 249145)
@@ -1,3 +1,85 @@
+2019-08-27 Alan Coon <alanc...@apple.com>
+
+ Cherry-pick r249074. rdar://problem/54735492
+
+ [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.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249074 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-08-23 Wenson Hsieh <wenson_hs...@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-22 Kocsen Chung <kocsen_ch...@apple.com>
Cherry-pick r249006. rdar://problem/54600921
Modified: branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (249144 => 249145)
--- branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2019-08-27 16:19:29 UTC (rev 249144)
+++ branches/safari-608-branch/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2019-08-27 16:19:34 UTC (rev 249145)
@@ -2369,12 +2369,13 @@
RefPtr<Range> range;
String textForRange;
+ auto originalTextWithFoldedQuoteMarks = foldQuoteMarks(originalText);
if (frame.selection().isCaret()) {
VisiblePosition position = frame.selection().selection().start();
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)
position = position.previous();
@@ -2404,7 +2405,7 @@
textForRange = plainTextReplacingNoBreakSpace(range.get());
}
- if (textForRange != originalText)
+ if (foldQuoteMarks(textForRange) != originalTextWithFoldedQuoteMarks)
return false;
// Correctly determine affinity, using logic currently only present in VisiblePosition
