forms: include optgroup children in a select's list of options#3058
Open
navidemad wants to merge 1 commit into
Open
forms: include optgroup children in a select's list of options#3058navidemad wants to merge 1 commit into
navidemad wants to merge 1 commit into
Conversation
HTMLSelectElement collected its options from its direct children only, so an
<option> inside an <optgroup> 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 <optgroup disabled>
(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 lightpanda-io#3057
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3057
HTMLSelectElementcollected its options from its direct children only, so an<option>inside an<optgroup>was invisible tooptions,length,value,selectedIndex,selectedOptionsand to form submission — whilequerySelectorAll("option")still found every option. A grouped select could not be read, set, or submitted.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
optgroupchildren.The path that changed
flowchart TD A["sel.value = 'b'<br/>(and options / length / selectedIndex / FormData)"] --> B{"how the options are enumerated"} B -->|"before — childrenIterator() / TreeWalker.Children"| C["direct children only<br/>options inside an optgroup never seen"] C --> D["no match<br/>selectedIndex = -1, value stays ''<br/>FormData entry dropped"] B -->|"after — OptionIterator / select_options walk"| E["children + optgroup children,<br/>in tree order"] E --> F["match<br/>value 'b', selectedIndex 2<br/>FormData entry 's=b'"]Fix
src/browser/webapi/collections/node_live.zig— newselect_optionsmode;selected_optionsmoves onto the same walk. Both now useTreeWalker.FullExcludeSelfwith a filter that accepts an option whose parent is the select or anoptgroupchild of the select, so an option buried any deeper is still excluded.src/browser/webapi/collections/HTMLCollection.zig— plumbing for the new mode (union arm, iterator walker type,nextTwarm).src/browser/webapi/element/html/Select.zig— oneOptionIteratorreplaces five hand-rolled child walks:effectiveOption,setValue,getSelectedIndex,setSelectedIndex,getLength, plus the index lookup inadd.getOptionsbuilds the new collection mode.Two behavioral details that come with making grouped options visible:
effectiveOptionnow asksElement.isDisabledinstead ofOption.getDisabled, so an option inside<optgroup disabled>is skipped when falling back to the first non-disabled option (concept-option-disabled). Without this, an option in a disabled group could become the implicit selection — only reachable now that those options are in the list at all.select.add(option, index)inserts into theoptgroupwhen the option at that index lives in one, per §dom-select-add (the insertion parent isbefore's parent).Tests
New fixture
src/browser/tests/element/html/select-optgroup.html, registered inWebApi: HTML.Select. Covers the options collection and its iterator, thevaluesetter/getter,option.selected,selectedIndex(including-1),selectedOptions,FormData, an explicitselectedattribute inside a group, the disabled-optgroup fallback,addwith an index inside a group, and that an option buried below anoptgroupis not in the list.Verification:
TEST_FILTER='WebApi: HTML.Select#select-optgroup.html' zig build test— passes.options.length(1 vs 3),value(""vs"b") andselectedIndex— so the fixture exercises the fix.zig build test— 1073/1073.1.0.0-nightly.8285(8 checks disagree with the spec), exit 0 on this branch (both selects identical, 9/9 each).