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.

Sign up for GitHub

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

Jump to bottom

REGRESSION (266610@main): Catalyst WKWebView does not dispatch contextmenu on right click #22620

Merged
merged 1 commit into from
Jan 11, 2024
Merged

REGRESSION (266610@main): Catalyst WKWebView does not dispatch contextmenu on right click #22620

merged 1 commit into from
Jan 11, 2024

Conversation

whsieh
Copy link
Member

@whsieh whsieh commented Jan 10, 2024 •
edited by webkit-early-warning-system
Loading

2b9a29c

REGRESSION (266610@main): Catalyst WKWebView does not dispatch `contextmenu` on right click
https://bugs.webkit.org/show_bug.cgi?id=267361
rdar://119634050

Reviewed by Megan Gardner and Tim Horton.

After my refactoring in 266610@main, we no longer dispatch `contextmenu` events when right clicking
in Mac Catalyst `WKWebView`. This is because UIKit's gesture environment skips over the new
`WKMouseTouchGestureRecognizer` when delivering events to all gesture recognizers on the window,
which in turn means that `-[WKMouseTouchGestureRecognizer touchesBegan:withEvent:]` never gets
triggered and we don't dispatch any mouse (or context menu) events to the page.

This is due to UIKit having a Catalyst-specific codepath that returns `NO` in the case where the
pointer is not the primary button. Previously, we avoided this by overriding
`-_shouldReceiveTouch:forEvent:recognizerView:` and returning `YES` in the case of pointer touches.
Since the new `WKMouseTouchGestureRecognizer` doesn't use any SPI or IPI, this bypass no longer
works.

To address this, simply override the IPI method `-_defaultAllowedMouseButtons` to return both
primary and secondary mouse buttons to restore shipping behavior.

* Source/WebKit/UIProcess/ios/WKMouseInteraction.mm:
(-[WKMouseTouchGestureRecognizer _defaultAllowedMouseButtons]):

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

9e5039d

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
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

@whsieh whsieh requested a review from cdumez as a code owner January 10, 2024 22:23
@whsieh whsieh self-assigned this Jan 10, 2024
@whsieh whsieh added the UI Events For bugs related to user interactions like keyboard, mouse, and touch events. label Jan 10, 2024
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jan 10, 2024 •
edited
Loading

EWS run on current version of this PR (hash 9e5039d)

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
✅ 🛠 🧪 merge ✅ 🛠 watch
✅ 🛠 watch-sim

aprotyas
aprotyas reviewed Jan 10, 2024
View reviewed changes
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 merge-queue Applied to send a pull request to merge-queue label Jan 10, 2024
@whsieh
Copy link
Member Author

whsieh commented Jan 10, 2024

Thanks for the reviews!

@whsieh
REGRESSION (266610@main): Catalyst WKWebView does not dispatch `conte…
2b9a29c 
…xtmenu` on right click

https://bugs.webkit.org/show_bug.cgi?id=267361
rdar://119634050

Reviewed by Megan Gardner and Tim Horton.

After my refactoring in 266610@main, we no longer dispatch `contextmenu` events when right clicking
in Mac Catalyst `WKWebView`. This is because UIKit's gesture environment skips over the new
`WKMouseTouchGestureRecognizer` when delivering events to all gesture recognizers on the window,
which in turn means that `-[WKMouseTouchGestureRecognizer touchesBegan:withEvent:]` never gets
triggered and we don't dispatch any mouse (or context menu) events to the page.

This is due to UIKit having a Catalyst-specific codepath that returns `NO` in the case where the
pointer is not the primary button. Previously, we avoided this by overriding
`-_shouldReceiveTouch:forEvent:recognizerView:` and returning `YES` in the case of pointer touches.
Since the new `WKMouseTouchGestureRecognizer` doesn't use any SPI or IPI, this bypass no longer
works.

To address this, simply override the IPI method `-_defaultAllowedMouseButtons` to return both
primary and secondary mouse buttons to restore shipping behavior.

* Source/WebKit/UIProcess/ios/WKMouseInteraction.mm:
(-[WKMouseTouchGestureRecognizer _defaultAllowedMouseButtons]):

Canonical link: https://commits.webkit.org/272883@main
@webkit-commit-queue webkit-commit-queue merged commit 2b9a29c into WebKit:main Jan 11, 2024
@webkit-commit-queue
Copy link
Collaborator

Committed 272883@main (2b9a29c): https://commits.webkit.org/272883@main

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

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jan 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aprotyas aprotyas aprotyas left review comments

@hortont424 hortont424 hortont424 approved these changes

@megangardner megangardner megangardner approved these changes

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

@pxlcoder pxlcoder Awaiting requested review from pxlcoder

@rr-codes rr-codes Awaiting requested review from rr-codes

Assignees

@whsieh whsieh

Labels
UI Events For bugs related to user interactions like keyboard, mouse, and touch events.
Projects
None yet
Milestone
No milestone
6 participants
@whsieh @webkit-early-warning-system @webkit-commit-queue @hortont424 @aprotyas @megangardner