Implement <select> parser “relaxation” — for “customizable” <select>#113
Open
sideshowbarker wants to merge 6 commits intomasterfrom
Open
Implement <select> parser “relaxation” — for “customizable” <select>#113sideshowbarker wants to merge 6 commits intomasterfrom
sideshowbarker wants to merge 6 commits intomasterfrom
Conversation
cdf2934 to
a1d8a3d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
hsivonen
requested changes
Jan 23, 2026
Member
hsivonen
left a comment
There was a problem hiding this comment.
Sorry about the delay.
As discussed on Matrix, I think the way this patch makes the cloning work on what the parser saw is incorrect and the spec instead requires the cloning to work on what's in the live DOM, which may be different if the option element opens, the network stalls, setTimeout modifies the DOM, and network resumes.
Further remarks in an inline comment.
020921d to
f7b4c0f
Compare
This change, in order to support the “customizable” <select> element: - “relaxes” parsing rules in <select> (to allow more elements as children) - adds the <selectedcontent> element for cloning selected <option> content ElementName.java ---------------- - Add <selectedcontent> element with TreeBuilder.SELECTEDCONTENT group TreeBuilder.java ---------------- - Add SELECTEDCONTENT group constant and IN_SELECT_IN_CONTENT mode - Track selectedContentPointer and activeOptionStackPos for cloning - Allow <div>, <span>, and other elements in <select> in “relaxed” mode - Deep clone <option> content to <selectedcontent> when <option> closes - Handle <selectedcontent> special-casing (store pointer, no stack push) SAXTreeBuilder.java ------------------- - Implement clearSelectedContentChildren() for clearing before clone - Implement deepCloneChildren() and deepCloneNode() for <option> cloning ParentNode.java --------------- - Add clearChildren() method, to support clearing <selectedcontent> html5lib-tests submodule ------------------------ - Updated to pull in corresponding tests for “relaxed” <select> parsing
The initial implementation did inline cloning during parsing (mirroring elements/characters into <selectedcontent> as tokens were processed) AND deep-cloned from the DOM when option was popped. The inline cloning was wrong per spec — cloning should operate on the DOM, not on what the parser saw (the adoption agency algorithm can restructure the tree) — and redundant, since pop() cleared <selectedcontent> and deep-cloned from the DOM anyway. This replaces all inline cloning machinery with a single overridable hook method, cloneOptionContentToSelectedContent(), called when <option> is popped. The tree builder tracks which <option> is active and where <selectedcontent> is, but delegates the actual DOM cloning to subclasses.
…no longer needed
Add necessary corresponding implementations to the other tree builders that construct persistent trees: - DOMTreeBuilder: uses the DOM cloneNode method for deep cloning - XOMTreeBuilder: uses XOM’s Node.copy() for deep cloning - BrowserTreeBuilder: uses the existing cloneNodeDeep method
Add a getBuffer() accessor to CharBufferNode so that deepCloneNode() can copy directly from char[] to char[] via the Characters constructor, instead of going through toString() and toCharArray().
f7b4c0f to
87fe37a
Compare
hsivonen
reviewed
Feb 4, 2026
…comments Move selectedcontent cloning from TreeBuilder to DOM-side subclasses: the optionElementPopped() hook now walks the live DOM to find the ancestor <select> and its <selectedcontent>, check selectedness, and clone option children — rather than relying on parser-tracked state. Implemented in all four subclasses (DOMTreeBuilder, SAXTreeBuilder, XOMTreeBuilder, BrowserTreeBuilder). Remove all dead IN_SELECT / IN_SELECT_IN_TABLE mode code (handlers, constants, end-tag special cases) — with <select> “relaxation”, those modes are never entered. Fix resetTheInsertionMode to skip past <select> elements on the stack instead of incorrectly returning IN_BODY — an enclosing td/th now correctly yields IN_CELL, an enclosing table yields IN_TABLE, etc. Align start-tag handling with the spec: - HR: use generateImpliedEndTags() instead of manual isCurrent/pop - OPTION/OPTGROUP: remove redundant pop loops (spec says parse error only), add missing reconstructTheActiveFormattingElements for optgroup, fix optgroup to check both option and optgroup in scope - SELECT: check fragment case first, remove generateImpliedEndTags from nested case, add framesetOk = false - Table elements (caption, tbody, tr, td, th): treat as stray start tags, removing the old IN_SELECT_IN_TABLE dual-scope check Fold </select> end tag into the standard block-element group (with div, fieldset, button, etc.) — the dedicated resetTheInsertionMode call is no longer needed. Add spec URLs and verbatim spec-text comments throughout all <select>-related code sections — for traceability and review-ability.
0466829 to
b3bbf98
Compare
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.
This change, in order to support the "customizable"
<select>element:<select>(to allow more elements as children)<selectedcontent>element for cloning selected<option>contentElementName.java<selectedcontent>element withTreeBuilder.SELECTEDCONTENTgroupTreeBuilder.javaStructural changes:
IN_SELECT/IN_SELECT_IN_TABLEconstants and all associated mode handlers and end-tag special cases — with select "relaxation", those modes are never enteredresetTheInsertionModeto skip past<select>elements on the stack instead of incorrectly returningIN_BODY— an enclosing<td>/<th>now correctly yieldsIN_CELL, an enclosing<table>yieldsIN_TABLE, etc.Start-tag alignment with spec:
<hr>: usegenerateImpliedEndTags()instead of manualisCurrent/pop<option>: remove redundant pop loop (spec says parse error only aftergenerateImpliedEndTagsExceptFor("optgroup"))<optgroup>: remove redundant pop loop, add missingreconstructTheActiveFormattingElements(), fix parse error to check both<option>and<optgroup>in scope<select>: check fragment case first, removegenerateImpliedEndTagsfrom nested case, addframesetOk = false<input>: pop select and reprocess when select is in scope<caption>,<tbody>,<tr>,<td>,<th>): treat as stray start tags, removing the oldIN_SELECT_IN_TABLEdual-scope checkEnd-tag changes:
</select>into the standard block-element end-tag group (with</div>,</fieldset>,</button>, etc.)</option>and</optgroup>end-tag cases — "any other end tag" handles themCloning hook:
optionElementPopped()hook called from allpop()variantsSpec comments:
DOMTreeBuilder.javaoptionElementPopped(): walk ancestors to find<select>, find<selectedcontent>descendant, check selectedness, deep-clone option children viacloneNode(true)SAXTreeBuilder.javaoptionElementPopped()as no-op (streaming/SAX mode has no live DOM to clone into)XOMTreeBuilder.javaoptionElementPopped()using XOM'sNode.copy()for deep cloningBrowserTreeBuilder.javaoptionElementPopped()usingcloneNodeDeep()for deep cloningCharBufferNode.javagetBuffer()accessor for directchar[]access during cloningParentNode.javaclearChildren()method, to support clearing<selectedcontent>