From f6ca133132b8092770ba606cb98df2982dc8c1fa Mon Sep 17 00:00:00 2001 From: Andrey Mnatsakanov Date: Wed, 22 Jul 2026 20:16:12 +0200 Subject: [PATCH] fix(js): make window-level scrolling actually scroll (#468) window.scrollTo/scrollBy/scroll were empty stubs, scrollX/scrollY and pageXOffset/pageYOffset were hard-coded 0 data properties, no scroll event was ever fired at the window, and document.scrollingElement did not exist. that "scroll-driven lazy loaders advance instead of throwing". But the dominant infinite-scroll idiom is window-level: window.scrollTo(0, document.body.scrollHeight); if (window.scrollY + innerHeight >= body.scrollHeight) loadMore(); all of which was inert -- the call succeeded, the offset never moved, the sentinel never tripped -- so automation that scrolls a page to trigger loading silently got one screenful. The offset is stored on the scrolling element rather than in separate window state, so window.scrollY and document.scrollingElement.scrollTop are two views of one value in both directions, which is what pages assume when they mix the two. scrollX/scrollY/pageXOffset/pageYOffset become read-only accessors over it, matching a real Window, and document.scrollingElement returns documentElement (standards mode; we never parse in quirks mode). The scroll event is dispatched at both the document and the window. Chrome propagates a page scroll to both, but Document.dispatchEvent here runs only its own listeners and does not propagate, so firing once would strand half the listeners. As with #431 there is no layout, so the offset still cannot be clamped to a real maximum; negative values clamp to 0 exactly as they do for elements. --- crates/obscura-js/js/bootstrap.js | 64 +++++++++++++++++++++++++-- crates/obscura-js/src/runtime.rs | 73 +++++++++++++++++++++++++++++++ 2 files changed, 134 insertions(+), 3 deletions(-) diff --git a/crates/obscura-js/js/bootstrap.js b/crates/obscura-js/js/bootstrap.js index 8e70cb3..2fce229 100644 --- a/crates/obscura-js/js/bootstrap.js +++ b/crates/obscura-js/js/bootstrap.js @@ -2733,6 +2733,10 @@ class Document extends Node { } getSelection() { return this.defaultView ? _selectionFor(this) : null; } get activeElement() { return globalThis.__obscura_focused || this.body; } + // The element that scrolls the viewport, and where the page offset lives + // (issue #468). Standards mode, so documentElement — quirks mode would be + // body, but we never parse in quirks mode. + get scrollingElement() { return this.documentElement; } get implementation() { const ownerDoc = this; return { @@ -7131,9 +7135,63 @@ URL.revokeObjectURL = function(url) { delete globalThis.__blobStore[url]; }; -globalThis.scrollTo = function(x, y) {}; -globalThis.scrollBy = function(x, y) {}; -globalThis.scroll = function(x, y) {}; +// Window-level scrolling (issue #468). #431 gave elements functional +// scrollTop/scrollLeft plus scroll methods, but left these three as no-ops, so +// the dominant infinite-scroll idiom -- window.scrollTo(0, body.scrollHeight), +// window.scrollBy(0, 500), then a window 'scroll' listener -- did nothing at +// all: the offset never moved and no event ever fired. +// +// The page offset is stored on the scrolling element rather than in separate +// window state, so window.scrollY and document.scrollingElement.scrollTop are +// two views of one value, which is what pages assume. As with #431 there is no +// layout, so the offset still cannot be clamped to a real maximum. +function _scrollRoot() { + const doc = globalThis.document; + return (doc && doc.scrollingElement) || null; +} +function _windowScroll(x, y, relative) { + const root = _scrollRoot(); + if (!root) return; + let left, top; + if (x !== null && typeof x === 'object') { left = x.left; top = x.top; } + else { left = x; top = y; } + if (left !== undefined) { + root.scrollLeft = (relative ? (root.scrollLeft || 0) : 0) + (+left || 0); + } + if (top !== undefined) { + root.scrollTop = (relative ? (root.scrollTop || 0) : 0) + (+top || 0); + } + // Async, matching the element path #431 added. Dispatched at the document + // AND the window: a page scroll event reaches both in Chrome, but + // Document.dispatchEvent here runs only its own listeners and does not + // propagate, so firing once would strand half the listeners. + setTimeout(() => { + try { + const doc = globalThis.document; + if (doc) doc.dispatchEvent(new Event('scroll', { bubbles: false })); + globalThis.dispatchEvent(new Event('scroll', { bubbles: false })); + } catch (e) {} + }, 0); +} +globalThis.scrollTo = function(x, y) { _windowScroll(x, y, false); }; +globalThis.scrollBy = function(x, y) { _windowScroll(x, y, true); }; +globalThis.scroll = function(x, y) { _windowScroll(x, y, false); }; +_markNative(globalThis.scrollTo); +_markNative(globalThis.scrollBy); +_markNative(globalThis.scroll); +// Read-only accessors, as on a real Window: assigning window.scrollY does not +// scroll the page. These replace the hard-coded 0 data properties defined +// earlier, so they must stay after them. +for (const [name, offset] of [ + ['scrollX', 'scrollLeft'], ['pageXOffset', 'scrollLeft'], + ['scrollY', 'scrollTop'], ['pageYOffset', 'scrollTop'], +]) { + Object.defineProperty(globalThis, name, { + configurable: true, + enumerable: true, + get() { const root = _scrollRoot(); return root ? (root[offset] || 0) : 0; }, + }); +} globalThis.focus = function() {}; _markNative(globalThis.focus); globalThis.blur = function() {}; _markNative(globalThis.blur); globalThis.print = function() {}; _markNative(globalThis.print); diff --git a/crates/obscura-js/src/runtime.rs b/crates/obscura-js/src/runtime.rs index ca7ade2..35fde4c 100644 --- a/crates/obscura-js/src/runtime.rs +++ b/crates/obscura-js/src/runtime.rs @@ -1541,6 +1541,79 @@ mod tests { assert_eq!(result, serde_json::json!(["P", "A"])); } + /// Issue #468: window.scrollTo/scrollBy/scroll were no-op stubs, so the + /// dominant infinite-scroll idiom never advanced the page offset. + #[test] + fn window_scroll_methods_move_the_page_offset() { + let mut rt = setup_runtime(r#"
"#); + let result = rt + .evaluate( + r#" + const scrolled = window.scrollTo(0, 500); + const afterTo = [window.scrollX, window.scrollY]; + window.scrollBy(0, 200); + const afterBy = [window.pageXOffset, window.pageYOffset]; + window.scrollTo({ left: 10, top: 40 }); + const afterOptions = [window.scrollX, window.scrollY]; + window.scroll(5, 5); + const afterScroll = [window.scrollX, window.scrollY]; + // Negative offsets clamp to 0, as they do for elements. + window.scrollTo(0, -100); + return [afterTo, afterBy, afterOptions, afterScroll, window.scrollY]; + "#, + ) + .unwrap(); + assert_eq!( + result, + serde_json::json!([[0, 500], [0, 700], [10, 40], [5, 5], 0]) + ); + } + + /// Issue #468: the page offset is one value, readable and writable through + /// either `window.scrollY` or `document.scrollingElement.scrollTop`. + #[test] + fn window_scroll_offset_is_shared_with_the_scrolling_element() { + let mut rt = setup_runtime(r#"
"#); + let result = rt + .evaluate( + r#" + const isDocEl = document.scrollingElement === document.documentElement; + window.scrollTo(0, 300); + // Written through the window, read through the element... + const viaElement = document.scrollingElement.scrollTop; + // ...and the reverse. + document.scrollingElement.scrollTop = 90; + return [isDocEl, viaElement, window.scrollY, window.pageYOffset]; + "#, + ) + .unwrap(); + assert_eq!(result, serde_json::json!([true, 300, 90, 90])); + } + + /// Issue #468: a scroll event must reach listeners on both the window and + /// the document — that is the signal lazy loaders wait for. + #[tokio::test(flavor = "current_thread")] + async fn window_scroll_fires_a_scroll_event() { + let mut rt = setup_runtime(r#"
"#); + let result = rt + .evaluate_for_cdp( + r#" + new Promise(resolve => { + let win = 0, doc = 0; + window.addEventListener('scroll', () => win++); + document.addEventListener('scroll', () => doc++); + window.scrollBy(0, 400); + setTimeout(() => resolve([win, doc, window.scrollY]), 5); + }) + "#, + true, + true, + ) + .await + .unwrap(); + assert_eq!(result.value.unwrap(), serde_json::json!([1, 1, 400])); + } + #[test] fn append_child_flattens_document_fragment() { let mut rt = setup_runtime(r#"
"#);