cdp: wheel events scroll the viewport unless over a scroll container - #3479
Conversation
Input.dispatchMouseEvent mouseWheel (and the BiDi wheel action, which shares triggerMouseWheel) only ever moved the scroll position of the element under the cursor, and gave up when no element was there. window.scrollY reads a separate field that Window.scrollTo maintains, so a wheel never moved the page and never fired the document's scroll event; every CDP driver's scroll-by-wheel API was a no-op. Scroll the nearest ancestor whose overflow is auto or scroll, as before, and otherwise scroll the window through Window.scrollBy, which already schedules the trusted scroll and scrollend events. A wheel over empty space targets the root element instead of returning early.
69d8c1e to
f9eb4e4
Compare
| } | ||
|
|
||
| fn isScrollContainer(el: *Element, frame: *Frame) bool { | ||
| const style = frame.window.getComputedStyle(el, null, frame) catch return false; |
There was a problem hiding this comment.
This will always return a CSSStyleProperties that has an empty _properties. But it'll cache the value inside of the Frame's _element_computed_styles and have extra ceremony to eventually just call style_manager.inlineStyleValue. I think you just just call style_manager.inlineStyleValue directly here.
There was a problem hiding this comment.
Done. It now reads style_manager.inlineStyleValue directly: the axis longhand (overflow-x/overflow-y) first, then the matching half of the overflow shorthand. The comment on isScrollContainer notes that only inline overflow is resolved, since that is all computed styles see today.
| if (scrollContainerOf(target, frame)) |container| { | ||
| const new_left: i32 = @as(i32, @intCast(container.getScrollLeft(frame))) +| dx; | ||
| const new_top: i32 = @as(i32, @intCast(container.getScrollTop(frame))) +| dy; | ||
| try container.setScrollLeft(new_left, frame); |
There was a problem hiding this comment.
This is an issue in main also, but setScrollTop fires events (via the scheduleScrollEvents) and it'll have a different bubbles flag in most cases...and then we fire scroll a few lines down immediately.
There was a problem hiding this comment.
Done. The container path goes through Element.scrollBy and the viewport path through Window.scrollBy, and the inline dispatch is gone, so only the scheduled scroll/scrollend fire, with the bubbles flag those tasks already pick. The scroll-container test waits for the scheduled event with runner.waitForScript instead of asserting it synchronously.
| try target.setScrollTop(new_top, frame); | ||
| const dx = deltaToScroll(delta_x); | ||
| const dy = deltaToScroll(delta_y); | ||
| if (scrollContainerOf(target, frame)) |container| { |
There was a problem hiding this comment.
This should be axis-aware. If you're scrolling the x and find a overflow-y: auto, then it isn't scrollable.
There was a problem hiding this comment.
Done. Each axis resolves its own container: a delta along an axis whose overflow on the box is not auto/scroll falls through to the viewport. Added a case to the container test with overflow: hidden scroll and a diagonal wheel, where the y delta lands on the box and the x delta on the window.
|
In addition to the inline comments, github says it can be merged as-is, but it can't. I tried, compiler errors when applied to |
Each wheel axis now looks for the nearest ancestor whose inline overflow along that axis is auto or scroll, and scrolls it through Element.scrollBy, or the viewport through Window.scrollBy, so the trusted scroll and scrollend events are scheduled once instead of firing a second, bubbling scroll inline. The lookup reads the inline style straight from the style manager instead of building a computed-style object per ancestor. Both scrollBy implementations saturate the addition, so an oversized delta from CDP or from a script no longer overflows the i32 position.
|
Merged |
2 participants
Input.dispatchMouseEventwithtype: mouseWheel(and the BiDi wheel action, which sharestriggerMouseWheel) only ever moved the scroll position of the element under the cursor, and returned early when no element was there.window.scrollYreads a separate field thatWindow.scrollTomaintains, so a wheel never moved the page and never fired the document'sscrollevent. Every CDP driver's scroll-by-wheel API was a no-op: Playwrightmouse.wheel, Puppeteermouse.wheel, chromedpInput.dispatchMouseEvent, Selenium BiDi wheel actions, browser-usescroll(), Stagehand v3mouse.wheel.Now the nearest ancestor whose
overflowisautoorscrollis scrolled as before (the#scrollboxcase), and anything else scrolls the window throughWindow.scrollBy, which already schedules the trustedscrollandscrollendevents. A wheel over empty space targets the root element instead of returning early.Adds
cdp.inputtests for the scroll-container case, viewport scrolling with accumulation and a negative delta, and a wheel over empty space.Evidence from the integrations matrix (https://github.com/lightpanda-io/integrations): with this branch, the
scrollcell goes from ❌ to ✅ for Playwright, Puppeteer, chromedp, Selenium (BiDi), Browser Use CLI and Stagehand v3, with no other cell changing.