Implement <select> parsing changes
#1610
Conversation
Bumps [test/data/html5lib-tests](https://github.com/html5lib/html5lib-tests) from `a9f4496` to `8f43b7e`. - [Commits](html5lib/html5lib-tests@a9f4496...8f43b7e) --- updated-dependencies: - dependency-name: test/data/html5lib-tests dependency-version: 8f43b7ec8c9d02179f5f38e0ea08cb5000fb9c9e dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
dependabot
bot
added
dependencies
fb55
changed the title
a9f4496 to 8f43b7e<select> parsing changes
There was a problem hiding this comment.
Pull request overview
This PR updates the HTML5 parser to implement relaxed <select> element parsing rules as specified in the HTML spec update (whatwg/html#10548). The changes remove the dedicated "in select" and "in select in table" insertion modes, integrate select-related parsing directly into the "in body" mode, and add support for the new <selectedcontent> element with automatic content cloning from selected options.
Key changes:
- Removes
IN_SELECTandIN_SELECT_IN_TABLEinsertion modes - Adds
<selectedcontent>element support with automatic cloning of selected option content - Updates
<select>to be a scoping element and modifies scope checking logic - Updates test suite reference to include new HTML5lib tests for relaxed select parsing
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/data/html5lib-tests | Updates submodule to version containing new select parsing tests |
| packages/parse5/lib/parser/open-element-stack.ts | Adds SELECT to scoping elements, removes hasInSelectScope method |
| packages/parse5/lib/parser/open-element-stack.test.ts | Removes tests for the deleted hasInSelectScope method |
| packages/parse5/lib/parser/index.ts | Removes select-specific insertion modes, adds selectedcontent cloning logic and integrates select parsing into body mode |
| packages/parse5/lib/parser/index.test.ts | Adds test exclusions for cases testing old select parsing behavior |
| packages/parse5/lib/common/html.ts | Adds SELECTEDCONTENT tag and SELECTED attribute constants |
| packages/parse5-parser-stream/test/parser-stream.test.ts | Adds same test exclusions as parser tests |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Selectedcontent cloning helpers | ||
|
|
||
| /** @internal */ | ||
| /** @internal */ |
There was a problem hiding this comment.
Remove duplicate @internal JSDoc comment on line 331. The comment already appears on line 330.
| /** @internal */ |
| this.selectedOptionInSelect = null; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Add JSDoc comment for _maybeCloneOptionIntoSelectedcontent method to document its purpose and the selectedcontent cloning logic.
| /** | |
| * Clones the children of the appropriate <option> element into the selectedcontent element. | |
| * | |
| * This is used when finalizing a <select> element with enabled selectedcontent. | |
| * The method determines which <option> to clone: | |
| * - If there is a selectedOptionInSelect (an <option> with the 'selected' attribute), its children are cloned. | |
| * - Otherwise, the children of the firstOptionInSelect (the first <option> in the <select>) are cloned. | |
| * The children are deeply cloned and appended to the selectedcontent element. | |
| * If no suitable <option> is found, no cloning occurs. | |
| */ |
| this._cloneChildrenInto(optionToClone, this.enabledSelectedcontent); | ||
| } | ||
|
|
||
| /** @internal */ |
There was a problem hiding this comment.
Add JSDoc comment for _cloneChildrenInto method to document parameters and cloning behavior.
| /** @internal */ | |
| /** | |
| * Clones all child nodes from the `source` element into the `target` element. | |
| * First, removes all existing children from the `target`. | |
| * Then, deep-clones each child node from `source` and appends it to `target`. | |
| * | |
| * @param {T['element']} source - The element whose children will be cloned. | |
| * @param {T['element']} target - The element to receive the cloned children. | |
| * @internal | |
| */ |
| this.treeAdapter.appendChild(target, cloned); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Add JSDoc comment for _deepCloneNode method to document the recursive cloning logic and return value.
| /** | |
| * Recursively creates a deep clone of the given node and all its descendants. | |
| * - For text nodes, returns a new text node with the same content. | |
| * - For comment nodes, returns a new comment node with the same content. | |
| * - For element nodes, creates a new element with the same tag name, namespace, and attributes, | |
| * then recursively clones and appends all child nodes. | |
| * - For other node types (e.g., document type), returns the node itself (no cloning). | |
| * | |
| * @param node The node to deep clone. | |
| * @returns A deep clone of the input node, including all child nodes. | |
| */ |
| // Track this as the enabled selectedcontent if: | ||
| // 1. We're inside a select (selectedcontentSelect is set) | ||
| // 2. We don't already have an enabled selectedcontent | ||
| // 3. The select doesn't have the 'multiple' attribute (checked via selectedcontentSelect being set) |
There was a problem hiding this comment.
The comment states that the 'multiple' attribute check is done via selectedcontentSelect being set, but there's no actual check for the 'multiple' attribute in this code. Either add the missing validation or update the comment to accurately reflect the implementation.
| // 3. The select doesn't have the 'multiple' attribute (checked via selectedcontentSelect being set) | |
| // (Note: This does not check for the 'multiple' attribute on the select element) |
Bumps test/data/html5lib-tests from
a9f4496to8f43b7e.Commits
8f43b7eTest multiple option elements against selectedcontentcfb6639Add select particular element in scope testf994590Update tests for relaxed <select> parserYou can trigger a rebase of this PR by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)