public/browser
1
0
Fork 0
mirror of https://github.com/lightpanda-io/browser.git synced 2026-02-04 06:23:45 +00:00
Code Issues Actions 5 Packages Projects Releases Wiki Activity

getElementById duplicate-id handling

Browse Source 
If 2 elements have the same id then,
1 - The first in document-order has to be retrieved. We were ordering by
    insertion order.

2 - When the element is removed, then document.getElementById should return the
    next element with that id in document-order. We were returning null
This commit is contained in:
Karl Seguin
2026-01-07 15:06:18 +08:00
parent c92903aae5
commit 6f30d459d5
11 changed files with 148 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
src/browser/Page.zig
View File

@@ -995,21 +995,32 @@ pub fn domChanged(self: *Page) void {
};
}
fn getElementIdMap(page: *Page, node: *Node) *std.StringHashMapUnmanaged(*Element) {
const ElementIdMaps = struct { lookup: *std.StringHashMapUnmanaged(*Element), removed_ids: *std.StringHashMapUnmanaged(void) };
fn getElementIdMap(page: *Page, node: *Node) ElementIdMaps {
// Walk up the tree checking for ShadowRoot and tracking the root
var current = node;
while (true) {
if (current.is(ShadowRoot)) |shadow_root| {
return &shadow_root._elements_by_id;
return .{
.lookup = &shadow_root._elements_by_id,
.removed_ids = &shadow_root._removed_ids,
};
}
const parent = current._parent orelse {
if (current._type == .document) {
return &current._type.document._elements_by_id;
return .{
.lookup = &current._type.document._elements_by_id,
.removed_ids = &current._type.document._removed_ids,
};
}
// Detached nodes should not have IDs registered
std.debug.assert(false);
return &page.document._elements_by_id;
return .{
.lookup = &page.document._elements_by_id,
.removed_ids = &page.document._removed_ids,
};
};
current = parent;
@@ -1017,22 +1028,35 @@ fn getElementIdMap(page: *Page, node: *Node) *std.StringHashMapUnmanaged(*Elemen
}
pub fn addElementId(self: *Page, parent: *Node, element: *Element, id: []const u8) !void {
var id_map = self.getElementIdMap(parent);
const gop = try id_map.getOrPut(self.arena, id);
var id_maps = self.getElementIdMap(parent);
const gop = try id_maps.lookup.getOrPut(self.arena, id);
if (!gop.found_existing) {
gop.value_ptr.* = element;
return;
}
const existing = gop.value_ptr.*.asNode();
switch (element.asNode().compareDocumentPosition(existing)) {
0x04 => gop.value_ptr.* = element,
else => {},
}
}
pub fn removeElementId(self: *Page, element: *Element, id: []const u8) void {
var id_map = self.getElementIdMap(element.asNode());
_ = id_map.remove(id);
const node = element.asNode();
self.removeElementIdWithMaps(self.getElementIdMap(node), id);
}
pub fn removeElementIdWithMaps(self: *Page, id_maps: ElementIdMaps, id: []const u8) void {
if (id_maps.lookup.remove(id)) {
id_maps.removed_ids.put(self.arena, id, {}) catch {};
}
}
pub fn getElementByIdFromNode(self: *Page, node: *Node, id: []const u8) ?*Element {
if (node.isConnected() or node.isInShadowTree()) {
const id_map = self.getElementIdMap(node);
return id_map.get(id);
const lookup = self.getElementIdMap(node).lookup;
return lookup.get(id);
}
var tw = @import("webapi/TreeWalker.zig").Full.Elements.init(node, .{});
while (tw.next()) |el| {
@@ -2097,7 +2121,7 @@ pub fn removeNode(self: *Page, parent: *Node, child: *Node, opts: RemoveNodeOpts
// grab this before we null the parent
const was_connected = child.isConnected();
// Capture the ID map before disconnecting, so we can remove IDs from the correct document
const id_map = if (was_connected) self.getElementIdMap(child) else null;
const id_maps = if (was_connected) self.getElementIdMap(child) else null;
child._parent = null;
child._child_link = .{};
@@ -2148,7 +2172,7 @@ pub fn removeNode(self: *Page, parent: *Node, child: *Node, opts: RemoveNodeOpts
var tw = @import("webapi/TreeWalker.zig").Full.Elements.init(child, .{});
while (tw.next()) |el| {
if (el.getAttributeSafe("id")) |id| {
_ = id_map.?.remove(id);
self.removeElementIdWithMaps(id_maps.?, id);
}
Element.Html.Custom.invokeDisconnectedCallbackOnElement(el, self);

5
src/browser/js/ExecutionWorld.zig
View File

@@ -184,9 +184,10 @@ pub fn unknownPropertyCallback(c_name: ?*const v8.C_Name, raw_info: ?*const v8.C
if (maybe_property) |prop| {
if (!ignored.has(prop)) {
const document = context.page.document;
const page = context.page;
const document = page.document;
if (document.getElementById(prop)) |el| {
if (document.getElementById(prop, page)) |el| {
const js_value = context.zigValueToJs(el, .{}) catch {
return v8.Intercepted.No;
};

19
src/browser/tests/element/duplicate_ids.html Normal file
View File

@@ -0,0 +1,19 @@
<!DOCTYPE html>
<script src="../testing.js"></script>
<div id="test">first</div>
<div id="test">second</div>
<script id=duplicateIds>
const first = document.getElementById('test');
testing.expectEqual('first', first.textContent);
first.remove();
const second = document.getElementById('test');
testing.expectEqual('second', second.textContent);
// second.remove();
// testing.expectEqual(null, document.getElementById('test'));
</script>

35
src/browser/webapi/Document.zig
View File

@@ -48,6 +48,8 @@ _location: ?*Location = null,
_ready_state: ReadyState = .loading,
_current_script: ?*Element.Html.Script = null,
_elements_by_id: std.StringHashMapUnmanaged(*Element) = .empty,
// Track IDs that were removed from the map - they might have duplicates in the tree
_removed_ids: std.StringHashMapUnmanaged(void) = .empty,
_active_element: ?*Element = null,
_style_sheets: ?*StyleSheetList = null,
_write_insertion_point: ?*Node = null,
@@ -163,11 +165,32 @@ pub fn createAttributeNS(_: *const Document, namespace: []const u8, name: []cons
});
}
pub fn getElementById(self: *const Document, id: []const u8) ?*Element {
pub fn getElementById(self: *Document, id: []const u8, page: *Page) ?*Element {
if (id.len == 0) {
return null;
}
return self._elements_by_id.get(id);
if (self._elements_by_id.get(id)) |element| {
return element;
}
//ID was removed but might have duplicates
if (self._removed_ids.remove(id)) {
var tw = @import("TreeWalker.zig").Full.Elements.init(self.asNode(), .{});
while (tw.next()) |el| {
const element_id = el.getAttributeSafe("id") orelse continue;
if (std.mem.eql(u8, element_id, id)) {
// we ignore this error to keep getElementById easy to call
// if it really failed, then we're out of memory and nothing's
// going to work like it should anyways.
const owned_id = page.dupeString(id) catch return null;
self._elements_by_id.put(page.arena, owned_id, el) catch return null;
return el;
}
}
}
return null;
}
const GetElementsByTagNameResult = union(enum) {
@@ -688,15 +711,15 @@ pub const JsApi = struct {
pub const createTreeWalker = bridge.function(Document.createTreeWalker, .{});
pub const createNodeIterator = bridge.function(Document.createNodeIterator, .{});
pub const getElementById = bridge.function(_getElementById, .{});
fn _getElementById(self: *const Document, value_: ?js.Value) !?*Element {
fn _getElementById(self: *Document, value_: ?js.Value, page: *Page) !?*Element {
const value = value_ orelse return null;
if (value.isNull()) {
return self.getElementById("null");
return self.getElementById("null", page);
}
if (value.isUndefined()) {
return self.getElementById("undefined");
return self.getElementById("undefined", page);
}
return self.getElementById(try value.toZig([]const u8));
return self.getElementById(try value.toZig([]const u8), page);
}
pub const querySelector = bridge.function(Document.querySelector, .{ .dom_exception = true });
pub const querySelectorAll = bridge.function(Document.querySelectorAll, .{ .dom_exception = true });

19
src/browser/webapi/DocumentFragment.zig
View File

@@ -71,8 +71,10 @@ pub fn className(_: *const DocumentFragment) []const u8 {
return "[object DocumentFragment]";
}
pub fn getElementById(self: *DocumentFragment, id_: ?[]const u8) ?*Element {
const id = id_ orelse return null;
pub fn getElementById(self: *DocumentFragment, id: []const u8) ?*Element {
if (id.len == 0) {
return null;
}
var tw = @import("TreeWalker.zig").Full.Elements.init(self.asNode(), .{});
while (tw.next()) |el| {
@@ -239,7 +241,18 @@ pub const JsApi = struct {
pub const constructor = bridge.constructor(DocumentFragment.init, .{});
pub const getElementById = bridge.function(DocumentFragment.getElementById, .{});
pub const getElementById = bridge.function(_getElementById, .{});
fn _getElementById(self: *DocumentFragment, value_: ?js.Value) !?*Element {
const value = value_ orelse return null;
if (value.isNull()) {
return self.getElementById("null");
}
if (value.isUndefined()) {
return self.getElementById("undefined");
}
return self.getElementById(try value.toZig([]const u8));
}
pub const querySelector = bridge.function(DocumentFragment.querySelector, .{ .dom_exception = true });
pub const querySelectorAll = bridge.function(DocumentFragment.querySelectorAll, .{ .dom_exception = true });
pub const children = bridge.accessor(DocumentFragment.getChildren, null, .{});

44
src/browser/webapi/ShadowRoot.zig
View File

@@ -39,6 +39,7 @@ _proto: *DocumentFragment,
_mode: Mode,
_host: *Element,
_elements_by_id: std.StringHashMapUnmanaged(*Element) = .{},
_removed_ids: std.StringHashMapUnmanaged(void) = .{},
pub fn init(host: *Element, mode: Mode, page: *Page) !*ShadowRoot {
return page._factory.documentFragment(ShadowRoot{
@@ -72,9 +73,34 @@ pub fn getHost(self: *const ShadowRoot) *Element {
return self._host;
}
pub fn getElementById(self: *ShadowRoot, id_: ?[]const u8) ?*Element {
const id = id_ orelse return null;
return self._elements_by_id.get(id);
pub fn getElementById(self: *ShadowRoot, id: []const u8, page: *Page) ?*Element {
if (id.len == 0) {
return null;
}
// Fast path: ID is in the map
if (self._elements_by_id.get(id)) |element| {
return element;
}
// Slow path: ID was removed but might have duplicates
if (self._removed_ids.remove(id)) {
// Do a tree walk to find another element with this ID
var tw = @import("TreeWalker.zig").Full.Elements.init(self.asNode(), .{});
while (tw.next()) |el| {
const element_id = el.getAttributeSafe("id") orelse continue;
if (std.mem.eql(u8, element_id, id)) {
// we ignore this error to keep getElementById easy to call
// if it really failed, then we're out of memory and nothing's
// going to work like it should anyways.
const owned_id = page.dupeString(id) catch return null;
self._elements_by_id.put(page.arena, owned_id, el) catch return null;
return el;
}
}
}
return null;
}
pub const JsApi = struct {
@@ -88,7 +114,17 @@ pub const JsApi = struct {
pub const mode = bridge.accessor(ShadowRoot.getMode, null, .{});
pub const host = bridge.accessor(ShadowRoot.getHost, null, .{});
pub const getElementById = bridge.function(ShadowRoot.getElementById, .{});
pub const getElementById = bridge.function(_getElementById, .{});
fn _getElementById(self: *ShadowRoot, value_: ?js.Value, page: *Page) !?*Element {
const value = value_ orelse return null;
if (value.isNull()) {
return self.getElementById("null", page);
}
if (value.isUndefined()) {
return self.getElementById("undefined", page);
}
return self.getElementById(try value.toZig([]const u8), page);
}
};
const testing = @import("../../testing.zig");

2
src/browser/webapi/collections/node_live.zig
View File

@@ -171,7 +171,7 @@ pub fn NodeLive(comptime mode: Mode) type {
}
pub fn getByName(self: *Self, name: []const u8, page: *Page) ?*Element {
if (page.document.getElementById(name)) |element| {
if (page.document.getElementById(name, page)) |element| {
const node = element.asNode();
if (self._tw.contains(node) and self.matches(node)) {
return element;

2
src/browser/webapi/element/html/Button.zig
View File

@@ -91,7 +91,7 @@ pub fn getForm(self: *Button, page: *Page) ?*Form {
// If form attribute exists, ONLY use that (even if it references nothing)
if (element.getAttributeSafe("form")) |form_id| {
if (page.document.getElementById(form_id)) |form_element| {
if (page.document.getElementById(form_id, page)) |form_element| {
return form_element.is(Form);
}
// form attribute present but invalid - no form owner

2
src/browser/webapi/element/html/Input.zig
View File

@@ -264,7 +264,7 @@ pub fn getForm(self: *Input, page: *Page) ?*Form {
// If form attribute exists, ONLY use that (even if it references nothing)
if (element.getAttributeSafe("form")) |form_id| {
if (page.document.getElementById(form_id)) |form_element| {
if (page.document.getElementById(form_id, page)) |form_element| {
return form_element.is(Form);
}
// form attribute present but invalid - no form owner

2
src/browser/webapi/element/html/Select.zig
View File

@@ -219,7 +219,7 @@ pub fn getForm(self: *Select, page: *Page) ?*Form {
// If form attribute exists, ONLY use that (even if it references nothing)
if (element.getAttributeSafe("form")) |form_id| {
if (page.document.getElementById(form_id)) |form_element| {
if (page.document.getElementById(form_id, page)) |form_element| {
return form_element.is(Form);
}
// form attribute present but invalid - no form owner

2
src/browser/webapi/element/html/TextArea.zig
View File

@@ -90,7 +90,7 @@ pub fn getForm(self: *TextArea, page: *Page) ?*Form {
// If form attribute exists, ONLY use that (even if it references nothing)
if (element.getAttributeSafe("form")) |form_id| {
if (page.document.getElementById(form_id)) |form_element| {
if (page.document.getElementById(form_id, page)) |form_element| {
return form_element.is(Form);
}
// form attribute present but invalid - no form owner