Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[UIAsyncTextInput] Keyboard sometimes autocapitalizes incorrectly while typing an inline completion #21244

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

whsieh
Copy link
Member

@whsieh whsieh commented Dec 3, 2023

01a600b

[UIAsyncTextInput] Keyboard sometimes autocapitalizes incorrectly while typing an inline completion
https://bugs.webkit.org/show_bug.cgi?id=265764
rdar://119084774

Reviewed by Richard Robinson.

After adopting `-invalidateTextEntryContext` in place of `-[UIKeyboardImpl layoutHasChanged]` when
the async text input codepath is enabled, the keyboard sometimes unexpectedly autocapitalizes when
typing while showing inline predictions in Mail compose.

This is because the replacement API (`-invalidateTextEntryContext`) now additionally updates text
context in addition to updating the layout and positioning of input method UI (i.e. when using
Chinese or Japanese input). This is meant to only be done only once, when initially starting IME
composition, in order to give certain web apps (e.g. Google Docs) a chance to reposition any
offscreen, hidden editable containers such that the input method UI and candidates bar shows up in
the right place — to achieve this, we set a `_candidateViewNeedsUpdate` flag upon setting any marked
text where the marked text range is previously empty, and clear out the flag after the next
selection change.

In iOS 17+, inline predictions also exercise this same marked text codepath; however, instead of
maintaining a persistent marked text string, inline predictions continuously clear out and reinsert
marked text as the prediction is regenerated while typing, which causes the
`_candidateViewNeedsUpdate` flag to be continuously set (and therefore, causes us to continuously
invoke `-invalidateTextEntryContext` while typing).

The combination of the above causes the keyboard to occasionally lose context while typing, which
results in the keyboard becoming erroneously autoshifted after typing a space. To fix this, we make
this logic more nuanced, such that we only `-invalidateTextEntryContext` if there's an active IME
session (excluding marked text, set by inline predictions).

Test: editing/input/ios/typing-with-inline-predictions.html

* LayoutTests/editing/input/ios/typing-with-inline-predictions-expected.txt: Added.
* LayoutTests/editing/input/ios/typing-with-inline-predictions.html: Added.

Add a layout test to exercise the change.

* Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:

Introduce a new flag, `_isDeferringKeyEventsToInputMethod`, to track whether or not we have an
active IME session.

* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView cleanUpInteraction]):
(-[WKContentView canPerformActionForWebView:withSender:]):

Drive-by fix: replace some internal calls to `-hasContent` with an internal version instead,
`-_hasContent`, to avoid self-induced release assertions after 271417@main.

(-[WKContentView _setMarkedText:underlines:highlights:selectedRange:]):
(-[WKContentView unmarkText]):

Unset the new flag when committing marked text.

(-[WKContentView _internalHandleKeyWebEvent:withCompletionHandler:]):

Set the new flag, `_isDeferringKeyEventsToInputMethod`, if we've actually deferred key event
handling to system IME.

(-[WKContentView hasContent]):
(-[WKContentView _hasContent]):

See drive-by fix above.

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

46f5cf0

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 gtk
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 api-gtk
✅ 🛠 tv-sim ✅ 🧪 mac-wk2-stress
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@whsieh whsieh self-assigned this Dec 3, 2023
@whsieh whsieh added the HTML Editing For bugs in HTML editing support (including designMode and contentEditable). label Dec 3, 2023
@whsieh whsieh marked this pull request as ready for review December 4, 2023 06:47
@whsieh whsieh added the merge-queue Applied to send a pull request to merge-queue label Dec 4, 2023
@whsieh
Copy link
Member Author

whsieh commented Dec 4, 2023

Thanks for the review!

…le typing an inline completion

https://bugs.webkit.org/show_bug.cgi?id=265764
rdar://119084774

Reviewed by Richard Robinson.

After adopting `-invalidateTextEntryContext` in place of `-[UIKeyboardImpl layoutHasChanged]` when
the async text input codepath is enabled, the keyboard sometimes unexpectedly autocapitalizes when
typing while showing inline predictions in Mail compose.

This is because the replacement API (`-invalidateTextEntryContext`) now additionally updates text
context in addition to updating the layout and positioning of input method UI (i.e. when using
Chinese or Japanese input). This is meant to only be done only once, when initially starting IME
composition, in order to give certain web apps (e.g. Google Docs) a chance to reposition any
offscreen, hidden editable containers such that the input method UI and candidates bar shows up in
the right place — to achieve this, we set a `_candidateViewNeedsUpdate` flag upon setting any marked
text where the marked text range is previously empty, and clear out the flag after the next
selection change.

In iOS 17+, inline predictions also exercise this same marked text codepath; however, instead of
maintaining a persistent marked text string, inline predictions continuously clear out and reinsert
marked text as the prediction is regenerated while typing, which causes the
`_candidateViewNeedsUpdate` flag to be continuously set (and therefore, causes us to continuously
invoke `-invalidateTextEntryContext` while typing).

The combination of the above causes the keyboard to occasionally lose context while typing, which
results in the keyboard becoming erroneously autoshifted after typing a space. To fix this, we make
this logic more nuanced, such that we only `-invalidateTextEntryContext` if there's an active IME
session (excluding marked text, set by inline predictions).

Test: editing/input/ios/typing-with-inline-predictions.html

* LayoutTests/editing/input/ios/typing-with-inline-predictions-expected.txt: Added.
* LayoutTests/editing/input/ios/typing-with-inline-predictions.html: Added.

Add a layout test to exercise the change.

* Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:

Introduce a new flag, `_isDeferringKeyEventsToInputMethod`, to track whether or not we have an
active IME session.

* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView cleanUpInteraction]):
(-[WKContentView canPerformActionForWebView:withSender:]):

Drive-by fix: replace some internal calls to `-hasContent` with an internal version instead,
`-_hasContent`, to avoid self-induced release assertions after 271417@main.

(-[WKContentView _setMarkedText:underlines:highlights:selectedRange:]):
(-[WKContentView unmarkText]):

Unset the new flag when committing marked text.

(-[WKContentView _internalHandleKeyWebEvent:withCompletionHandler:]):

Set the new flag, `_isDeferringKeyEventsToInputMethod`, if we've actually deferred key event
handling to system IME.

(-[WKContentView hasContent]):
(-[WKContentView _hasContent]):

See drive-by fix above.

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

Committed 271482@main (01a600b): https://commits.webkit.org/271482@main

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

@webkit-commit-queue webkit-commit-queue merged commit 01a600b into WebKit:main Dec 4, 2023
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Dec 4, 2023
Copy link
Contributor

@megangardner megangardner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r+

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
HTML Editing For bugs in HTML editing support (including designMode and contentEditable).
Projects
None yet
5 participants