mirror of
https://github.com/lightpanda-io/browser.git
synced 2026-06-11 17:46:32 -04:00
Fix querySelector('#id') vs getElementById('id') disagreement
Frame.getElementByIdFromNode is the selector engine's fast path for
`#id` queries. It only consulted `_elements_by_id`, while
Document.getElementById and ShadowRoot.getElementById both fall back
to `_removed_ids` + TreeWalker to recover a surviving duplicate after
the first has been removed.
The two APIs could disagree after DOM manipulation that removes a
duplicate-ID element (e.g. Turbo Drive's PageRenderer.replaceBody
doing innerHTML + replaceWith):
document.querySelector('#page-title') => null
document.getElementById('page-title') => <h1>
Dispatch to the existing canonical getElementById on the scope
(ShadowRoot or Document) instead of keeping a third, map-only copy
of the lookup. The two APIs now agree by construction.
This commit is contained in:
@@ -1364,8 +1364,22 @@ pub fn removeElementIdWithMaps(self: *Frame, id_maps: ElementIdMaps, id: []const
|
||||
|
||||
pub fn getElementByIdFromNode(self: *Frame, node: *Node, id: []const u8) ?*Element {
|
||||
if (node.isConnected() or node.isInShadowTree()) {
|
||||
const lookup = self.getElementIdMap(node).lookup;
|
||||
return lookup.get(id);
|
||||
var current = node;
|
||||
while (true) {
|
||||
if (current.is(ShadowRoot)) |shadow_root| {
|
||||
return shadow_root.getElementById(id, self);
|
||||
}
|
||||
const parent = current._parent orelse {
|
||||
if (current._type == .document) {
|
||||
return current._type.document.getElementById(id, self);
|
||||
}
|
||||
if (IS_DEBUG) {
|
||||
std.debug.assert(false);
|
||||
}
|
||||
return null;
|
||||
};
|
||||
current = parent;
|
||||
}
|
||||
}
|
||||
var tw = @import("webapi/TreeWalker.zig").Full.Elements.init(node, .{});
|
||||
while (tw.next()) |el| {
|
||||
|
||||
@@ -17,3 +17,24 @@
|
||||
|
||||
// testing.expectEqual(null, document.getElementById('test'));
|
||||
</script>
|
||||
|
||||
<div id="qs-test">first</div>
|
||||
<div id="qs-test">second</div>
|
||||
|
||||
<script id=duplicateIdsQuerySelector>
|
||||
{
|
||||
// Regression test: querySelector('#id') must agree with getElementById('id')
|
||||
// when the first duplicate is removed. Selector engine fast path goes through
|
||||
// Frame.getElementByIdFromNode, which previously only checked the lookup map
|
||||
// and missed the _removed_ids recovery, so it returned null.
|
||||
const first = document.querySelector('#qs-test');
|
||||
testing.expectEqual('first', first.textContent);
|
||||
|
||||
first.remove();
|
||||
|
||||
testing.expectEqual('second', document.querySelector('#qs-test').textContent);
|
||||
testing.expectEqual('second', document.body.querySelector('#qs-test').textContent);
|
||||
testing.expectEqual(1, document.querySelectorAll('#qs-test').length);
|
||||
testing.expectEqual('second', document.getElementById('qs-test').textContent);
|
||||
}
|
||||
</script>
|
||||
|
||||
Reference in New Issue
Block a user