Changeset 282247 in webkit

Timestamp:

Sep 9, 2021, 5:15:44 PM (5 years ago)

Author:

Wenson Hsieh

Message:

[Mac Catalyst] Korean characters do not combine correctly in editable fields in WKWebView
​https://bugs.webkit.org/show_bug.cgi?id=230122
rdar://79062204

Reviewed by Devin Rousso.

Source/WebKit:

When using Korean input method (e.g. 2-set Korean) to type into editable input fields in WKWebView on Catalyst,
character combination is broken. While this is partly due to a WebKit-specific UIKit bug (see rdar://82938391),
this is also broken due to the fact that WKContentView reports a UIWKAutocorrectionContext with a
contextBeforeSelection that starts with a newline (\n); this causes UIKit to believe that the text selection
range when a single character has been typed is NSRange { 2, 0 } instead of NSRange { 1, 0 }, due to the
extra newline. This subsequently causes character combination logic to proceed with a range that is off by one
character, which leads to characters not being combined (or the wrong characters being combined).

To address this, we simply avoid including the leading newline in the case where the starting position for the
"context before selection" is exactly at the first position inside the editable root.

Test: AutocorrectionTests.AutocorrectionContextDoesNotIncludeNewlineInTextField

• WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::autocorrectionContext):

Also drive-by fix a typo in "minContextLenght" while I'm here, and rename currentPosition to
contextStartPosition, to better describe the purpose of this local variable.

Tools:

Add a new API test to exercise the change. See WebKit/ChangeLog for more information.

• TestWebKitAPI/Tests/ios/AutocorrectionTestsIOS.mm:

(-[TestWKWebView synchronouslyRequestAutocorrectionContext]):
(TEST):

• TestWebKitAPI/ios/UIKitSPI.h:

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (modified) (2 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/ios/AutocorrectionTestsIOS.mm (modified) (3 diffs)
• Tools/TestWebKitAPI/ios/UIKitSPI.h (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-09-09  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Mac Catalyst] Korean characters do not combine correctly in editable fields in WKWebView
+        https://bugs.webkit.org/show_bug.cgi?id=230122
+        rdar://79062204
+
+        Reviewed by Devin Rousso.
+
+        When using Korean input method (e.g. 2-set Korean) to type into editable input fields in WKWebView on Catalyst,
+        character combination is broken. While this is partly due to a WebKit-specific UIKit bug (see rdar://82938391),
+        this is also broken due to the fact that WKContentView reports a UIWKAutocorrectionContext with a
+        `contextBeforeSelection` that starts with a newline (\n); this causes UIKit to believe that the text selection
+        range when a single character has been typed is `NSRange { 2, 0 }` instead of `NSRange { 1, 0 }`, due to the
+        extra newline. This subsequently causes character combination logic to proceed with a range that is off by one
+        character, which leads to characters not being combined (or the wrong characters being combined).
+
+        To address this, we simply avoid including the leading newline in the case where the starting position for the
+        "context before selection" is exactly at the first position inside the editable root.
+
+        Test: AutocorrectionTests.AutocorrectionContextDoesNotIncludeNewlineInTextField
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::autocorrectionContext):
+
+        Also drive-by fix a typo in "minContextLenght" while I'm here, and rename `currentPosition` to
+        `contextStartPosition`, to better describe the purpose of this local variable.
+
 2021-09-09  Fujii Hironori  <Hironori.Fujii@sony.com>
 

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

@@ -2501 +2501 @@
     VisiblePosition endPosition = frame->selection().selection().end();
     const unsigned minContextWordCount = 3;
-    const unsigned minContextLenght = 12;
+    const unsigned minContextLength = 12;
     const unsigned maxContextLength = 30;
 
@@ -2516 +2516 @@
         }
     } else {
-        if (startPosition != startOfEditableContent(startPosition)) {
-            VisiblePosition currentPosition = startPosition;
+        auto firstPositionInEditableContent = startOfEditableContent(startPosition);
+        if (startPosition != firstPositionInEditableContent) {
+            VisiblePosition contextStartPosition = startPosition;
             VisiblePosition previousPosition;
             unsigned totalContextLength = 0;
             for (unsigned i = 0; i < minContextWordCount; ++i) {
-                if (contextBefore.length() >= minContextLenght)
+                if (contextBefore.length() >= minContextLength)
                     break;
-                previousPosition = startOfWord(positionOfNextBoundaryOfGranularity(currentPosition, TextGranularity::WordGranularity, SelectionDirection::Backward));
+                previousPosition = startOfWord(positionOfNextBoundaryOfGranularity(contextStartPosition, TextGranularity::WordGranularity, SelectionDirection::Backward));
                 if (previousPosition.isNull())
                     break;
-                String currentWord = plainTextForContext(makeSimpleRange(previousPosition, currentPosition));
+                String currentWord = plainTextForContext(makeSimpleRange(previousPosition, contextStartPosition));
                 totalContextLength += currentWord.length();
                 if (totalContextLength >= maxContextLength)
                     break;
-                currentPosition = previousPosition;
+                contextStartPosition = previousPosition;
             }
-            if (currentPosition.isNotNull() && currentPosition != startPosition) {
-                contextBefore = plainTextForContext(makeSimpleRange(currentPosition, startPosition));
-                if (atBoundaryOfGranularity(currentPosition, TextGranularity::ParagraphGranularity, SelectionDirection::Backward))
+            if (contextStartPosition.isNotNull() && contextStartPosition != startPosition) {
+                contextBefore = plainTextForContext(makeSimpleRange(contextStartPosition, startPosition));
+                if (atBoundaryOfGranularity(contextStartPosition, TextGranularity::ParagraphGranularity, SelectionDirection::Backward) && firstPositionInEditableContent != contextStartPosition)
                     contextBefore = makeString("\n "_s, contextBefore);
             }

trunk/Tools/ChangeLog

@@ +1 @@
+2021-09-09  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Mac Catalyst] Korean characters do not combine correctly in editable fields in WKWebView
+        https://bugs.webkit.org/show_bug.cgi?id=230122
+        rdar://79062204
+
+        Reviewed by Devin Rousso.
+
+        Add a new API test to exercise the change. See WebKit/ChangeLog for more information.
+
+        * TestWebKitAPI/Tests/ios/AutocorrectionTestsIOS.mm:
+        (-[TestWKWebView synchronouslyRequestAutocorrectionContext]):
+        (TEST):
+        * TestWebKitAPI/ios/UIKitSPI.h:
+
 2021-09-09  Chris Dumez  <cdumez@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/ios/AutocorrectionTestsIOS.mm

@@ -37 +37 @@
 @interface TestWKWebView (AutocorrectionTests)
 - (UIWKAutocorrectionRects *)autocorrectionRectsForString:(NSString *)string;
+- (UIWKAutocorrectionContext *)synchronouslyRequestAutocorrectionContext;
 @end
 
@@ -47 +48 @@
     [self.textInputContentView requestAutocorrectionRectsForString:string withCompletionHandler:[&] (UIWKAutocorrectionRects *rects) {
         result = rects;
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+    return result.autorelease();
+}
+
+- (UIWKAutocorrectionContext *)synchronouslyRequestAutocorrectionContext
+{
+    RetainPtr<UIWKAutocorrectionContext> result;
+    bool done = false;
+    [self.textInputContentView requestAutocorrectionContextWithCompletionHandler:[&] (UIWKAutocorrectionContext *context) {
+        result = context;
         done = true;
     }];
@@ -105 +118 @@
     [webView _close];
 
-    bool done = false;
-    [contentView requestAutocorrectionContextWithCompletionHandler:[&] (UIWKAutocorrectionContext *) {
-        done = true;
-    }];
-    TestWebKitAPI::Util::run(&done);
+    // This test just verifies that attempting to request an autocorrection context in this state
+    // does not trigger a crash or hang, and also still invokes the given completion handler.
+    [webView synchronouslyRequestAutocorrectionContext];
+}
+
+TEST(AutocorrectionTests, AutocorrectionContextDoesNotIncludeNewlineInTextField)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] init]);
+    [webView synchronouslyLoadTestPageNamed:@"autofocused-text-input"];
+
+    RetainPtr contextBeforeTyping = [webView synchronouslyRequestAutocorrectionContext];
+    EXPECT_EQ(0U, [contextBeforeTyping contextBeforeSelection].length);
+    EXPECT_EQ(0U, [contextBeforeTyping selectedText].length);
+    EXPECT_EQ(0U, [contextBeforeTyping contextAfterSelection].length);
+
+    [[webView textInputContentView] insertText:@"a"];
+    [webView waitForNextPresentationUpdate];
+
+    RetainPtr contextAfterTyping = [webView synchronouslyRequestAutocorrectionContext];
+    EXPECT_WK_STREQ(@"a", [contextAfterTyping contextBeforeSelection]);
+    EXPECT_EQ(0U, [contextAfterTyping selectedText].length);
+    EXPECT_EQ(0U, [contextAfterTyping contextAfterSelection].length);
 }
 

trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h

@@ -184 +184 @@
 
 @interface UIWKAutocorrectionContext : NSObject
+@property (nonatomic, copy) NSString *contextBeforeSelection;
+@property (nonatomic, copy) NSString *selectedText;
+@property (nonatomic, copy) NSString *contextAfterSelection;
 @end