Title: [293051] trunk
- Revision
- 293051
- Author
- [email protected]
- Date
- 2022-04-19 17:25:17 -0700 (Tue, 19 Apr 2022)
Log Message
[iOS] Dictation text that contains emojis is inserted twice upon finalization
https://bugs.webkit.org/show_bug.cgi?id=239508
rdar://91895524
Reviewed by Aditya Keerthi.
Source/WebKit:
`WebPage::replaceDictatedText` contains logic to only proceed with text replacement in the case where the
dictated text still matches the text before the current selection. To ensure this, it marches backwards through
visible positions from the start of the selection, for a total number of times equal to the length of the
expected string (`oldText`). If the text within this found range (i.e. from the position before the start of the
selection, to the start of the selection) no longer matches the `oldText` provided by the client, we return
early and do nothing.
However, this logic fails in the case where `oldText` contains emojis (or more generally, multiple codepoints
that combine to form a single grapheme cluster), since the length of the string is more than the number of
visible position iterations needed to find the correct starting position for the range. As a result, we end up
not replacing any characters, and when UIKit dictation code later calls `-insertText:` with the final dictation
string, we end up duplicating the finalized dictation text.
To address this issue, simply advance backwards by grapheme count instead of the raw string length to keep the
starting visible position consistent with the start of the range that we're trying to replace.
Test: UIWKInteractionViewProtocol.ReplaceDictatedTextContainingEmojis
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::replaceDictatedText):
(WebKit::WebPage::applyAutocorrectionInternal):
Also add a FIXME here around similar code that might also be susceptible to the same bug. However, in this case,
we avoid the problem because of subsequent logic that adjusts the starting position to try and match the
expected string. We may be able to remove or further limit this extra adjustment by using grapheme cluster count
here instead of string length when finding the initial starting position.
Tools:
Add an API test to exercise the change.
* TestWebKitAPI/Tests/ios/UIWKInteractionViewProtocol.mm:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (293050 => 293051)
--- trunk/Source/WebKit/ChangeLog 2022-04-20 00:23:31 UTC (rev 293050)
+++ trunk/Source/WebKit/ChangeLog 2022-04-20 00:25:17 UTC (rev 293051)
@@ -1,3 +1,38 @@
+2022-04-19 Wenson Hsieh <[email protected]>
+
+ [iOS] Dictation text that contains emojis is inserted twice upon finalization
+ https://bugs.webkit.org/show_bug.cgi?id=239508
+ rdar://91895524
+
+ Reviewed by Aditya Keerthi.
+
+ `WebPage::replaceDictatedText` contains logic to only proceed with text replacement in the case where the
+ dictated text still matches the text before the current selection. To ensure this, it marches backwards through
+ visible positions from the start of the selection, for a total number of times equal to the length of the
+ expected string (`oldText`). If the text within this found range (i.e. from the position before the start of the
+ selection, to the start of the selection) no longer matches the `oldText` provided by the client, we return
+ early and do nothing.
+
+ However, this logic fails in the case where `oldText` contains emojis (or more generally, multiple codepoints
+ that combine to form a single grapheme cluster), since the length of the string is more than the number of
+ visible position iterations needed to find the correct starting position for the range. As a result, we end up
+ not replacing any characters, and when UIKit dictation code later calls `-insertText:` with the final dictation
+ string, we end up duplicating the finalized dictation text.
+
+ To address this issue, simply advance backwards by grapheme count instead of the raw string length to keep the
+ starting visible position consistent with the start of the range that we're trying to replace.
+
+ Test: UIWKInteractionViewProtocol.ReplaceDictatedTextContainingEmojis
+
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+ (WebKit::WebPage::replaceDictatedText):
+ (WebKit::WebPage::applyAutocorrectionInternal):
+
+ Also add a FIXME here around similar code that might also be susceptible to the same bug. However, in this case,
+ we avoid the problem because of subsequent logic that adjusts the starting position to try and match the
+ expected string. We may be able to remove or further limit this extra adjustment by using grapheme cluster count
+ here instead of string length when finding the initial starting position.
+
2022-04-19 Chris Dumez <[email protected]>
Inline Element::shadowRoot()
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (293050 => 293051)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2022-04-20 00:23:31 UTC (rev 293050)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2022-04-20 00:25:17 UTC (rev 293051)
@@ -148,14 +148,13 @@
#import <WebCore/UserGestureIndicator.h>
#import <WebCore/VisibleUnits.h>
#import <WebCore/WebEvent.h>
-#import <pal/cocoa/RevealSoftLink.h>
#import <wtf/MathExtras.h>
#import <wtf/MemoryPressureHandler.h>
#import <wtf/Scope.h>
#import <wtf/SetForScope.h>
-#import <wtf/SoftLinking.h>
#import <wtf/cocoa/Entitlements.h>
#import <wtf/text/StringToIntegerConversion.h>
+#import <wtf/text/TextBreakIterator.h>
#import <wtf/text/TextStream.h>
#if ENABLE(ATTACHMENT_ELEMENT)
@@ -162,6 +161,8 @@
#import <WebCore/PromisedAttachmentInfo.h>
#endif
+#import <pal/cocoa/RevealSoftLink.h>
+
#define WEBPAGE_RELEASE_LOG(channel, fmt, ...) RELEASE_LOG(channel, "%p - WebPage::" fmt, this, ##__VA_ARGS__)
#define WEBPAGE_RELEASE_LOG_ERROR(channel, fmt, ...) RELEASE_LOG_ERROR(channel, "%p - WebPage::" fmt, this, ##__VA_ARGS__)
@@ -2402,7 +2403,7 @@
return;
}
VisiblePosition position = frame->selection().selection().start();
- for (size_t i = 0; i < oldText.length(); ++i)
+ for (auto i = numGraphemeClusters(oldText); i; --i)
position = position.previous();
if (position.isNull())
position = startOfDocument(frame->document());
@@ -2519,6 +2520,9 @@
// forward such that it matches the original selection as much as possible.
if (foldQuoteMarks(textForRange) != originalTextWithFoldedQuoteMarks) {
// Search for the original text before the selection caret.
+ // FIXME: Does this do the right thing in the case where `originalText` contains one or more grapheme clusters
+ // that encompass multiple codepoints? Advancing backwards by grapheme cluster count here may also allow us to
+ // sidestep the position adjustment logic below in some cases.
for (size_t i = 0; i < originalText.length(); ++i)
position = position.previous();
if (position.isNull())
Modified: trunk/Tools/ChangeLog (293050 => 293051)
--- trunk/Tools/ChangeLog 2022-04-20 00:23:31 UTC (rev 293050)
+++ trunk/Tools/ChangeLog 2022-04-20 00:25:17 UTC (rev 293051)
@@ -1,3 +1,16 @@
+2022-04-19 Wenson Hsieh <[email protected]>
+
+ [iOS] Dictation text that contains emojis is inserted twice upon finalization
+ https://bugs.webkit.org/show_bug.cgi?id=239508
+ rdar://91895524
+
+ Reviewed by Aditya Keerthi.
+
+ Add an API test to exercise the change.
+
+ * TestWebKitAPI/Tests/ios/UIWKInteractionViewProtocol.mm:
+ (TestWebKitAPI::TEST):
+
2022-04-19 Michael Saboff <[email protected]>
Various WebKit tools need to be told about the system content path
Modified: trunk/Tools/TestWebKitAPI/Tests/ios/UIWKInteractionViewProtocol.mm (293050 => 293051)
--- trunk/Tools/TestWebKitAPI/Tests/ios/UIWKInteractionViewProtocol.mm 2022-04-20 00:23:31 UTC (rev 293050)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/UIWKInteractionViewProtocol.mm 2022-04-20 00:25:17 UTC (rev 293051)
@@ -238,6 +238,19 @@
EXPECT_TRUE(allowsTextInteractionInsideSelection);
}
+TEST(UIWKInteractionViewProtocol, ReplaceDictatedTextContainingEmojis)
+{
+ auto [webView, inputDelegate] = setUpEditableWebViewAndWaitForInputSession();
+ auto contentView = [webView textInputContentView];
+ [contentView selectAll:nil];
+ [contentView insertText:@"Hello world. This 👉🏻 is a good boy"];
+ [webView waitForNextPresentationUpdate];
+
+ [contentView replaceDictatedText:@"This 👉🏻 is a good boy" withText:@"This 👉🏻 is a 🦮"];
+ [webView waitForNextPresentationUpdate];
+ EXPECT_WK_STREQ(@"Hello world. This 👉🏻 is a 🦮", [webView contentsAsString]);
+}
+
TEST(UIWKInteractionViewProtocol, SuppressSelectionChangesDuringDictation)
{
auto [webView, inputDelegate] = setUpEditableWebViewAndWaitForInputSession();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
