From f239004bbbc0df80309d211491c88d4dfd29306f Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Sat, 25 Jul 2026 04:31:41 +0200 Subject: [PATCH] forms: include optgroup children in a select's list of options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit HTMLSelectElement collected its options from its direct children only, so an was invisible to options, length, value, selectedIndex, selectedOptions and to form submission. A grouped select could not be read, set or submitted, while querySelectorAll("option") still found every option. Per HTML §the-select-element a select's list of options is its option element descendants: its option children plus the option children of its optgroup children. - add a select_options collection mode, and move selected_options onto the same walk, matching an option whose parent is the select or an optgroup child of the select - add one OptionIterator in Select.zig, used by effectiveOption, setValue, getSelectedIndex, setSelectedIndex, getLength and add - effectiveOption skips options disabled through (concept-option-disabled); those options only become reachable now that grouped options are part of the list - select.add(option, index) inserts into the optgroup when the option at that index lives in one, per §dom-select-add Closes #3057 --- .../tests/element/html/select-optgroup.html | 170 ++++++++++++++++++ .../webapi/collections/HTMLCollection.zig | 7 +- src/browser/webapi/collections/node_live.zig | 24 ++- src/browser/webapi/element/html/Select.zig | 90 +++++++--- 4 files changed, 260 insertions(+), 31 deletions(-) create mode 100644 src/browser/tests/element/html/select-optgroup.html diff --git a/src/browser/tests/element/html/select-optgroup.html b/src/browser/tests/element/html/select-optgroup.html new file mode 100644 index 000000000..3fcfe0f2f --- /dev/null +++ b/src/browser/tests/element/html/select-optgroup.html @@ -0,0 +1,170 @@ + + + + + +
+ +
+ + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/browser/webapi/collections/HTMLCollection.zig b/src/browser/webapi/collections/HTMLCollection.zig index 4311623d2..57969c406 100644 --- a/src/browser/webapi/collections/HTMLCollection.zig +++ b/src/browser/webapi/collections/HTMLCollection.zig @@ -36,6 +36,7 @@ const Mode = enum { child_elements, child_tag, cells, + select_options, selected_options, links, anchors, @@ -54,6 +55,7 @@ _data: union(Mode) { child_elements: NodeLive(.child_elements), child_tag: NodeLive(.child_tag), cells: NodeLive(.cells), + select_options: NodeLive(.select_options), selected_options: NodeLive(.selected_options), links: NodeLive(.links), anchors: NodeLive(.anchors), @@ -111,6 +113,7 @@ pub fn iterator(self: *HTMLCollection, exec: *const Execution) !*Iterator { .child_elements => |*impl| .{ .child_elements = impl._tw.clone() }, .child_tag => |*impl| .{ .child_tag = impl._tw.clone() }, .cells => |*impl| .{ .cells = impl._tw.clone() }, + .select_options => |*impl| .{ .select_options = impl._tw.clone() }, .selected_options => |*impl| .{ .selected_options = impl._tw.clone() }, .links => |*impl| .{ .links = impl._tw.clone() }, .anchors => |*impl| .{ .anchors = impl._tw.clone() }, @@ -132,7 +135,8 @@ pub const Iterator = GenericIterator(struct { child_elements: TreeWalker.Children, child_tag: TreeWalker.Children, cells: TreeWalker.Children, - selected_options: TreeWalker.Children, + select_options: TreeWalker.FullExcludeSelf, + selected_options: TreeWalker.FullExcludeSelf, links: TreeWalker.FullExcludeSelf, anchors: TreeWalker.FullExcludeSelf, form: TreeWalker.FullExcludeSelf, @@ -157,6 +161,7 @@ pub const Iterator = GenericIterator(struct { .child_elements => |*impl| impl.nextTw(&self.tw.child_elements), .child_tag => |*impl| impl.nextTw(&self.tw.child_tag), .cells => |*impl| impl.nextTw(&self.tw.cells), + .select_options => |*impl| impl.nextTw(&self.tw.select_options), .selected_options => |*impl| impl.nextTw(&self.tw.selected_options), .links => |*impl| impl.nextTw(&self.tw.links), .anchors => |*impl| impl.nextTw(&self.tw.anchors), diff --git a/src/browser/webapi/collections/node_live.zig b/src/browser/webapi/collections/node_live.zig index 221075eb6..36c919732 100644 --- a/src/browser/webapi/collections/node_live.zig +++ b/src/browser/webapi/collections/node_live.zig @@ -40,6 +40,7 @@ const Mode = enum { child_elements, child_tag, cells, + select_options, selected_options, links, anchors, @@ -68,6 +69,7 @@ const Filters = union(Mode) { child_elements, child_tag: Element.Tag, cells, + select_options, selected_options, links, anchors, @@ -100,7 +102,10 @@ pub fn NodeLive(comptime mode: Mode) type { const Filter = Filters.TypeOf(mode); const TW = switch (mode) { .tag, .tag_name, .tag_name_ns, .class_name, .name, .all_elements, .links, .anchors, .form => TreeWalker.FullExcludeSelf, - .child_elements, .child_tag, .cells, .selected_options => TreeWalker.Children, + .child_elements, .child_tag, .cells => TreeWalker.Children, + // A select's options can sit one level down, inside an , so + // these two walk the subtree and filter on the parent instead. + .select_options, .selected_options => TreeWalker.FullExcludeSelf, }; return struct { _tw: TW, @@ -299,11 +304,23 @@ pub fn NodeLive(comptime mode: Mode) type { const el = node.is(Element) orelse return false; return el.is(Element.Html.TableCell) != null; }, - .selected_options => { + .select_options, .selected_options => { + // The select's list of options is its option children plus + // the option children of its optgroup children — NOT every + // option descendant. + // https://html.spec.whatwg.org/multipage/form-elements.html#the-select-element const el = node.is(Element) orelse return false; const Option = Element.Html.Option; const opt = el.is(Option) orelse return false; - return opt.getSelected(); + + const parent = node.parentNode() orelse return false; + if (parent != self._tw._root) { + const group = parent.is(Element) orelse return false; + if (group.getTag() != .optgroup) return false; + if (parent.parentNode() != self._tw._root) return false; + } + + return if (comptime mode == .selected_options) opt.getSelected() else true; }, .links => { // Links are elements with href attribute (TODO: also when implemented) @@ -390,6 +407,7 @@ pub fn NodeLive(comptime mode: Mode) type { .child_elements => HTMLCollection{ ._data = .{ .child_elements = self } }, .child_tag => HTMLCollection{ ._data = .{ .child_tag = self } }, .cells => HTMLCollection{ ._data = .{ .cells = self } }, + .select_options => HTMLCollection{ ._data = .{ .select_options = self } }, .selected_options => HTMLCollection{ ._data = .{ .selected_options = self } }, .links => HTMLCollection{ ._data = .{ .links = self } }, .anchors => HTMLCollection{ ._data = .{ .anchors = self } }, diff --git a/src/browser/webapi/element/html/Select.zig b/src/browser/webapi/element/html/Select.zig index 3c3a04a3d..62eb68329 100644 --- a/src/browser/webapi/element/html/Select.zig +++ b/src/browser/webapi/element/html/Select.zig @@ -48,6 +48,43 @@ pub fn asConstNode(self: *const Select) *const Node { return self.asConstElement().asConstNode(); } +// Walks the select's list of options in tree order: its option children plus +// the option children of its optgroup children, per HTML +// §the-select-element. Options nested any deeper are not in the list. +const OptionIterator = struct { + // Cursor over the select's children. + _child: ?*Node, + // Cursor over the current optgroup's children, while inside one. + _in_group: ?*Node = null, + + fn init(select: *const Select) OptionIterator { + return .{ ._child = select.asConstNode().firstChild() }; + } + + fn next(self: *OptionIterator) ?*Option { + while (true) { + if (self._in_group) |node| { + self._in_group = node.nextSibling(); + if (node.is(Option)) |option| { + return option; + } + continue; + } + + const node = self._child orelse return null; + self._child = node.nextSibling(); + + if (node.is(Option)) |option| { + return option; + } + const element = node.is(Element) orelse continue; + if (element.getTag() == .optgroup) { + self._in_group = node.firstChild(); + } + } + } +}; + // Resolves the option whose selectedness contributes to the select's value // per HTML §form-elements§selectedness-setting-algorithm: an explicitly // selected non-disabled option, falling back to the first non-disabled @@ -56,10 +93,12 @@ pub fn asConstNode(self: *const Select) *const Node { // and contributes no entry to a FormData set. pub fn effectiveOption(self: *const Select) ?*Option { var first_option: ?*Option = null; - var maybe_child = self.asConstNode().firstChild(); - while (maybe_child) |child| : (maybe_child = child.nextSibling()) { - const option = child.is(Option) orelse continue; - if (option.getDisabled()) continue; + var it = OptionIterator.init(self); + while (it.next()) |option| { + // Element.isDisabled, not Option.getDisabled: an option is also + // disabled when its parent is an + // (HTML "concept-option-disabled"). + if (option.asElement().isDisabled()) continue; if (option.getSelected()) return option; if (first_option == null) first_option = option; } @@ -77,9 +116,8 @@ pub fn setValue(self: *Select, value: []const u8, frame: *Frame) !void { // Find option with matching value and select it // Note: This updates the current state (_selected), not the default state (attribute) // Setting value always deselects all others, even for multiple selects - var iter = self.asNode().childrenIterator(); - while (iter.next()) |child| { - const option = child.is(Option) orelse continue; + var it = OptionIterator.init(self); + while (it.next()) |option| { option._selected = std.mem.eql(u8, option.getValue(frame), value); } } @@ -87,9 +125,8 @@ pub fn setValue(self: *Select, value: []const u8, frame: *Frame) !void { pub fn getSelectedIndex(self: *Select) i32 { var index: i32 = 0; var has_options = false; - var iter = self.asNode().childrenIterator(); - while (iter.next()) |child| { - const option = child.is(Option) orelse continue; + var it = OptionIterator.init(self); + while (it.next()) |option| { has_options = true; if (option.getSelected()) { return index; @@ -112,9 +149,8 @@ pub fn setSelectedIndex(self: *Select, index: i32) !void { // Note: This updates the current state (_selected), not the default state (attribute) const is_multiple = self.getMultiple(); var current_index: i32 = 0; - var iter = self.asNode().childrenIterator(); - while (iter.next()) |child| { - const option = child.is(Option) orelse continue; + var it = OptionIterator.init(self); + while (it.next()) |option| { if (current_index == index) { option._selected = true; } else if (!is_multiple) { @@ -200,8 +236,9 @@ pub fn setRequired(self: *Select, required: bool, frame: *Frame) !void { } pub fn getOptions(self: *Select, frame: *Frame) !*collections.HTMLOptionsCollection { - // For options, we use the child_tag mode to filter only