Skip to content

Previously cleared text reappears when typing in Korean after tapping ⨯ in Google search field#38390

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
whsieh:eng/285175
Dec 27, 2024
Merged

Previously cleared text reappears when typing in Korean after tapping ⨯ in Google search field#38390
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
whsieh:eng/285175

Conversation

@whsieh
Copy link
Member

@whsieh whsieh commented Dec 27, 2024

296ab29

Previously cleared text reappears when typing in Korean after tapping ⨯ in Google search field
https://bugs.webkit.org/show_bug.cgi?id=285175
rdar://131897908

Reviewed by Richard Robinson.

When using Korean keyboards to search on Google (typing in a `textarea` element) on Google on iOS,
if you:

(1) Type some Korean text (stopping in the middle of a composed word).
(2) Tap the ⨯ button, which programmatically clears the text field.
(3) Continue typing on Korean.

...text that was previously typed and then cleared during (2) will reappear. This happens because
there's no mechanism to invalidate and update the keyboard's text entry context when text content
changes programmatically, so the stored input context ends up being reinserted.

To fix this, we add a heuristic to respond to programmatic changes in text content that cause the
value of focused text form controls to become empty, and `-invalidateTextEntryContextForTextInput:`
in the UI process. While we could potentially apply this treatment to *all* programmatic value
changes, this poses a more significant web compat and performance risk.

* LayoutTests/editing/input/ios/invalidate-text-entry-context-when-clearing-text-area-expected.txt: Added.
* LayoutTests/editing/input/ios/invalidate-text-entry-context-when-clearing-text-area.html: Added.

Add a layout test to exercise this change, by teaching the test runner to count the number of times
`-[_UIKeyboardStateManager updateForChangedSelection]` is invoked and verifying that clearing the
text in a `textarea` calls into this method.

* LayoutTests/resources/ui-helper.js:
(window.UIHelper.async keyboardUpdateForChangedSelectionCount):
(window.UIHelper):
* Source/WebCore/html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::setValue):

Call the new client hook below, when the value is set to the empty string from bindings.

* Source/WebCore/html/HTMLTextAreaElement.cpp:
(WebCore::HTMLTextAreaElement::setValueCommon):
* Source/WebCore/page/ChromeClient.h:
(WebCore::ChromeClient::didProgrammaticallyClearTextFormControl):

Add a client hook to notify `WebPage` when a text form control's text value is cleared.

* Source/WebKit/UIProcess/PageClient.h:
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.messages.in:
* Source/WebKit/UIProcess/ios/PageClientImplIOS.h:
* Source/WebKit/UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::didProgrammaticallyClearFocusedElement):
* Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:
* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _updateInputContextAfterBlurringAndRefocusingElement]):
(-[WKContentView _didProgrammaticallyClearFocusedElement:]):
(-[WKContentView _internalInvalidateTextEntryContext]):
* Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::didProgrammaticallyClearFocusedElement):
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::didProgrammaticallyClearTextFormControl):
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::contextForElement const):

Make this method const, so that we can pass in the form control element below.

* Source/WebKit/WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::didProgrammaticallyClearTextFormControl):
* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::didProgrammaticallyClearTextFormControl):

Notify the UI process when the focused form control value is cleared. The IPC message is sent on the
next runloop, so that we'll still invalidate the text entry context in the case where a focused
element is blurred and immediately refocused.

* Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
* Tools/TestRunnerShared/UIScriptContext/UIScriptController.h:
(WTR::UIScriptController::keyboardUpdateForChangedSelectionCount const):
* Tools/WebKitTestRunner/TestController.h:
* Tools/WebKitTestRunner/ios/TestControllerIOS.mm:
(-[NSObject swizzled_updateForChangedSelection]):
(WTR::TestController::platformInitialize):
(WTR::TestController::keyboardUpdateForChangedSelectionCount const):
* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h:
* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptControllerIOS::keyboardUpdateForChangedSelectionCount const):

Add test infrastructure to keep track of the number of times `-updateForChangedSelection` is
invoked; also expose a new script controller hook to request this count from layout tests.

Canonical link: https://commits.webkit.org/288313@main

e03c81a

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl 🧪 ios-wk2 🧪 api-mac ✅ 🧪 api-wpe
🧪 ios-wk2-wpt 🧪 mac-wk1 ✅ 🛠 wpe-cairo
🧪 api-ios 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@whsieh whsieh self-assigned this Dec 27, 2024
@whsieh whsieh added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Dec 27, 2024
@rr-codes
Copy link
Contributor

"the number of types" is this supposed to say "the number of times"?

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 27, 2024
@whsieh whsieh removed the merging-blocked Applied to prevent a change from being merged label Dec 27, 2024
@whsieh whsieh changed the title [iOS] Previously cleared text reappears when typing in Korean after tapping ⨯ in Google search field Previously cleared text reappears when typing in Korean after tapping ⨯ in Google search field Dec 27, 2024
@whsieh whsieh added the merge-queue Applied to send a pull request to merge-queue label Dec 27, 2024
@whsieh
Copy link
Member Author

whsieh commented Dec 27, 2024

Thanks for the review!

… ⨯ in Google search field

https://bugs.webkit.org/show_bug.cgi?id=285175
rdar://131897908

Reviewed by Richard Robinson.

When using Korean keyboards to search on Google (typing in a `textarea` element) on Google on iOS,
if you:

(1) Type some Korean text (stopping in the middle of a composed word).
(2) Tap the ⨯ button, which programmatically clears the text field.
(3) Continue typing on Korean.

...text that was previously typed and then cleared during (2) will reappear. This happens because
there's no mechanism to invalidate and update the keyboard's text entry context when text content
changes programmatically, so the stored input context ends up being reinserted.

To fix this, we add a heuristic to respond to programmatic changes in text content that cause the
value of focused text form controls to become empty, and `-invalidateTextEntryContextForTextInput:`
in the UI process. While we could potentially apply this treatment to *all* programmatic value
changes, this poses a more significant web compat and performance risk.

* LayoutTests/editing/input/ios/invalidate-text-entry-context-when-clearing-text-area-expected.txt: Added.
* LayoutTests/editing/input/ios/invalidate-text-entry-context-when-clearing-text-area.html: Added.

Add a layout test to exercise this change, by teaching the test runner to count the number of times
`-[_UIKeyboardStateManager updateForChangedSelection]` is invoked and verifying that clearing the
text in a `textarea` calls into this method.

* LayoutTests/resources/ui-helper.js:
(window.UIHelper.async keyboardUpdateForChangedSelectionCount):
(window.UIHelper):
* Source/WebCore/html/HTMLInputElement.cpp:
(WebCore::HTMLInputElement::setValue):

Call the new client hook below, when the value is set to the empty string from bindings.

* Source/WebCore/html/HTMLTextAreaElement.cpp:
(WebCore::HTMLTextAreaElement::setValueCommon):
* Source/WebCore/page/ChromeClient.h:
(WebCore::ChromeClient::didProgrammaticallyClearTextFormControl):

Add a client hook to notify `WebPage` when a text form control's text value is cleared.

* Source/WebKit/UIProcess/PageClient.h:
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.messages.in:
* Source/WebKit/UIProcess/ios/PageClientImplIOS.h:
* Source/WebKit/UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::didProgrammaticallyClearFocusedElement):
* Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:
* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView _updateInputContextAfterBlurringAndRefocusingElement]):
(-[WKContentView _didProgrammaticallyClearFocusedElement:]):
(-[WKContentView _internalInvalidateTextEntryContext]):
* Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::didProgrammaticallyClearFocusedElement):
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::didProgrammaticallyClearTextFormControl):
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h:
* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::contextForElement const):

Make this method const, so that we can pass in the form control element below.

* Source/WebKit/WebProcess/WebPage/WebPage.h:
(WebKit::WebPage::didProgrammaticallyClearTextFormControl):
* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::didProgrammaticallyClearTextFormControl):

Notify the UI process when the focused form control value is cleared. The IPC message is sent on the
next runloop, so that we'll still invalidate the text entry context in the case where a focused
element is blurred and immediately refocused.

* Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
* Tools/TestRunnerShared/UIScriptContext/UIScriptController.h:
(WTR::UIScriptController::keyboardUpdateForChangedSelectionCount const):
* Tools/WebKitTestRunner/TestController.h:
* Tools/WebKitTestRunner/ios/TestControllerIOS.mm:
(-[NSObject swizzled_updateForChangedSelection]):
(WTR::TestController::platformInitialize):
(WTR::TestController::keyboardUpdateForChangedSelectionCount const):
* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h:
* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptControllerIOS::keyboardUpdateForChangedSelectionCount const):

Add test infrastructure to keep track of the number of times `-updateForChangedSelection` is
invoked; also expose a new script controller hook to request this count from layout tests.

Canonical link: https://commits.webkit.org/288313@main
@webkit-commit-queue
Copy link
Collaborator

Committed 288313@main (296ab29): https://commits.webkit.org/288313@main

Reviewed commits have been landed. Closing PR #38390 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit 296ab29 into WebKit:main Dec 27, 2024
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Dec 27, 2024
@whsieh whsieh deleted the eng/285175 branch December 27, 2024 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

New Bugs Unclassified bugs are placed in this component until the correct component can be determined.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants