Skip to content

Commit e26d4af

Browse files
karlseguinkarlseguin
authored
Merge pull request #963 from lightpanda-io/wpt_runner_fix_and_nodeiterator_tweak
Improves correctness of NodeIterator
2 parents 11485d2 + b9ae4c6 commit e26d4af

File tree

5 files changed

+143
-29
lines changed

5 files changed

+143
-29
lines changed

‎src/browser/dom/document.zig‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -249,12 +249,12 @@ pub const Document = struct {
249249
return Node.replaceChildren(parser.documentToNode(self), nodes);
250250
}
251251

252-
pub fn _createTreeWalker(_: *parser.Document, root: *parser.Node, what_to_show: ?u32, filter: ?TreeWalker.TreeWalkerOpts) !TreeWalker {
253-
return try TreeWalker.init(root, what_to_show, filter);
252+
pub fn _createTreeWalker(_: *parser.Document, root: *parser.Node, what_to_show: ?TreeWalker.WhatToShow, filter: ?TreeWalker.TreeWalkerOpts) !TreeWalker {
253+
return TreeWalker.init(root, what_to_show, filter);
254254
}
255255

256-
pub fn _createNodeIterator(_: *parser.Document, root: *parser.Node, what_to_show: ?u32, filter: ?NodeIterator.NodeIteratorOpts) !NodeIterator {
257-
return try NodeIterator.init(root, what_to_show, filter);
256+
pub fn _createNodeIterator(_: *parser.Document, root: *parser.Node, what_to_show: ?NodeIterator.WhatToShow, filter: ?NodeIterator.NodeIteratorOpts) !NodeIterator {
257+
return NodeIterator.init(root, what_to_show, filter);
258258
}
259259

260260
pub fn getActiveElement(self: *parser.Document, page: *Page) !?*parser.Element {

‎src/browser/dom/node_iterator.zig‎

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,32 +17,42 @@
1717
// along with this program. If not, see <https://www.gnu.org/licenses/>.
1818

1919
const std = @import("std");
20+
2021
const parser = @import("../netsurf.zig");
2122
const Env = @import("../env.zig").Env;
2223
const NodeFilter = @import("node_filter.zig");
2324
const Node = @import("node.zig").Node;
2425
const NodeUnion = @import("node.zig").Union;
26+
const DOMException = @import("exceptions.zig").DOMException;
2527

2628
// https://developer.mozilla.org/en-US/docs/Web/API/NodeIterator
2729
// While this is similar to TreeWalker it has its own implementation as there are several subtle differences
2830
// For example:
2931
// - nextNode returns the reference node, whereas TreeWalker returns the next node
3032
// - Skip and reject are equivalent for NodeIterator, for TreeWalker they are different
3133
pub const NodeIterator = struct {
34+
pub const Exception = DOMException;
35+
3236
root: *parser.Node,
3337
reference_node: *parser.Node,
3438
what_to_show: u32,
3539
filter: ?NodeIteratorOpts,
3640
filter_func: ?Env.Function,
37-
3841
pointer_before_current: bool = true,
42+
// used to track / block recursive filters
43+
is_in_callback: bool = false,
44+
45+
// One of the few cases where null and undefined resolve to different default.
46+
// We need the raw JsObject so that we can probe the tri state:
47+
// null, undefined or i32.
48+
pub const WhatToShow = Env.JsObject;
3949

4050
pub const NodeIteratorOpts = union(enum) {
4151
function: Env.Function,
4252
object: struct { acceptNode: Env.Function },
4353
};
4454

45-
pub fn init(node: *parser.Node, what_to_show: ?u32, filter: ?NodeIteratorOpts) !NodeIterator {
55+
pub fn init(node: *parser.Node, what_to_show_: ?WhatToShow, filter: ?NodeIteratorOpts) !NodeIterator {
4656
var filter_func: ?Env.Function = null;
4757
if (filter) |f| {
4858
filter_func = switch (f) {
@@ -51,10 +61,21 @@ pub const NodeIterator = struct {
5161
};
5262
}
5363

64+
var what_to_show: u32 = undefined;
65+
if (what_to_show_) |wts| {
66+
switch (try wts.triState(NodeIterator, "what_to_show", u32)) {
67+
.null => what_to_show = 0,
68+
.undefined => what_to_show = NodeFilter.NodeFilter._SHOW_ALL,
69+
.value => |v| what_to_show = v,
70+
}
71+
} else {
72+
what_to_show = NodeFilter.NodeFilter._SHOW_ALL;
73+
}
74+
5475
return .{
5576
.root = node,
5677
.reference_node = node,
57-
.what_to_show = what_to_show orelse NodeFilter.NodeFilter._SHOW_ALL,
78+
.what_to_show = what_to_show,
5879
.filter = filter,
5980
.filter_func = filter_func,
6081
};
@@ -81,9 +102,13 @@ pub const NodeIterator = struct {
81102
}
82103

83104
pub fn _nextNode(self: *NodeIterator) !?NodeUnion {
84-
if (self.pointer_before_current) { // Unlike TreeWalker, NodeIterator starts at the first node
85-
self.pointer_before_current = false;
105+
try self.callbackStart();
106+
defer self.callbackEnd();
107+
108+
if (self.pointer_before_current) {
109+
// Unlike TreeWalker, NodeIterator starts at the first node
86110
if (.accept == try NodeFilter.verify(self.what_to_show, self.filter_func, self.reference_node)) {
111+
self.pointer_before_current = false;
87112
return try Node.toInterface(self.reference_node);
88113
}
89114
}
@@ -107,13 +132,19 @@ pub const NodeIterator = struct {
107132
}
108133

109134
pub fn _previousNode(self: *NodeIterator) !?NodeUnion {
135+
try self.callbackStart();
136+
defer self.callbackEnd();
137+
110138
if (!self.pointer_before_current) {
111-
self.pointer_before_current = true;
112139
if (.accept == try NodeFilter.verify(self.what_to_show, self.filter_func, self.reference_node)) {
113-
return try Node.toInterface(self.reference_node); // Still need to verify as last may be first as well
140+
self.pointer_before_current = true;
141+
// Still need to verify as last may be first as well
142+
return try Node.toInterface(self.reference_node);
114143
}
115144
}
116-
if (self.reference_node == self.root) return null;
145+
if (self.reference_node == self.root) {
146+
return null;
147+
}
117148

118149
var current = self.reference_node;
119150
while (try parser.nodePreviousSibling(current)) |previous| {
@@ -151,6 +182,11 @@ pub const NodeIterator = struct {
151182
return null;
152183
}
153184

185+
pub fn _detach(self: *const NodeIterator) void {
186+
// no-op as per spec
187+
_ = self;
188+
}
189+
154190
fn firstChild(self: *const NodeIterator, node: *parser.Node) !?*parser.Node {
155191
const children = try parser.nodeGetChildNodes(node);
156192
const child_count = try parser.nodeListLength(children);
@@ -217,6 +253,18 @@ pub const NodeIterator = struct {
217253

218254
return null;
219255
}
256+
257+
fn callbackStart(self: *NodeIterator) !void {
258+
if (self.is_in_callback) {
259+
// this is the correct DOMExeption
260+
return error.InvalidState;
261+
}
262+
self.is_in_callback = true;
263+
}
264+
265+
fn callbackEnd(self: *NodeIterator) void {
266+
self.is_in_callback = false;
267+
}
220268
};
221269

222270
const testing = @import("../../testing.zig");

‎src/browser/dom/tree_walker.zig‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,17 @@ pub const TreeWalker = struct {
3333
filter: ?TreeWalkerOpts,
3434
filter_func: ?Env.Function,
3535

36+
// One of the few cases where null and undefined resolve to different default.
37+
// We need the raw JsObject so that we can probe the tri state:
38+
// null, undefined or i32.
39+
pub const WhatToShow = Env.JsObject;
40+
3641
pub const TreeWalkerOpts = union(enum) {
3742
function: Env.Function,
3843
object: struct { acceptNode: Env.Function },
3944
};
4045

41-
pub fn init(node: *parser.Node, what_to_show: ?u32, filter: ?TreeWalkerOpts) !TreeWalker {
46+
pub fn init(node: *parser.Node, what_to_show_: ?WhatToShow, filter: ?TreeWalkerOpts) !TreeWalker {
4247
var filter_func: ?Env.Function = null;
4348

4449
if (filter) |f| {
@@ -48,10 +53,21 @@ pub const TreeWalker = struct {
4853
};
4954
}
5055

56+
var what_to_show: u32 = undefined;
57+
if (what_to_show_) |wts| {
58+
switch (try wts.triState(TreeWalker, "what_to_show", u32)) {
59+
.null => what_to_show = 0,
60+
.undefined => what_to_show = NodeFilter.NodeFilter._SHOW_ALL,
61+
.value => |v| what_to_show = v,
62+
}
63+
} else {
64+
what_to_show = NodeFilter.NodeFilter._SHOW_ALL;
65+
}
66+
5167
return .{
5268
.root = node,
5369
.current_node = node,
54-
.what_to_show = what_to_show orelse NodeFilter.NodeFilter._SHOW_ALL,
70+
.what_to_show = what_to_show,
5571
.filter = filter,
5672
.filter_func = filter_func,
5773
};

‎src/main_wpt.zig‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -263,30 +263,33 @@ const Writer = struct {
263263
if (line.len == 0) {
264264
break;
265265
}
266-
var fields = std.mem.splitScalar(u8, line, '|');
267-
const case_name = fields.next() orelse {
268-
std.debug.print("invalid result line: {s}\n", .{line});
269-
return error.InvalidResult;
270-
};
271-
272-
const text_status = fields.next() orelse {
273-
std.debug.print("invalid result line: {s}\n", .{line});
274-
return error.InvalidResult;
275-
};
276-
277-
const case_pass = std.mem.eql(u8, text_status, "Pass");
278-
if (case_pass) {
266+
// case names can have | in them, so we can't simply split on |
267+
var case_name = line;
268+
var case_pass = false; // so pessimistic!
269+
var case_message: []const u8 = "";
270+
271+
if (std.mem.endsWith(u8, line, "|Pass")) {
272+
case_name = line[0 .. line.len - 5];
273+
case_pass = true;
279274
case_pass_count += 1;
280275
} else {
281-
// If 1 case fails, we treat the entire file as a fail.
276+
// both cases names and messages can have | in them. Our only
277+
// chance to "parse" this is to anchor off the |Fail.
278+
const pos = std.mem.indexOf(u8, line, "|Fail") orelse {
279+
std.debug.print("invalid result line: {s}\n", .{line});
280+
return error.InvalidResult;
281+
};
282+
283+
case_name = line[0..pos];
284+
case_message = line[pos + 1 ..];
282285
pass = false;
283286
case_fail_count += 1;
284287
}
285288

286289
try cases.append(self.arena, .{
287290
.name = case_name,
288291
.pass = case_pass,
289-
.message = fields.next(),
292+
.message = case_message,
290293
});
291294
}
292295

‎src/runtime/js.zig‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -941,6 +941,27 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
941941
fn jsValueToZig(self: *JsContext, comptime named_function: NamedFunction, comptime T: type, js_value: v8.Value) !T {
942942
switch (@typeInfo(T)) {
943943
.optional => |o| {
944+
if (comptime isJsObject(o.child)) {
945+
// If type type is a ?JsObject, then we want to pass
946+
// a JsObject, not null. Consider a function,
947+
// _doSomething(arg: ?Env.JsObjet) void { ... }
948+
//
949+
// And then these two calls:
950+
// doSomething();
951+
// doSomething(null);
952+
//
953+
// In the first case, we'll pass `null`. But in the
954+
// second, we'll pass a JsObject which represents
955+
// null.
956+
// If we don't have this code, both cases will
957+
// pass in `null` and the the doSomething won't
958+
// be able to tell if `null` was explicitly passed
959+
// or whether no parameter was passed.
960+
return JsObject{
961+
.js_obj = js_value.castTo(v8.Object),
962+
.js_context = self,
963+
};
964+
}
944965
if (js_value.isNullOrUndefined()) {
945966
return null;
946967
}
@@ -1938,6 +1959,24 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
19381959
return try js_context.createFunction(js_value);
19391960
}
19401961

1962+
pub fn isNull(self: JsObject) bool {
1963+
return self.js_obj.toValue().isNull();
1964+
}
1965+
1966+
pub fn isUndefined(self: JsObject) bool {
1967+
return self.js_obj.toValue().isUndefined();
1968+
}
1969+
1970+
pub fn triState(self: JsObject, comptime Struct: type, comptime name: []const u8, comptime T: type) !TriState(T) {
1971+
if (self.isNull()) {
1972+
return .{ .null = {} };
1973+
}
1974+
if (self.isUndefined()) {
1975+
return .{ .undefined = {} };
1976+
}
1977+
return .{ .value = try self.toZig(Struct, name, T) };
1978+
}
1979+
19411980
pub fn isNullOrUndefined(self: JsObject) bool {
19421981
return self.js_obj.toValue().isNullOrUndefined();
19431982
}
@@ -1965,6 +2004,14 @@ pub fn Env(comptime State: type, comptime WebApis: type) type {
19652004
const named_function = comptime NamedFunction.init(Struct, name);
19662005
return self.js_context.jsValueToZig(named_function, T, self.js_obj.toValue());
19672006
}
2007+
2008+
pub fn TriState(comptime T: type) type {
2009+
return union(enum) {
2010+
null: void,
2011+
undefined: void,
2012+
value: T,
2013+
};
2014+
}
19682015
};
19692016

19702017
// This only exists so that we know whether a function wants the opaque

0 commit comments

Comments
 (0)