Title: [282247] trunk
Revision
282247
Author
[email protected]
Date
2021-09-09 17:15:44 -0700 (Thu, 09 Sep 2021)

Log 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:

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (282246 => 282247)


--- trunk/Source/WebKit/ChangeLog	2021-09-10 00:01:20 UTC (rev 282246)
+++ trunk/Source/WebKit/ChangeLog	2021-09-10 00:15:44 UTC (rev 282247)
@@ -1,3 +1,30 @@
+2021-09-09  Wenson Hsieh  <[email protected]>
+
+        [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  <[email protected]>
 
         Enable IPCMessages debug logging for non-Cocoa ports

Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (282246 => 282247)


--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2021-09-10 00:01:20 UTC (rev 282246)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2021-09-10 00:15:44 UTC (rev 282247)
@@ -2500,7 +2500,7 @@
     VisiblePosition startPosition = frame->selection().selection().start();
     VisiblePosition endPosition = frame->selection().selection().end();
     const unsigned minContextWordCount = 3;
-    const unsigned minContextLenght = 12;
+    const unsigned minContextLength = 12;
     const unsigned maxContextLength = 30;
 
     if (frame->selection().isRange())
@@ -2515,25 +2515,26 @@
             markedTextRange.length = selectedText.length();
         }
     } 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);
             }
         }

Modified: trunk/Tools/ChangeLog (282246 => 282247)


--- trunk/Tools/ChangeLog	2021-09-10 00:01:20 UTC (rev 282246)
+++ trunk/Tools/ChangeLog	2021-09-10 00:15:44 UTC (rev 282247)
@@ -1,3 +1,18 @@
+2021-09-09  Wenson Hsieh  <[email protected]>
+
+        [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  <[email protected]>
 
         Implement a WebProcess cap

Modified: trunk/Tools/TestWebKitAPI/Tests/ios/AutocorrectionTestsIOS.mm (282246 => 282247)


--- trunk/Tools/TestWebKitAPI/Tests/ios/AutocorrectionTestsIOS.mm	2021-09-10 00:01:20 UTC (rev 282246)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/AutocorrectionTestsIOS.mm	2021-09-10 00:15:44 UTC (rev 282247)
@@ -36,6 +36,7 @@
 
 @interface TestWKWebView (AutocorrectionTests)
 - (UIWKAutocorrectionRects *)autocorrectionRectsForString:(NSString *)string;
+- (UIWKAutocorrectionContext *)synchronouslyRequestAutocorrectionContext;
 @end
 
 @implementation TestWKWebView (AutocorrectionTests)
@@ -52,6 +53,18 @@
     return result.autorelease();
 }
 
+- (UIWKAutocorrectionContext *)synchronouslyRequestAutocorrectionContext
+{
+    RetainPtr<UIWKAutocorrectionContext> result;
+    bool done = false;
+    [self.textInputContentView requestAutocorrectionContextWithCompletionHandler:[&] (UIWKAutocorrectionContext *context) {
+        result = context;
+        done = true;
+    }];
+    TestWebKitAPI::Util::run(&done);
+    return result.autorelease();
+}
+
 @end
 
 static void checkCGRectIsEqualToCGRectWithLogging(CGRect expected, CGRect observed)
@@ -104,11 +117,28 @@
     [contentView resignFirstResponder];
     [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);
+}
+
 #endif // PLATFORM(IOS_FAMILY)

Modified: trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h (282246 => 282247)


--- trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h	2021-09-10 00:01:20 UTC (rev 282246)
+++ trunk/Tools/TestWebKitAPI/ios/UIKitSPI.h	2021-09-10 00:15:44 UTC (rev 282247)
@@ -183,6 +183,9 @@
 @end
 
 @interface UIWKAutocorrectionContext : NSObject
+@property (nonatomic, copy) NSString *contextBeforeSelection;
+@property (nonatomic, copy) NSString *selectedText;
+@property (nonatomic, copy) NSString *contextAfterSelection;
 @end
 
 @protocol UIWebFormAccessoryDelegate
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to