[webkit-changes] [264112] trunk

Wed, 08 Jul 2020 10:10:13 -0700

Title: [264112] trunk
Revision
264112
Author
[email protected]
Date
2020-07-08 10:08:22 -0700 (Wed, 08 Jul 2020)

Log Message

autocapitalize="words" capitalizes every word's second character
https://bugs.webkit.org/show_bug.cgi?id=148504
<rdar://problem/57814304>

Reviewed by Tim Horton.

Source/WebKit:

This bug resurfaced in iOS 13 due to timing changes that caused a task added to UIKeyboardTaskQueue to no longer
get dequeued after the next editor state update is received in the UI process; as a result, UIKit asks us for
the context before the caret selection (i.e. `characterBeforeSelection`) before we have a chance to update it,
which means that UIKeyboardImpl's autoshift state will always be off by one character whie typing.

Note that solely deferring this update until after the next editor state update is received is insufficient to
fix this bug, since the keyboard's shift state will still be incorrect when typing characters very quickly (or
if the web process is unresponsive).

Instead, completely address this bug by adding a mechanism for WKContentView to cache the last character in the
string that is currently being inserted using `-insertText:`, and return it in `-_characterBeforeCaretSelection`
until the next editor state update comes in with the real character before the selection. This way, we're able
to respond to UIKit's requests immediately with a (reasonable) guess of what the character before the selection
should be, and then follow it up with the actual up-to-date value from the web process during the next editor
state update (invalidating the stale shift state if necessary). This cached last character should only be
incorrect in the case where the page uses script to influence what was typed, or the typed character was never
inserted at all (e.g. typing a newline character in a single line input element).

Test: fast/forms/ios/autocapitalize-words.html

* Platform/spi/ios/UIKitSPI.h:
* UIProcess/ios/WKContentViewInteraction.h:
* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView cleanUpInteraction]):
(-[WKContentView _characterBeforeCaretSelection]):

Use `_lastInsertedCharacterToOverrideCharacterBeforeSelection`, if it is set.

(-[WKContentView _characterInRelationToCaretSelection:]):
(-[WKContentView insertText:]):

Update `_lastInsertedCharacterToOverrideCharacterBeforeSelection` and ensure an editor state update immediately
after text is inserted, if we're currently editing an element with `autocapitalize="words"`. The editor state
update could be unnecessarily expensive, so we don't want to do this unconditionally. Also,
`autocapitalize="words"` is special in the sense that it is the only autocapitalization type where the platform
(UIKit) will ask for the last character immediately and use it to unshift the keyboard. Even in the case of
`autocapitalize="sentences"`, UIKit makes the decision to unshift using information from `kbd` instead.

(-[WKContentView _elementDidBlur]):
(-[WKContentView _selectionChanged]):

Clear out `_lastInsertedCharacterToOverrideCharacterBeforeSelection` when the next editor state update is
received, and invalidate the current keyboard shift state by calling `-clearShiftState`. This SPI method
automatically schedules a shift state update, as well.

Note the extra `_usingGestureForSelection` check here, which prevents the keyboard from autoshifting and
unshifting while changing the selection via loupe gesture.

LayoutTests:

Add a new layout test to verify that the bug does not occur.

* fast/forms/ios/autocapitalize-words-expected.txt: Added.
* fast/forms/ios/autocapitalize-words.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (264111 => 264112)


--- trunk/LayoutTests/ChangeLog	2020-07-08 17:03:18 UTC (rev 264111)
+++ trunk/LayoutTests/ChangeLog	2020-07-08 17:08:22 UTC (rev 264112)
@@ -1,3 +1,16 @@
+2020-07-08  Wenson Hsieh  <[email protected]>
+
+        autocapitalize="words" capitalizes every word's second character
+        https://bugs.webkit.org/show_bug.cgi?id=148504
+        <rdar://problem/57814304>
+
+        Reviewed by Tim Horton.
+
+        Add a new layout test to verify that the bug does not occur.
+
+        * fast/forms/ios/autocapitalize-words-expected.txt: Added.
+        * fast/forms/ios/autocapitalize-words.html: Added.
+
 2020-06-29  Jer Noble  <[email protected]>
 
         Unreviewed test gardening; "rebaseline" the video-orientation-canvas.html test to center the expected red color

Added: trunk/LayoutTests/fast/forms/ios/autocapitalize-words-expected.txt (0 => 264112)


--- trunk/LayoutTests/fast/forms/ios/autocapitalize-words-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/ios/autocapitalize-words-expected.txt	2020-07-08 17:08:22 UTC (rev 264112)
@@ -0,0 +1,11 @@
+This test verifies that typing in editable areas with autocapitalize='words' causes every word to be capitalized. To test manually, type into both elements.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS input.value is "Do Re"
+PASS contenteditable.innerText is "Mi Fa\nSo"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/forms/ios/autocapitalize-words.html (0 => 264112)


--- trunk/LayoutTests/fast/forms/ios/autocapitalize-words.html	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/ios/autocapitalize-words.html	2020-07-08 17:08:22 UTC (rev 264112)
@@ -0,0 +1,57 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<head>
+<script src=""
+<script src=""
+<style>
+input, div[contenteditable] {
+    width: 100px;
+    height: 50px;
+    border: 1px solid black;
+    font-size: 20px;
+}
+
+#console, #description {
+    font-size: small;
+}
+</style>
+<script>
+jsTestIsAsync = true;
+description("This test verifies that typing in editable areas with autocapitalize='words' causes every word to be capitalized. To test manually, type into both elements.");
+
+async function typeStringInElement(element, string)
+{
+    await UIHelper.activateElementAndWaitForInputSession(element);
+    for (const character of [...string])
+        await UIHelper.callFunctionAndWaitForEvent(() => UIHelper.typeCharacter(character), element, "input");
+    element.blur();
+    await UIHelper.waitForKeyboardToHide();
+}
+
+addEventListener("load", async () => {
+    if (!window.testRunner)
+        return;
+
+    input = document.querySelector("input");
+    contenteditable = document.querySelector("div[contenteditable]");
+
+    await typeStringInElement(input, "do re");
+    await typeStringInElement(contenteditable, "fa\nso");
+
+    shouldBeEqualToString("input.value", "Do Re");
+    shouldBeEqualToString("contenteditable.innerText", "Mi Fa\nSo");
+
+    input.remove();
+    contenteditable.remove();
+    finishJSTest();
+});
+</script>
+</head>
+<body>
+<div id="console"></div>
+<div id="description"></div>
+<input autocapitalize="words">
+<div contenteditable autocapitalize="words">Mi&nbsp;</div>
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebKit/ChangeLog (264111 => 264112)


--- trunk/Source/WebKit/ChangeLog	2020-07-08 17:03:18 UTC (rev 264111)
+++ trunk/Source/WebKit/ChangeLog	2020-07-08 17:08:22 UTC (rev 264112)
@@ -1,3 +1,59 @@
+2020-07-08  Wenson Hsieh  <[email protected]>
+
+        autocapitalize="words" capitalizes every word's second character
+        https://bugs.webkit.org/show_bug.cgi?id=148504
+        <rdar://problem/57814304>
+
+        Reviewed by Tim Horton.
+
+        This bug resurfaced in iOS 13 due to timing changes that caused a task added to UIKeyboardTaskQueue to no longer
+        get dequeued after the next editor state update is received in the UI process; as a result, UIKit asks us for
+        the context before the caret selection (i.e. `characterBeforeSelection`) before we have a chance to update it,
+        which means that UIKeyboardImpl's autoshift state will always be off by one character whie typing.
+
+        Note that solely deferring this update until after the next editor state update is received is insufficient to
+        fix this bug, since the keyboard's shift state will still be incorrect when typing characters very quickly (or
+        if the web process is unresponsive).
+
+        Instead, completely address this bug by adding a mechanism for WKContentView to cache the last character in the
+        string that is currently being inserted using `-insertText:`, and return it in `-_characterBeforeCaretSelection`
+        until the next editor state update comes in with the real character before the selection. This way, we're able
+        to respond to UIKit's requests immediately with a (reasonable) guess of what the character before the selection
+        should be, and then follow it up with the actual up-to-date value from the web process during the next editor
+        state update (invalidating the stale shift state if necessary). This cached last character should only be
+        incorrect in the case where the page uses script to influence what was typed, or the typed character was never
+        inserted at all (e.g. typing a newline character in a single line input element).
+
+        Test: fast/forms/ios/autocapitalize-words.html
+
+        * Platform/spi/ios/UIKitSPI.h:
+        * UIProcess/ios/WKContentViewInteraction.h:
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView cleanUpInteraction]):
+        (-[WKContentView _characterBeforeCaretSelection]):
+
+        Use `_lastInsertedCharacterToOverrideCharacterBeforeSelection`, if it is set.
+
+        (-[WKContentView _characterInRelationToCaretSelection:]):
+        (-[WKContentView insertText:]):
+
+        Update `_lastInsertedCharacterToOverrideCharacterBeforeSelection` and ensure an editor state update immediately
+        after text is inserted, if we're currently editing an element with `autocapitalize="words"`. The editor state
+        update could be unnecessarily expensive, so we don't want to do this unconditionally. Also,
+        `autocapitalize="words"` is special in the sense that it is the only autocapitalization type where the platform
+        (UIKit) will ask for the last character immediately and use it to unshift the keyboard. Even in the case of
+        `autocapitalize="sentences"`, UIKit makes the decision to unshift using information from `kbd` instead.
+
+        (-[WKContentView _elementDidBlur]):
+        (-[WKContentView _selectionChanged]):
+
+        Clear out `_lastInsertedCharacterToOverrideCharacterBeforeSelection` when the next editor state update is
+        received, and invalidate the current keyboard shift state by calling `-clearShiftState`. This SPI method
+        automatically schedules a shift state update, as well.
+
+        Note the extra `_usingGestureForSelection` check here, which prevents the keyboard from autoshifting and
+        unshifting while changing the selection via loupe gesture.
+
 2020-07-07  Antoine Quint  <[email protected]>
 
         [iOS] Sharing an <img> element with a base64-encoded URL shares the URL as raw text instead of an image

Modified: trunk/Source/WebKit/Platform/spi/ios/UIKitSPI.h (264111 => 264112)


--- trunk/Source/WebKit/Platform/spi/ios/UIKitSPI.h	2020-07-08 17:03:18 UTC (rev 264111)
+++ trunk/Source/WebKit/Platform/spi/ios/UIKitSPI.h	2020-07-08 17:08:22 UTC (rev 264112)
@@ -318,6 +318,7 @@
 - (void)addInputString:(NSString *)string withFlags:(NSUInteger)flags withInputManagerHint:(NSString *)hint;
 - (BOOL)autocorrectSpellingEnabled;
 - (BOOL)isAutoShifted;
+- (void)clearShiftState;
 - (void)deleteFromInput;
 - (void)deleteFromInputWithFlags:(NSUInteger)flags;
 - (void)replaceText:(id)replacement;

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (264111 => 264112)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h	2020-07-08 17:03:18 UTC (rev 264111)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h	2020-07-08 17:08:22 UTC (rev 264112)
@@ -390,6 +390,8 @@
 
     RetainPtr<NSDictionary> _additionalContextForStrongPasswordAssistance;
 
+    Optional<UChar32> _lastInsertedCharacterToOverrideCharacterBeforeSelection;
+
 #if ENABLE(DATA_INTERACTION)
     WebKit::DragDropInteractionState _dragDropInteractionState;
     RetainPtr<UIDragInteraction> _dragInteraction;

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (264111 => 264112)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2020-07-08 17:03:18 UTC (rev 264111)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2020-07-08 17:08:22 UTC (rev 264112)
@@ -941,6 +941,8 @@
         _interactionViewsContainerView = nil;
     }
 
+    _lastInsertedCharacterToOverrideCharacterBeforeSelection = WTF::nullopt;
+
     [_touchEventGestureRecognizer setDelegate:nil];
     [self removeGestureRecognizer:_touchEventGestureRecognizer.get()];
 
@@ -4135,7 +4137,7 @@
 
 - (UTF32Char)_characterBeforeCaretSelection
 {
-    return _page->editorState().postLayoutData().characterBeforeSelection;
+    return _lastInsertedCharacterToOverrideCharacterBeforeSelection.valueOr(_page->editorState().postLayoutData().characterBeforeSelection);
 }
 
 - (UTF32Char)_characterInRelationToCaretSelection:(int)amount
@@ -4144,7 +4146,7 @@
     case 0:
         return _page->editorState().postLayoutData().characterAfterSelection;
     case -1:
-        return _page->editorState().postLayoutData().characterBeforeSelection;
+        return self._characterBeforeCaretSelection;
     case -2:
         return _page->editorState().postLayoutData().twoCharacterBeforeSelection;
     default:
@@ -4826,6 +4828,11 @@
     options.processingUserGesture = [keyboard respondsToSelector:@selector(isCallingInputDelegate)] && keyboard.isCallingInputDelegate;
     options.shouldSimulateKeyboardInput = [self _shouldSimulateKeyboardInputOnTextInsertion];
     _page->insertTextAsync(aStringValue, WebKit::EditingRange(), WTFMove(options));
+
+    if (_focusedElementInformation.autocapitalizeType == WebCore::AutocapitalizeType::Words && aStringValue.length) {
+        _lastInsertedCharacterToOverrideCharacterBeforeSelection = [aStringValue characterAtIndex:aStringValue.length - 1];
+        _page->scheduleFullEditorStateUpdate();
+    }
 }
 
 - (void)insertText:(NSString *)aStringValue alternatives:(NSArray<NSString *> *)alternatives style:(UITextAlternativeStyle)style
@@ -6073,6 +6080,8 @@
 
     if (!_isChangingFocus)
         _didAccessoryTabInitiateFocus = NO;
+
+    _lastInsertedCharacterToOverrideCharacterBeforeSelection = WTF::nullopt;
 }
 
 - (void)_updateInputContextAfterBlurringAndRefocusingElement
@@ -6552,6 +6561,12 @@
 #endif
     
     [_webView _didChangeEditorState];
+
+    if (!_page->editorState().isMissingPostLayoutData) {
+        _lastInsertedCharacterToOverrideCharacterBeforeSelection = WTF::nullopt;
+        if (!_usingGestureForSelection && _focusedElementInformation.autocapitalizeType == WebCore::AutocapitalizeType::Words)
+            [UIKeyboardImpl.sharedInstance clearShiftState];
+    }
 }
 
 - (void)selectWordForReplacement

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to