Skip to content

Commit 71ae39e

Browse files
authored
BUGFIX: Correct position checking logic in details element (#128)
The original code compared `child_idx != parent.first_child_idx` which didn't account for skipped text/comment nodes. For example, prior to this change the summary element in the following code was considered in the wrong position: <details> <!-- Comment --> <summary></summary> </details> After changing to track flow elements instead it is valid.
1 parent 2fc7507 commit 71ae39e

1 file changed

Lines changed: 64 additions & 1 deletion

File tree

src/html/elements/details.zig

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ pub fn validateContent(
100100
const parent_span = parent.span(src);
101101

102102
var summary_span: ?Span = null;
103+
var seen_flow_element = false;
103104

104105
var child_idx = parent.first_child_idx;
105106
while (child_idx != 0) {
@@ -119,7 +120,7 @@ pub fn validateContent(
119120
});
120121
} else {
121122
summary_span = child.span(src);
122-
if (child_idx != parent.first_child_idx) {
123+
if (seen_flow_element) {
123124
try errors.append(gpa, .{
124125
.tag = .{ .wrong_position = .first },
125126
.main_location = summary_span.?,
@@ -133,6 +134,8 @@ pub fn validateContent(
133134
.main_location = child.span(src),
134135
.node_idx = child_idx,
135136
});
137+
} else {
138+
seen_flow_element = true;
136139
}
137140
}
138141

@@ -173,3 +176,63 @@ fn completionsContent(
173176
.{},
174177
);
175178
}
179+
180+
fn expectError(errors: []const Ast.Error, comptime tag: std.meta.FieldEnum(@TypeOf(errors[0].tag))) bool {
181+
for (errors) |err| {
182+
if (err.tag == tag) return true;
183+
}
184+
return false;
185+
}
186+
187+
test "details: valid - summary as first element child" {
188+
const case = "<details><summary>Title</summary><p>Content</p></details>";
189+
const ast = try Ast.init(std.testing.allocator, case, .html, false);
190+
defer ast.deinit(std.testing.allocator);
191+
try std.testing.expectEqual(0, ast.errors.len);
192+
}
193+
194+
test "details: valid - summary with whitespace before" {
195+
const case =
196+
\\<details>
197+
\\ <summary>Title</summary>
198+
\\</details>
199+
;
200+
const ast = try Ast.init(std.testing.allocator, case, .html, false);
201+
defer ast.deinit(std.testing.allocator);
202+
try std.testing.expectEqual(0, ast.errors.len);
203+
}
204+
205+
test "details: valid - summary with comment before" {
206+
const case = "<details><!-- comment --><summary>Title</summary></details>";
207+
const ast = try Ast.init(std.testing.allocator, case, .html, false);
208+
defer ast.deinit(std.testing.allocator);
209+
try std.testing.expectEqual(0, ast.errors.len);
210+
}
211+
212+
test "details: error - missing summary" {
213+
const case = "<details><p>Content</p></details>";
214+
const ast = try Ast.init(std.testing.allocator, case, .html, false);
215+
defer ast.deinit(std.testing.allocator);
216+
try std.testing.expect(expectError(ast.errors, .missing_child));
217+
}
218+
219+
test "details: error - summary not first (flow element before)" {
220+
const case = "<details><p>Content</p><summary>Title</summary></details>";
221+
const ast = try Ast.init(std.testing.allocator, case, .html, false);
222+
defer ast.deinit(std.testing.allocator);
223+
try std.testing.expect(expectError(ast.errors, .wrong_position));
224+
}
225+
226+
test "details: error - duplicate summary" {
227+
const case = "<details><summary>A</summary><summary>B</summary></details>";
228+
const ast = try Ast.init(std.testing.allocator, case, .html, false);
229+
defer ast.deinit(std.testing.allocator);
230+
try std.testing.expect(expectError(ast.errors, .duplicate_child));
231+
}
232+
233+
test "details: error - invalid child (non-flow content)" {
234+
const case = "<details><summary>Title</summary><col></details>";
235+
const ast = try Ast.init(std.testing.allocator, case, .html, false);
236+
defer ast.deinit(std.testing.allocator);
237+
try std.testing.expect(expectError(ast.errors, .invalid_nesting));
238+
}

0 commit comments

Comments
 (0)