From 96a4edc22ff2ca7c9a3e521fcda70d84672fc9b3 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Sat, 15 Aug 2026 15:50:35 +0800 Subject: [PATCH] webapi: improve xpath support Largely about passing the 1024 /domxpath/xml_xpath_runner.html WPT cases (0 passing before this). -evaluate/createExpression's `resolver` can be a function OR an callback object. We don't use it, so ?js.Function -> ?js.Value is an easy win -animated use legacy xlink:href attribute if present -better pseudo namespace support for attributes. We now preserve the prefix in the attribute name (so xlink:href stays xlink:href) AND, for 3 common namespaces with fixed prefixes, we getAttributeNS and hasAttributeNS _will_ work. (because these are fixed, it's ok that we don't store the namespace in attribute, we can just look for the fully qualified name and fallback to the original check if we don't find it). --- src/browser/frame/node_factory.zig | 17 ++++- src/browser/parser/Parser.zig | 4 +- src/browser/tests/domparser.html | 23 ++++++ .../tests/element/svg/animated_string.html | 29 ++++---- .../tests/xpath/document_evaluate.html | 18 +++++ src/browser/webapi/Document.zig | 4 +- src/browser/webapi/Element.zig | 74 ++++++++++--------- src/browser/webapi/XPathEvaluator.zig | 4 +- src/browser/webapi/element/Svg.zig | 1 - src/browser/webapi/svg/AnimatedString.zig | 14 +++- 10 files changed, 132 insertions(+), 56 deletions(-) diff --git a/src/browser/frame/node_factory.zig b/src/browser/frame/node_factory.zig index 9cd822889..6de08a90d 100644 --- a/src/browser/frame/node_factory.zig +++ b/src/browser/frame/node_factory.zig @@ -1087,10 +1087,25 @@ fn populateElementAttributes(frame: *Frame, element: *Element, list: anytype) !v var attributes = &element._attributes; try attributes.ensureTotalCapacity(count, frame); while (list.next()) |attr| { - try attributes.putNew(attr.name.local.slice(), attr.value.slice(), frame); + const name = try parserAttributeName(frame, attr.name); + try attributes.putNew(name, attr.value.slice(), frame); } } +// Attributes are keyed by qualified name (no namespace model), so a prefixed +// attribute (`xlink:href` in foreign content, `xml:id` in XML) must keep its +// prefix — that is what `getAttribute("xlink:href")` and `Attr.name` see in +// browsers. The joined name only has to outlive putNew, which canonicalizes +// it into the frame arena. (Not frame.buf: name normalization writes there.) +fn parserAttributeName(frame: *Frame, qname: Parser.QualName) ![]const u8 { + const local = qname.local.slice(); + const prefix = (qname.prefix.unwrap() orelse return local).slice(); + if (prefix.len == 0) { + return local; + } + return std.fmt.allocPrint(frame.local_arena, "{s}:{s}", .{ prefix, local }); +} + // Called when `new MyElement()` is invoked directly in JS (not via the // customElements.define/upgrade path). `new_target` is the constructor // function that was used with `new`. We find the matching definition in the diff --git a/src/browser/parser/Parser.zig b/src/browser/parser/Parser.zig index 2bbe994cb..f735916ad 100644 --- a/src/browser/parser/Parser.zig +++ b/src/browser/parser/Parser.zig @@ -25,6 +25,7 @@ const Node = @import("../webapi/Node.zig"); const Element = @import("../webapi/Element.zig"); const CData = @import("../webapi/CData.zig"); +pub const QualName = h5e.QualName; pub const AttributeIterator = h5e.AttributeIterator; const Allocator = std.mem.Allocator; @@ -453,7 +454,8 @@ fn createElementCallback(ctx: *anyopaque, data: *anyopaque, qname: h5e.QualName, } fn createXMLElementCallback(ctx: *anyopaque, data: *anyopaque, qname: h5e.QualName, attributes: h5e.AttributeIterator) callconv(.c) ?*anyopaque { - return _createElementCallbackWithDefaultnamespace(ctx, data, qname, attributes, .xml); + // An XML element outside any xmlns declaration is in no namespace (null namespace) + return _createElementCallbackWithDefaultnamespace(ctx, data, qname, attributes, .null); } // html5ever_parse_fragment materializes the fragment's context element through diff --git a/src/browser/tests/domparser.html b/src/browser/tests/domparser.html index 71640464e..9bb5ccde0 100644 --- a/src/browser/tests/domparser.html +++ b/src/browser/tests/domparser.html @@ -419,3 +419,26 @@ testing.expectEqual(2, allElements.length); } + + diff --git a/src/browser/tests/element/svg/animated_string.html b/src/browser/tests/element/svg/animated_string.html index 7a8520eca..21b3833aa 100644 --- a/src/browser/tests/element/svg/animated_string.html +++ b/src/browser/tests/element/svg/animated_string.html @@ -34,35 +34,34 @@ + + diff --git a/src/browser/webapi/Document.zig b/src/browser/webapi/Document.zig index a1e4364be..a90c7d14a 100644 --- a/src/browser/webapi/Document.zig +++ b/src/browser/webapi/Document.zig @@ -674,7 +674,7 @@ pub fn evaluate( self: *Document, expression: []const u8, context_node: ?*Node, - resolver: ?js.Function, + resolver: ?js.Value, result_type: ?u16, result: ?*XPathResult, frame: *Frame, @@ -697,7 +697,7 @@ pub fn evaluate( pub fn createExpression( _: *const Document, expression: []const u8, - resolver: ?js.Function, + resolver: ?js.Value, frame: *Frame, ) !*XPathExpression { _ = resolver; diff --git a/src/browser/webapi/Element.zig b/src/browser/webapi/Element.zig index a11caf0b9..30cc3927b 100644 --- a/src/browser/webapi/Element.zig +++ b/src/browser/webapi/Element.zig @@ -114,6 +114,9 @@ pub const Namespace = enum(u8) { pub fn parse(namespace_: ?[]const u8) Namespace { const namespace = namespace_ orelse return .null; + if (namespace.len == 0) { + return .null; + } if (namespace.len == "http://www.w3.org/1999/xhtml".len) { // Common case, avoid the string comparison. Recklessly @branchHint(.likely); @@ -652,22 +655,40 @@ pub fn getAttribute(self: *const Element, name: String, frame: *Frame) !?String return self._attributes.get(name, frame); } -/// For simplicity, the namespace is currently ignored and only the local name is used. pub fn getAttributeNS( self: *const Element, - maybe_namespace: ?[]const u8, + namespace_: ?[]const u8, local_name: String, frame: *Frame, ) !?String { - if (maybe_namespace) |namespace| { - if (!std.mem.eql(u8, namespace, "http://www.w3.org/1999/xhtml")) { - log.warn(.not_implemented, "Element.getAttributeNS", .{ .namespace = namespace }); + if (namespace_) |namespace| { + // we don't really support namespaces, but if the namespace has a fixed + // prefix, we can try to fetch the attribute with it + if (try prefixedAttributeName(namespace, local_name.str(), frame)) |prefixed| { + if (try self.getAttribute(.wrap(prefixed), frame)) |value| { + return value; + } } } - return self.getAttribute(local_name, frame); } +fn prefixedAttributeName(namespace: []const u8, local_name: []const u8, frame: *Frame) !?[]const u8 { + const prefix = blk: { + if (std.mem.eql(u8, namespace, "http://www.w3.org/1999/xlink")) { + break :blk "xlink"; + } + if (std.mem.eql(u8, namespace, "http://www.w3.org/XML/1998/namespace")) { + break :blk "xml"; + } + if (std.mem.eql(u8, namespace, "http://www.w3.org/2000/xmlns/")) { + break :blk "xmlns"; + } + return null; + }; + return try std.fmt.allocPrint(frame.local_arena, "{s}:{s}", .{ prefix, local_name }); +} + pub fn getAttributeSafe(self: *const Element, name: String) ?[]const u8 { return self._attributes.getSafe(name); } @@ -677,20 +698,13 @@ pub fn hasAttribute(self: *const Element, name: String, frame: *Frame) !bool { return value != null; } -/// Like getAttributeNS, the namespace is currently ignored. pub fn hasAttributeNS( self: *const Element, - maybe_namespace: ?[]const u8, + namespace_: ?[]const u8, local_name: String, frame: *Frame, ) !bool { - if (maybe_namespace) |namespace| { - if (!std.mem.eql(u8, namespace, "http://www.w3.org/1999/xhtml")) { - log.warn(.not_implemented, "Element.hasAttributeNS", .{ .namespace = namespace }); - } - } - - return self.hasAttribute(local_name, frame); + return try self.getAttributeNS(namespace_, local_name, frame) != null; } pub fn hasAttributeSafe(self: *const Element, name: String) bool { @@ -769,30 +783,24 @@ pub fn setAttribute(self: *Element, name: String, value: String, frame: *Frame) pub fn setAttributeNS( self: *Element, - maybe_namespace: ?[]const u8, + namespace_: ?[]const u8, qualified_name: []const u8, value: String, frame: *Frame, ) !void { - const attr_name = if (maybe_namespace) |namespace| blk: { - // For xmlns namespace, store the full qualified name (e.g. "xmlns:bar") - // so lookupNamespaceURI can find namespace declarations. - if (std.mem.eql(u8, namespace, "http://www.w3.org/2000/xmlns/")) { - break :blk qualified_name; + const local_start = if (std.mem.indexOfScalarPos(u8, qualified_name, 0, ':')) |idx| blk: { + if (idx == 0 or idx == qualified_name.len - 1) { + // cannot be at the start or end of the qname + return error.InvalidCharacterError; } - if (!std.mem.eql(u8, namespace, "http://www.w3.org/1999/xhtml")) { - log.warn(.not_implemented, "Element.setAttributeNS", .{ .namespace = namespace }); + if (std.mem.indexOfScalarPos(u8, qualified_name, idx + 1, ':') != null) { + // and can only have one + return error.InvalidCharacterError; } - break :blk if (std.mem.indexOfScalarPos(u8, qualified_name, 0, ':')) |idx| - qualified_name[idx + 1 ..] - else - qualified_name; - } else blk: { - break :blk if (std.mem.indexOfScalarPos(u8, qualified_name, 0, ':')) |idx| - qualified_name[idx + 1 ..] - else - qualified_name; - }; + break :blk idx + 1; + } else 0; + + const attr_name = if (namespace_ != null) qualified_name else qualified_name[local_start..]; return self.setAttribute(.wrap(attr_name), value, frame); } diff --git a/src/browser/webapi/XPathEvaluator.zig b/src/browser/webapi/XPathEvaluator.zig index c35a58178..5a5f28fcf 100644 --- a/src/browser/webapi/XPathEvaluator.zig +++ b/src/browser/webapi/XPathEvaluator.zig @@ -43,7 +43,7 @@ pub fn evaluate( _: *const XPathEvaluator, expression: []const u8, context_node: *Node, - resolver: ?js.Function, + resolver: ?js.Value, requested_type: ?u16, result: ?*XPathResult, frame: *Frame, @@ -59,7 +59,7 @@ pub fn evaluate( pub fn createExpression( _: *const XPathEvaluator, expression: []const u8, - resolver: ?js.Function, + resolver: ?js.Value, frame: *Frame, ) !*XPathExpression { _ = resolver; diff --git a/src/browser/webapi/element/Svg.zig b/src/browser/webapi/element/Svg.zig index 39960ec0b..70abc8412 100644 --- a/src/browser/webapi/element/Svg.zig +++ b/src/browser/webapi/element/Svg.zig @@ -194,6 +194,5 @@ pub const JsApi = struct { const testing = @import("../../../testing.zig"); test "WebApi: Svg" { - testing.expectLog(&.{ .not_implemented, .not_implemented }); try testing.htmlRunner("element/svg", .{}); } diff --git a/src/browser/webapi/svg/AnimatedString.zig b/src/browser/webapi/svg/AnimatedString.zig index d53de59bc..2ad187d58 100644 --- a/src/browser/webapi/svg/AnimatedString.zig +++ b/src/browser/webapi/svg/AnimatedString.zig @@ -67,8 +67,20 @@ pub fn getAnimVal(self: *const AnimatedString) []const u8 { fn attributeName(self: *const AnimatedString) String { return switch (self._kind) { - .href => comptime .wrap("href"), .class => comptime .wrap("class"), + .href => { + const href: String = comptime .wrap("href"); + if (self._element.hasAttributeSafe(href)) { + return href; + } + + const xlink_href: String = comptime .wrap("xlink:href"); + if (self._element.hasAttributeSafe(xlink_href)) { + // legacy attribute, returned if it exists and href doens't + return xlink_href; + } + return href; + }, }; }