Implement <select> parser “relaxation” — for “customizable” <select> #113
+531
−100
Conversation
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
sideshowbarker
force-pushed
the
sideshowbarker/relaxed-select-parsing
branch
2 times, most recently
from
cdf2934 to
a1d8a3d
Compare
Member
|
Sorry I missed this over the holidays. |
Member
Author
No worries — there's no real rush on it |
hsivonen
requested changes
Jan 23, 2026
Member
hsivonen
left a comment
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.
sideshowbarker
force-pushed
the
sideshowbarker/relaxed-select-parsing
branch
4 times, most recently
from
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().
sideshowbarker
force-pushed
the
sideshowbarker/relaxed-select-parsing
branch
from
f7b4c0f to
87fe37a
Compare
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”
selectelement:select(to allow more elements as children)selectedcontentelement for cloning selectedoptioncontentElementName.javaselectedcontentelement withTreeBuilder.SELECTEDCONTENTgroupTreeBuilder.javaSELECTEDCONTENTgroup constant andIN_SELECT_IN_CONTENTmodeselectedContentPointerandactiveOptionStackPosfor cloningdiv,span, and other elements inselectin “relaxed” modeoptioncontent toselectedcontentwhenoptionclosesselectedcontentspecial-casing (store pointer, no stack push)SAXTreeBuilder.javaclearSelectedContentChildren()for clearing before clonedeepCloneChildren()anddeepCloneNode()foroptioncloningParentNode.javaclearChildren()method, to support clearingselectedcontenthtml5lib-tests(submodule)selectparsing