Implement <select> parser “relaxation” — for “customizable” <select> #113

sideshowbarker · 2025-12-30T02:22:06Z

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
When <option> is popped, deep-clone its content to <selectedcontent> via an overridable cloneOptionContentToSelectedContent() hook — operating on the live DOM, not parser state (so adoption-agency restructuring is correctly reflected)
Handle <selectedcontent> special-casing (store pointer, no stack push)
Add cloning hook to all pop variants (pop(), popForeign(), silentPop(), popOnEof())

`SAXTreeBuilder.java`

Implement cloneOptionContentToSelectedContent(): clears <selectedcontent> children, then deep-clones from the live DOM via getFirstChild()/getNextSibling() traversal
Avoid intermediate String allocation when cloning Characters nodes (use getBuffer() directly)

`DOMTreeBuilder.java`

Implement cloneOptionContentToSelectedContent() using DOM cloneNode(true)

`XOMTreeBuilder.java`

Implement cloneOptionContentToSelectedContent() using XOM’s Node.copy()

`BrowserTreeBuilder.java`

Implement cloneOptionContentToSelectedContent() using cloneNodeDeep()

`CharBufferNode.java`

Add getBuffer() accessor for direct char[] access during cloning

`ParentNode.java`

Add clearChildren() method, to support clearing <selectedcontent>

hsivonen · 2026-01-12T12:54:06Z

Sorry I missed this over the holidays.

sideshowbarker · 2026-01-12T13:19:53Z

Sorry I missed this over the holidays.

No worries — there's no real rush on it

hsivonen

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.

src/nu/validator/htmlparser/sax/SAXTreeBuilder.java

src/nu/validator/htmlparser/impl/TreeBuilder.java

src/nu/validator/htmlparser/impl/ElementName.java

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().

hsivonen · 2026-02-04T13:16:28Z

src/nu/validator/htmlparser/impl/TreeBuilder.java

                                attributes = null; // CPP
                                break starttagloop;
                            case HR:
+                                // Check if select is in scope


The spec seems to implicitly close "p" before the "select" check.

hsivonen · 2026-02-04T13:20:02Z

src/nu/validator/htmlparser/impl/TreeBuilder.java

+                                // Check if select is in scope
+                                if (findLastInScope("select") != TreeBuilder.NOT_FOUND_ON_STACK) {
+                                    // Close any open option or optgroup first
+                                    if (isCurrent("option")) {


Why are "option" and "optgroup" popped separately instead of calling generateImpliedEndTags();?

hsivonen · 2026-02-04T13:22:59Z

src/nu/validator/htmlparser/impl/TreeBuilder.java

+                                attributes = null; // CPP
+                                break starttagloop;
                            case INPUT:
+                                // Check if select is in scope and close it (for compatibility)


It seems that the fragment-case special case if context is "select" is missing.

hsivonen · 2026-02-04T13:24:25Z

src/nu/validator/htmlparser/impl/TreeBuilder.java

+                                    while (currentPtr >= eltPos) {
+                                        pop();
+                                    }
+                                    resetTheInsertionMode();


Please add a comment that explains this.

hsivonen · 2026-02-04T13:58:33Z

src/nu/validator/htmlparser/impl/TreeBuilder.java

                                attributes = null; // CPP
                                break starttagloop;
                            case TEXTAREA:
+                                // Check if select is in scope and close it (for compatibility)


I don't find spec text corresponding to this. Why is this needed?

hsivonen · 2026-02-04T14:19:53Z

src/nu/validator/htmlparser/impl/TreeBuilder.java

+                                    if (eltPos != TreeBuilder.NOT_FOUND_ON_STACK) {
+                                        if (errorHandler != null && !isCurrent("optgroup")) {
+                                            errUnclosedElementsImplied(eltPos, "optgroup");
+                                        }


It appears that also "option" being in scope is an error per spec.

hsivonen · 2026-02-04T14:28:55Z

src/nu/validator/htmlparser/impl/TreeBuilder.java

                            case TBODY_OR_THEAD_OR_TFOOT:
                            case TR:
                            case TD_OR_TH:
+                                // Check if we're inside a select inside a table


How does this map to the spec?

hsivonen · 2026-02-04T14:29:37Z

src/nu/validator/htmlparser/impl/TreeBuilder.java

                                        attributes, formPointer);
                                attributes = null; // CPP
                                break starttagloop;
+                            case SELECTEDCONTENT:


It seems to me the tree builder shouldn't know about selectedcontent and looking for it is a DOM-side operation.

hsivonen · 2026-02-04T14:36:25Z

src/nu/validator/htmlparser/impl/TreeBuilder.java

                                }
                            }
                            break endtagloop;
+                        case OPTION:


It seems to me that "Any other end tag" should continue to handle these, except poping an "option" (regardless of tag) should trigger the DOM-side cloning.

hsivonen · 2026-02-04T14:39:43Z

src/nu/validator/htmlparser/impl/TreeBuilder.java

                        case TEMPLATE:
                            // fall through to IN_HEAD;
                            break;
+                        case SELECT:


Why is this handled in a select-specific way instead of being bundled together with various block-ish elements?

sideshowbarker requested review from alphan102 and hsivonen December 30, 2025 02:22

sideshowbarker force-pushed the sideshowbarker/relaxed-select-parsing branch 2 times, most recently from cdf2934 to a1d8a3d Compare December 30, 2025 08:00

sideshowbarker mentioned this pull request Dec 30, 2025

Run html5lib-tests against real browsers html5lib/html5lib-tests#188

Open

hsivonen requested changes Jan 23, 2026

View reviewed changes

sideshowbarker requested a review from hsivonen February 1, 2026 19:57

sideshowbarker force-pushed the sideshowbarker/relaxed-select-parsing branch 4 times, most recently from 020921d to f7b4c0f Compare February 2, 2026 05:31

sideshowbarker added 5 commits February 2, 2026 14:35

Remove TreeBuilder.SELECTEDCONTENT from treeBuilderGroupToName()

4c01288

…no longer needed

Avoid intermediate String allocation when cloning Characters nodes

87fe37a

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 February 2, 2026 05:35

hsivonen reviewed Feb 4, 2026

View reviewed changes

Implement <select> parser “relaxation” — for “customizable” <select> #113

Are you sure you want to change the base?

Implement <select> parser “relaxation” — for “customizable” <select> #113

Conversation

sideshowbarker commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ElementName.java

TreeBuilder.java

SAXTreeBuilder.java

DOMTreeBuilder.java

XOMTreeBuilder.java

BrowserTreeBuilder.java

CharBufferNode.java

ParentNode.java

Uh oh!

hsivonen commented Jan 12, 2026

Uh oh!

sideshowbarker commented Jan 12, 2026

Uh oh!

hsivonen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sideshowbarker commented Dec 30, 2025 •

edited

Loading

`ElementName.java`

`TreeBuilder.java`

`SAXTreeBuilder.java`

`DOMTreeBuilder.java`

`XOMTreeBuilder.java`

`BrowserTreeBuilder.java`

`CharBufferNode.java`

`ParentNode.java`