From 1e67c6fc91eeb030ff9c44802f6b36c65d010998 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Mon, 14 Sep 2026 17:37:22 +0200 Subject: [PATCH] Scroll the nearest scroll container from the scroll tool The scroll tool (MCP, agent, LP.scrollNode) wrote scrollTop on the exact node it was given, so a leaf inside an overflow:auto panel stored an offset on a non-scroller, the panel's own scroll listener never ran, and the tool reported the requested coordinates as if it had worked. It also fired a synchronous bubbling scroll on top of the async non-bubbling scroll/scrollend the setters already schedule. actions.scroll now resolves the nearest ancestor-or-self scroll container, falls back to the node itself, and returns the node that moved plus the read-back position. The tool and LP.scrollNode report that instead of the request. The container query moves from user_input.zig onto Element as scrollContainer(axes), so the wheel path, the tool and WebDriver share one resolver. WebDriver's wheel scrolled the hit-test element directly and fired its own bubbling scroll; it now goes through user_input.wheelScroll like CDP and BiDi wheel. Window and Element share one ScrollToOpts. Its offsets() helper normalizes the positional and dictionary forms once, and an omitted axis in the dictionary form leaves that axis untouched for the window too, matching browsers, so scrolling the window on one axis no longer resets the other. --- src/browser/actions.zig | 47 ++++++++-------- src/browser/frame/user_input.zig | 60 +++++---------------- src/browser/tests/mcp_actions.html | 3 ++ src/browser/tests/window_scroll.html | 10 ++++ src/browser/tools.zig | 17 +++--- src/browser/webapi/Element.zig | 81 +++++++++++++++++++++------- src/browser/webapi/WebDriver.zig | 12 +---- src/browser/webapi/Window.zig | 38 +++---------- src/mcp/tools.zig | 53 +++++++++++++++--- src/server/cdp/domains/lp.zig | 37 +++++++++---- 10 files changed, 204 insertions(+), 154 deletions(-) diff --git a/src/browser/actions.zig b/src/browser/actions.zig index 7bbe3371f..a60505d04 100644 --- a/src/browser/actions.zig +++ b/src/browser/actions.zig @@ -287,33 +287,34 @@ pub fn fill(node: *DOMNode, text: []const u8, frame: *Frame) !void { try dispatchInputAndChangeEvents(el, frame); } -pub fn scroll(node: ?*DOMNode, x: ?i32, y: ?i32, frame: *Frame) !void { - if (node) |n| { - const el = n.is(Element) orelse return error.InvalidNodeType; +pub const ScrollResult = struct { + /// What moved: the given node, its nearest scroll container, or null for + /// the window. + scrolled: ?*DOMNode, + x: u32, + y: u32, +}; - if (x) |val| { - el.setScrollLeft(val, frame) catch |err| { - lp.log.err(.app, "setScrollLeft failed", .{ .err = err }); - return error.ActionFailed; - }; - } - if (y) |val| { - el.setScrollTop(val, frame) catch |err| { - lp.log.err(.app, "setScrollTop failed", .{ .err = err }); - return error.ActionFailed; - }; - } - - const scroll_evt: *Event = try .initTrusted(comptime .wrap("scroll"), .{ .bubbles = true }, frame._page); - frame._event_manager.dispatch(el.asEventTarget(), scroll_evt) catch |err| { - lp.log.err(.app, "dispatch scroll event failed", .{ .err = err }); - }; - } else { - frame.window.scrollTo(.{ .x = x orelse 0 }, y, frame) catch |err| { +pub fn scroll(node: ?*DOMNode, x: ?i32, y: ?i32, frame: *Frame) !ScrollResult { + const n = node orelse { + frame.window.scrollTo(.{ .opts = .{ .left = x, .top = y } }, null, frame) catch |err| { lp.log.err(.app, "scroll failed", .{ .err = err }); return error.ActionFailed; }; - } + return .{ .scrolled = null, .x = frame.window.getScrollX(), .y = frame.window.getScrollY() }; + }; + const el = n.is(Element) orelse return error.InvalidNodeType; + + const target = el.scrollContainer(.{ .x = x != null, .y = y != null }, frame) orelse el; + target.scrollTo(.{ .opts = .{ .left = x, .top = y } }, null, frame) catch |err| { + lp.log.err(.app, "scroll failed", .{ .err = err }); + return error.ActionFailed; + }; + return .{ + .scrolled = target.asNode(), + .x = target.getScrollLeft(frame), + .y = target.getScrollTop(frame), + }; } // Floored to 1 so timeout_ms=0 still gets one check instead of failing outright. diff --git a/src/browser/frame/user_input.zig b/src/browser/frame/user_input.zig index 41d492de2..32f53dfaa 100644 --- a/src/browser/frame/user_input.zig +++ b/src/browser/frame/user_input.zig @@ -302,65 +302,29 @@ pub fn triggerMouseWheel(frame: *Frame, x: f64, y: f64, delta_x: f64, delta_y: f } // CDP deltas are untrusted, so guard NaN and saturate the addition. - try scrollAlong(target, .x, deltaToScroll(delta_x), frame); - try scrollAlong(target, .y, deltaToScroll(delta_y), frame); + try wheelScroll(target, deltaToScroll(delta_x), deltaToScroll(delta_y), frame); } -const ScrollAxis = enum { x, y }; +/// Each axis scrolls the nearest ancestor-or-self scroll container along it, +/// else the viewport. Relative deltas may land on different scrollers per +/// axis, unlike an absolute position. +pub fn wheelScroll(target: *Element, delta_x: i32, delta_y: i32, frame: *Frame) !void { + try scrollAlong(target, .{ .x = true }, delta_x, frame); + try scrollAlong(target, .{ .y = true }, delta_y, frame); +} -// Each axis scrolls the nearest ancestor-or-self that is a scroll container -// along it, else the viewport. Both scrollBy paths schedule the trusted -// scroll/scrollend events themselves. -fn scrollAlong(target: *Element, axis: ScrollAxis, delta: i32, frame: *Frame) !void { +fn scrollAlong(target: *Element, axes: Element.ScrollAxes, delta: i32, frame: *Frame) !void { if (delta == 0) { return; } - const left: i32, const top: i32 = switch (axis) { - .x => .{ delta, 0 }, - .y => .{ 0, delta }, - }; - if (scrollContainerOf(target, axis, frame)) |container| { + const left: i32 = if (axes.x) delta else 0; + const top: i32 = if (axes.y) delta else 0; + if (target.scrollContainer(axes, frame)) |container| { return container.scrollBy(.{ .opts = .{ .left = left, .top = top } }, null, frame); } return frame.window.scrollBy(.{ .opts = .{ .left = left, .top = top } }, null, frame); } -// html/body scroll the viewport. -fn scrollContainerOf(start: *Element, axis: ScrollAxis, frame: *Frame) ?*Element { - var current: ?*Element = start; - while (current) |el| : (current = el.parentElement()) { - switch (el.getTag()) { - .html, .body => return null, - else => {}, - } - if (isScrollContainer(el, axis, frame)) { - return el; - } - } - return null; -} - -// Only inline `overflow` is resolved: computed styles don't cascade stylesheet -// rules, so a sheet-declared scroll container is treated as page content. -fn isScrollContainer(el: *Element, axis: ScrollAxis, frame: *Frame) bool { - const style_manager = &frame._style_manager; - const longhand = switch (axis) { - .x => style_manager.inlineStyleValue(el, comptime .wrap("overflow-x")), - .y => style_manager.inlineStyleValue(el, comptime .wrap("overflow-y")), - }; - const value = longhand orelse blk: { - // `overflow: []`; a single value applies to both axes. - const shorthand = style_manager.inlineStyleValue(el, comptime .wrap("overflow")) orelse return false; - var it = std.mem.tokenizeScalar(u8, shorthand, ' '); - const x = it.next() orelse return false; - break :blk switch (axis) { - .x => x, - .y => it.next() orelse x, - }; - }; - return std.ascii.eqlIgnoreCase(value, "auto") or std.ascii.eqlIgnoreCase(value, "scroll"); -} - fn deltaToScroll(d: f64) i32 { if (std.math.isNan(d)) return 0; return @trunc(std.math.clamp(d, std.math.minInt(i32), std.math.maxInt(i32))); diff --git a/src/browser/tests/mcp_actions.html b/src/browser/tests/mcp_actions.html index 2bab8999f..50268246c 100644 --- a/src/browser/tests/mcp_actions.html +++ b/src/browser/tests/mcp_actions.html @@ -10,6 +10,9 @@
Long content
+
+

Inner leaf

+
Hover Me