diff --git a/.isu/issues.json b/.isu/issues.json index 68cb332..85d5845 100644 --- a/.isu/issues.json +++ b/.isu/issues.json @@ -1,5 +1,5 @@ { - "next_id": 377, + "next_id": 378, "issues": [ { "id": 1, @@ -4608,6 +4608,19 @@ "author": "piefev", "state": "closed", "created_at": "2026-06-19T10:15:48Z" + }, + { + "id": 377, + "repo": "we", + "title": "Bing offline hydration: es-module-shims importShim hits missing function (after NodeList iterability fix)", + "body": "Parent: isu issue 375 (and tracker 280).\n\nAfter fixing the NodeList/HTMLCollection iterability bug (collections returned by querySelectorAll / getElementsBy* / .children now inherit a NodeList prototype and are iterable with for...of / spread / forEach), es-module-shims' `processImportMaps` no longer throws — it does `for (const script of document.querySelectorAll(...))` and previously aborted on the missing [Symbol.iterator].\n\nes-module-shims now advances one step further and hits a NEW uncaught `TypeError: undefined is not a function` inside its `importShim` entry point.\n\nRepro:\n`cargo run -p we-e2e -- --scenario crates/e2e/scenarios/real-web/bing.com.we --out-dir crates/e2e/artifacts`\n\nDiagnostic technique (temporary): in crates/js/src/vm.rs, at the Op::Call non-function branch (the `{desc} is not a function` site), print the enclosing function name + `func.source_location_for(pc)` + `func.names` when `handle_exception` returns false (uncaught). The uncaught failures observed after the iterability fix:\n\n- fn='importShim' loc=(1, 9763) names=[length, initPromise, this, window, importMapPromise, resolve, r, credentials]\n Source: crates/e2e/real-web/snapshots/bing.com/r.bing.com/rp/p5FEdYrxIEuxYfzXnnTB9pKvt6U.js (es-module-shims). Around col 9763:\n `async function importShim(id,...args){let parentUrl=args[args.length-1];...await initPromise...` then `topLevelLoad((await resolve(id,parentUrl)).r,{credentials:\\\"same-origin\\\"})`.\n Something in the initPromise/resolve/topLevelLoad chain resolves to undefined and is then called.\n\n- fn='' loc=(1, 86822) names=[toString, this, window, length]\n A SEPARATE, unrelated uncaught `undefined is not a function` (not es-module-shims). Needs its own bundle/source mapping.\n\nBoth are thrown inside a promise `.then` microtask handler whose chained promise has no rejection handler, so they are swallowed silently (no console output, no unhandledrejection). That is why modules_wrapper hydration aborts with an empty console.\n\nNote on the dominant visual gap: even with full hydration, the bing.com screenshot cannot match the committed Chromium golden because the snapshot's background (SichuanTea) differs from the golden's (Jaipur) — see isu issue 305 (snapshot/golden drift) — and the module/news cards come from uncached MSN feeds — see isu issue 334. So this issue tracks only the engine-side hydration blocker, not the screenshot parity (which stays xfail under 280).", + "labels": [ + "real-web" + ], + "assigned": [], + "author": "piefev", + "state": "open", + "created_at": "2026-06-19T11:21:38Z" } ] } diff --git a/crates/e2e/pages/58_nodelist_iteration.html b/crates/e2e/pages/58_nodelist_iteration.html new file mode 100644 index 0000000..520c620 --- /dev/null +++ b/crates/e2e/pages/58_nodelist_iteration.html @@ -0,0 +1,52 @@ + + + +NodeList iteration + + + +

NodeList / HTMLCollection iteration

+ +
pending
+ + + diff --git a/crates/e2e/scenarios/nodelist_iteration.we b/crates/e2e/scenarios/nodelist_iteration.we new file mode 100644 index 0000000..ed73530 --- /dev/null +++ b/crates/e2e/scenarios/nodelist_iteration.we @@ -0,0 +1,16 @@ +# Regression scenario for isu issue 375 (parent 280): the array-like +# collections returned by querySelectorAll / getElementsBy* / .children had no +# prototype, so they were not iterable. `for...of`, spread, and `forEach` all +# threw "undefined is not a function" when the engine looked up the missing +# [Symbol.iterator]. es-module-shims' `processImportMaps` iterates +# `document.querySelectorAll(...)` with for...of, so this silently aborted +# Bing's offline hydration. They must now behave like a real NodeList: +# iterable + forEach, but without the Array mutators (push). + +viewport 800 600 +goto crates/e2e/pages/58_nodelist_iteration.html +screenshot 58_nodelist_iteration.png +dump_dom 58_nodelist_iteration.dom.txt +dump_console 58_nodelist_iteration.console.txt +assert_dom_contains "forof=3 spread=3 fe=3 kids=3 none=0 forEach=function push=undefined" +assert_console_contains "nodelist-iter-ok:forof=3 spread=3 fe=3 kids=3 none=0 forEach=function push=undefined" diff --git a/crates/js/src/dom_bridge.rs b/crates/js/src/dom_bridge.rs index 70e898b..7d35edf 100644 --- a/crates/js/src/dom_bridge.rs +++ b/crates/js/src/dom_bridge.rs @@ -544,6 +544,41 @@ fn walk_tree(doc: &Document, root: NodeId, visitor: &mut dyn FnMut(NodeId) -> bo // ── Helper: make an array of wrapper Values ───────────────────────── +/// Build the shared `NodeList`/`HTMLCollection` prototype and store it on the +/// DOM bridge. The array-like collections returned by `querySelectorAll`, +/// `getElementsBy*`, `.children`, `select.options`, etc. inherit from it so +/// they are iterable with `for...of` and spread, and expose +/// `forEach`/`entries`/`keys`/`values` like a real `NodeList`. The iteration +/// helpers are borrowed directly from `Array.prototype` (already populated by +/// `init_builtins`, including the JS-preamble `forEach`) so one set of +/// natives backs both. The prototype itself chains to `Object.prototype`, so +/// unlike a real array these collections do not expose mutators such as +/// `push`/`splice`. +pub fn init_node_list_prototype(vm: &mut Vm) { + // Borrow the iteration helpers off Array.prototype. + let mut methods: Vec<(&'static str, Value)> = Vec::new(); + if let Some(arr_proto) = vm.array_prototype { + if let Some(HeapObject::Object(arr_data)) = vm.gc.get(arr_proto) { + for key in ["@@iterator", "forEach", "entries", "keys", "values"] { + if let Some(prop) = arr_data.get_property(key, &vm.shapes) { + methods.push((key, prop.value)); + } + } + } + } + + let mut proto = ObjectData::new(); + proto.prototype = vm.object_prototype; + for (key, value) in methods { + proto.insert_property(key.to_string(), Property::builtin(value), &mut vm.shapes); + } + let proto_ref = vm.gc.alloc(HeapObject::Object(proto)); + + if let Some(bridge) = &vm.dom_bridge { + *bridge.node_list_prototype.borrow_mut() = Some(proto_ref); + } +} + fn make_wrapper_array( nodes: &[NodeId], gc: &mut Gc, @@ -552,6 +587,9 @@ fn make_wrapper_array( object_proto: Option, ) -> Value { let mut obj = ObjectData::new(); + // Inherit from the shared NodeList prototype so the collection is iterable + // (`for...of`, spread) and exposes `forEach`/`entries`/`keys`/`values`. + obj.prototype = *bridge.node_list_prototype.borrow(); for (i, &nid) in nodes.iter().enumerate() { let wrapper = get_or_create_wrapper(nid, gc, shapes, bridge, object_proto); obj.insert_property( @@ -8904,6 +8942,36 @@ mod tests { } } + #[test] + fn test_query_selector_all_is_iterable() { + // Regression: the array-like collections returned by querySelectorAll / + // getElementsBy* / .children used to be plain objects without a + // prototype, so `for...of`, spread, and `forEach` threw + // "undefined is not a function" when calling the missing + // [Symbol.iterator]. es-module-shims' `processImportMaps` does + // `for (const s of document.querySelectorAll(...))`, which silently + // aborted Bing's hydration. They must now behave like a NodeList. + let result = eval_with_doc( + r#"

a

b

c

"#, + r#" + var els = document.querySelectorAll("p"); + var forof = 0; for (const el of els) { forof = forof + 1; } + var spread = [...els].length; + var fe = 0; els.forEach(function(e) { fe = fe + 1; }); + var bodyEl = document.querySelectorAll("body")[0]; + var kids = 0; for (const k of bodyEl.children) { kids = kids + 1; } + var none = 0; + for (const x of document.querySelectorAll(".nomatch")) { none = none + 1; } + [forof, spread, fe, kids, none, typeof els.forEach, typeof els.push].join(",") + "#, + ) + .unwrap(); + match result { + Value::String(s) => assert_eq!(s, "3,3,3,3,0,function,undefined"), + v => panic!("expected summary string, got {v:?}"), + } + } + #[test] fn test_get_elements_by_tag_name() { let result = eval_with_doc( diff --git a/crates/js/src/location.rs b/crates/js/src/location.rs index 3bfc5c3..debad15 100644 --- a/crates/js/src/location.rs +++ b/crates/js/src/location.rs @@ -483,6 +483,7 @@ mod tests { iframe_windows: RefCell::new(std::collections::HashMap::new()), location_object: RefCell::new(None), ready_state: RefCell::new("loading".to_string()), + node_list_prototype: RefCell::new(None), }) } diff --git a/crates/js/src/vm.rs b/crates/js/src/vm.rs index 2fb9460..bcd0a3b 100644 --- a/crates/js/src/vm.rs +++ b/crates/js/src/vm.rs @@ -660,6 +660,12 @@ pub struct DomBridge { pub location_object: RefCell>, /// The document's readyState: "loading", "interactive", or "complete". pub ready_state: RefCell, + /// Shared prototype for the array-like collections returned by + /// `querySelectorAll`, `getElementsBy*`, `.children`, etc. It borrows the + /// array prototype's iteration helpers so these `NodeList`/`HTMLCollection` + /// values are iterable with `for...of`/spread and expose `forEach`. + /// `None` until [`crate::dom_bridge::init_node_list_prototype`] runs. + pub node_list_prototype: RefCell>, } /// Context passed to native functions, providing GC access and `this` binding. @@ -1792,9 +1798,11 @@ impl Vm { iframe_windows: RefCell::new(HashMap::new()), location_object: RefCell::new(None), ready_state: RefCell::new("loading".to_string()), + node_list_prototype: RefCell::new(None), }); self.dom_bridge = Some(bridge); crate::dom_bridge::init_document_object(self); + crate::dom_bridge::init_node_list_prototype(self); crate::dom_bridge::init_html_image_api(self); crate::dom_bridge::init_event_system(self); crate::dom_bridge::init_storage_objects(self); @@ -3812,6 +3820,11 @@ impl Vm { roots.push(listener.callback); } } + // The shared NodeList prototype must survive collection so the + // collection objects that inherit from it stay iterable. + if let Some(r) = *bridge.node_list_prototype.borrow() { + roots.push(r); + } } // Pending microtask handlers and chained promises must be GC roots // so they survive a collection cycle between enqueue and drain.