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

[visionOS] Remote Effects don't work inside of pointer-events: none scrollers #18351

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

etiennesegonzac
Copy link
Contributor

@etiennesegonzac etiennesegonzac commented Sep 28, 2023

0fed4da

[visionOS] Remote Effects don't work inside of `pointer-events: none` scrollers
https://bugs.webkit.org/show_bug.cgi?id=262307
<rdar://115776753>

Reviewed by NOBODY (OOPS!).

Remote Effects use CA-based hit-testing.
So when we set `userInteractionEnabled = NO` on a `pointer-events: none`
ScrollView, we also prevent potential `pointer-events: auto` children
from hit-testing.

This patch introduces a new `_wk_userInteractive` flag on
WKChildScrollViews that disables the scroll container without blocking
hit-testing of its children. Effectively taking the view out of
WKHitTesting without impacting its layer's `allowsHitTesting` property.

* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreePropertyApplier.mm:
(WebKit::RemoteLayerTreePropertyApplier::applyPropertiesToUIView):
Use `_wk_userInteractive` instead of `userInteractionEnabled` for
WKChildScrollViews.

* Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.h:
* Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:
(-[WKChildScrollView initWithFrame:]):
Introduce the new `_wk_userInteractive` property defaulting to YES.
(WebKit::collectDescendantViewsAtPoint):
(WebKit::collectDescendantViewsInRect):
Remove the FIXME since we use `userInteractionEnabled` for
`WKNativelyInteractible` views.
Skip WKChildScrollViews marked as `_wk_userInteractive = NO`.

* Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.h:
Clean-up, remove unused member variable.

* Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm:
(dumpCALayer):
Add `allowsHitTesting` to the dump format when Interaction Regions are
turned on.

* LayoutTests/interaction-region/scroll-view-hit-testing-expected.txt: Added.
* LayoutTests/interaction-region/scroll-view-hit-testing.html: Added.
Add a test covering the change, based on
`touch-scroll-pointer-events-none.html`.
* LayoutTests/interaction-region/interaction-layers-culling-expected.txt:
* LayoutTests/interaction-region/layer-tree-expected.txt:
Update expectations with the new format.

0fed4da

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
βœ… πŸ›  watch
βœ… πŸ›  watch-sim

@etiennesegonzac etiennesegonzac self-assigned this Sep 28, 2023
@etiennesegonzac etiennesegonzac added the Layout and Rendering For bugs with layout and rendering of Web pages. label Sep 28, 2023
@etiennesegonzac
Copy link
Contributor Author

Hi @aprotyas ! Mentioning you here because this seems tangentially related to https://bugs.webkit.org/show_bug.cgi?id=258255

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 28, 2023
@etiennesegonzac etiennesegonzac removed the merging-blocked Applied to prevent a change from being merged label Sep 28, 2023
@@ -225,6 +225,9 @@ class RenderLayerScrollableArea final : public ScrollableArea {
void updateScrollbarsAfterStyleChange(const RenderStyle* oldStyle);
void updateScrollbarsAfterLayout();

ScrollbarMode horizontalScrollbarMode() const final;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think pointer-events:none should affect the scrollbar mode (it doesn't affect how scrollbars render, only whether the user can interact).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was a bit fragile having completely different codepaths calling into setScrollEnabled anyway.

Trying a simpler approach: a custom property on WKChildScrollView.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 29, 2023
@etiennesegonzac etiennesegonzac removed the merging-blocked Applied to prevent a change from being merged label Sep 29, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 29, 2023
@etiennesegonzac etiennesegonzac removed the merging-blocked Applied to prevent a change from being merged label Sep 29, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Sep 30, 2023
@etiennesegonzac etiennesegonzac removed the merging-blocked Applied to prevent a change from being merged label Oct 1, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Oct 6, 2023
@etiennesegonzac etiennesegonzac removed the merging-blocked Applied to prevent a change from being merged label Oct 6, 2023
… scrollers

https://bugs.webkit.org/show_bug.cgi?id=262307
<rdar://115776753>

Reviewed by NOBODY (OOPS!).

Remote Effects use CA-based hit-testing.
So when we set `userInteractionEnabled = NO` on a `pointer-events: none`
ScrollView, we also prevent potential `pointer-events: auto` children
from hit-testing.

This patch introduces a new `_wk_userInteractive` flag on
WKChildScrollViews that disables the scroll container without blocking
hit-testing of its children. Effectively taking the view out of
WKHitTesting without impacting its layer's `allowsHitTesting` property.

* Source/WebKit/Shared/RemoteLayerTree/RemoteLayerTreePropertyApplier.mm:
(WebKit::RemoteLayerTreePropertyApplier::applyPropertiesToUIView):
Use `_wk_userInteractive` instead of `userInteractionEnabled` for
WKChildScrollViews.

* Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.h:
* Source/WebKit/UIProcess/RemoteLayerTree/ios/RemoteLayerTreeViews.mm:
(-[WKChildScrollView initWithFrame:]):
Introduce the new `_wk_userInteractive` property defaulting to YES.
(WebKit::collectDescendantViewsAtPoint):
(WebKit::collectDescendantViewsInRect):
Remove the FIXME since we use `userInteractionEnabled` for
`WKNativelyInteractible` views.
Skip WKChildScrollViews marked as `_wk_userInteractive = NO`.

* Source/WebKit/UIProcess/RemoteLayerTree/ios/ScrollingTreeScrollingNodeDelegateIOS.h:
Clean-up, remove unused member variable.

* Source/WebKit/UIProcess/API/Cocoa/WKWebViewTesting.mm:
(dumpCALayer):
Add `allowsHitTesting` to the dump format when Interaction Regions are
turned on.

* LayoutTests/interaction-region/scroll-view-hit-testing-expected.txt: Added.
* LayoutTests/interaction-region/scroll-view-hit-testing.html: Added.
Add a test covering the change, based on
`touch-scroll-pointer-events-none.html`.
* LayoutTests/interaction-region/interaction-layers-culling-expected.txt:
* LayoutTests/interaction-region/layer-tree-expected.txt:
Update expectations with the new format.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Layout and Rendering For bugs with layout and rendering of Web pages.
Projects
None yet
5 participants