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] Release assertion after selecting "Translate" when async text input is enabled #21578

Merged
merged 1 commit into from
Dec 10, 2023

Conversation

whsieh
Copy link
Member

@whsieh whsieh commented Dec 9, 2023

161dbc0

[UIAsyncTextInput] Release assertion after selecting "Translate" when async text input is enabled
https://bugs.webkit.org/show_bug.cgi?id=266157
rdar://119434259

Reviewed by Megan Gardner.

After the UIKit changes in rdar://119283621, the default commands corresponding to these 3 actions:

```
_define:     define:
_translate:  translate:
_lookup:     lookup:
```

...now follow an inverted model for dispatching fallback actions, where the primary action selector
is the legacy private action, and the fallback selector is the new selector on `UIAsyncTextInput`.
This means that UIKit ends up calling into private selectors on the content view instead of their
non-underscore-prefixed counterparts even when `UIKit/async_text_input` is enabled, causing us to
hit release assertions under these three methods.

To avoid this, we adjust the default behavior of `-targetForActionForWebView:withSender:` to always
return `nil` for these three private selectors, in order to force UIKit to fall back to calling into
the new action selectors.

Changes covered by the layout test `editing/selection/ios/look-up-selected-text.html`, when async
text input is enabled.

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

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

a85fbe3

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 βœ… πŸ›  gtk
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk2   πŸ§ͺ gtk-wk2
βœ… πŸ›  tv   πŸ§ͺ mac-AS-debug-wk2   πŸ§ͺ api-gtk
βœ… πŸ›  tv-sim
βœ… πŸ›  watch
βœ… πŸ›  πŸ§ͺ unsafe-merge βœ… πŸ›  watch-sim

@whsieh whsieh requested a review from cdumez as a code owner December 9, 2023 23:32
@whsieh whsieh self-assigned this Dec 9, 2023
@whsieh whsieh added the Platform Portability improvements and other general platform improvements not driven directly by site bugs. label Dec 9, 2023
Copy link
Member

@aprotyas aprotyas left a comment

Choose a reason for hiding this comment

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

LGTM

@whsieh whsieh added the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Dec 10, 2023
… async text input is enabled

https://bugs.webkit.org/show_bug.cgi?id=266157
rdar://119434259

Reviewed by Megan Gardner.

After the UIKit changes in rdar://119283621, the default commands corresponding to these 3 actions:

```
_define:     define:
_translate:  translate:
_lookup:     lookup:
```

...now follow an inverted model for dispatching fallback actions, where the primary action selector
is the legacy private action, and the fallback selector is the new selector on `UIAsyncTextInput`.
This means that UIKit ends up calling into private selectors on the content view instead of their
non-underscore-prefixed counterparts even when `UIKit/async_text_input` is enabled, causing us to
hit release assertions under these three methods.

To avoid this, we adjust the default behavior of `-targetForActionForWebView:withSender:` to always
return `nil` for these three private selectors, in order to force UIKit to fall back to calling into
the new action selectors.

Changes covered by the layout test `editing/selection/ios/look-up-selected-text.html`, when async
text input is enabled.

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

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

Committed 271817@main (161dbc0): https://commits.webkit.org/271817@main

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

@webkit-commit-queue webkit-commit-queue merged commit 161dbc0 into WebKit:main Dec 10, 2023
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Dec 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform Portability improvements and other general platform improvements not driven directly by site bugs.
Projects
None yet
5 participants