Bug 264469

Summary: Incorrect wheel event behavior with Shadow DOM
Product: WebKit Reporter: Bretislav Wajtr <bretislav>
Component: ScrollingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: RESOLVED FIXED    
Severity: Normal CC: nmouchtaris, rniwa, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 17   
Hardware: Mac (Apple Silicon)   
OS: macOS 14   
Bug Depends on:    
Bug Blocks: 148695    
Attachments:
Description Flags
safari-wheel-listener-scroll-issue.html none

Description Bretislav Wajtr 2023-11-08 23:47:30 PST 
Created attachment 468530 [details]
safari-wheel-listener-scroll-issue.html

One of our customers complained about the inability to scroll the page when over certain elements on the page when using MacBook, Safari and either Trackpad or MagicMouse. Note that the same issue cannot be reproduced with other browsers or when scrolling with a mouse with a physical wheel. For reference, this is the bug on our side: https://github.com/vaadin/web-components/issues/6695

We reduced the issue test case to what is attached in safari-wheel-listener-scroll-issue.html. The DOM structure presented there is a simplification of our app structure, the minimal structure where I was still able to reproduce the issue.

Test instructions:
1. Open Safari on Mac and use either Trackpad or MagicMouse for scrolling
2. Open safari-wheel-listener-scroll-issue.html
3. Position the mouse over one of the colored areas and swipe with two fingers in a way that the page should scroll down
4. -> "test-container" element content scrolls 
5. Now Position the mouse over the white stripe
6. Swipe the same way
Actual behavior: Now the "body" scrolls instead of the "test-container"
Expected  behavior: I'd expect the "test-container" to scroll as in other browsers, or when scrolling with mouse with physical wheel

The weird thing is that the issue is caused just by attaching an *empty* "wheel" event listener to one of the elements in the DOM structure -> comment out line 35, so the event listener is not added, and suddenly, the scroll behavior is different and the "test-container" scrolls even when positioned over the white stripe.

I believe that adding an empty event listener anywhere should not change the scrolling behavior of the page, regardless of the DOM structure.

Safari Version 17.1 (19616.2.9.11.7)
Mac Mini M2
macOS 14.1 (23B74)
Comment 1 Alexey Proskuryakov 2023-11-10 13:35:26 PST 
Adding a wheel event listener changes everything about how scrolling works, as every event now has to hop across processes.

What you are describing looks like a bug, however I expect that there will always be performance implications from having event listeners.
Comment 2 Simon Fraser (smfr) 2023-11-10 13:47:36 PST 
Does this reproduce without the Shadow DOM stuff in the test page?
Comment 3 Bretislav Wajtr 2023-11-14 05:44:15 PST 
@Simon 
No, it cannot be reproduced if I use simple <div> instead of that web-component with a Shadow DOM and <slot> in it. So using the web component in the DOM structure is required to reproduce this issue.

In the real use-case, where we noticed the bug, we use much more complicated structure of web components and other elements... however, I was able to narrow down the test case to what is in the attachment.
Comment 4 Radar WebKit Bug Importer 2023-11-15 23:48:13 PST 
<rdar://problem/118496293>
Comment 5 Simon Fraser (smfr) 2024-01-17 20:56:41 PST 
This fixes it, but I'm not entirely sure if it's correct:

diff --git a/Source/WebCore/page/mac/EventHandlerMac.mm b/Source/WebCore/page/mac/EventHandlerMac.mm
index c13fcc320751735a86565b8e3e67f8d9c9984608..97e4eb43530930df997f462d942da55b4dde4404 100644
--- a/Source/WebCore/page/mac/EventHandlerMac.mm
+++ b/Source/WebCore/page/mac/EventHandlerMac.mm
@@ -800,7 +800,7 @@ static ContainerNode* findEnclosingScrollableContainer(ContainerNode* node, cons
 {
     // Find the first node with a valid scrollable area starting with the current
     // node and traversing its parents (or shadow hosts).
-    for (ContainerNode* candidate = node; candidate; candidate = candidate->parentOrShadowHostNode()) {
+    for (auto* candidate = node; candidate; candidate = candidate->parentInComposedTree()) {
         if (is<HTMLIFrameElement>(*candidate))
             continue;
Comment 6 Simon Fraser (smfr) 2024-01-17 21:21:56 PST 
Pull request: https://github.com/WebKit/WebKit/pull/22892
Comment 7 EWS 2024-01-18 09:02:54 PST 
Committed 273181@main (bd33a8e97f27): <https://commits.webkit.org/273181@main>

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