Repository navigation
Match controls disabled by an ancestor fieldset - #1913
feritcemkahraman wants to merge 2 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description Check | |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: matching controls disabled by an ancestor fieldset. |
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
A rabbit checks each fieldset gate
The first legend marks a different state
Selects pass the setting down
To options gathered all around
Tests trace the paths, both wide and small
Then hops away beyond them all
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply to a comment to ask cubic a question or push back. It learns from your replies.
View guided diff | Re-trigger cubic
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f8e728e785
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| fieldset[disabled]:not(fieldset[disabled] legend:first-of-type *) | ||
| :is(button, input, select, textarea, fieldset):is( | ||
| fieldset[disabled] > :not(legend:first-of-type), | ||
| fieldset[disabled] > :not(legend:first-of-type) * |
There was a problem hiding this comment.
When the search root is below the fieldset—for example <fieldset disabled><div id="root"><input></div></fieldset> followed by selectAll(":disabled", root)—this nested combinator is compiled with root as its context. With the default relativeSelector handling, fieldset[disabled] must then be root or its descendant, so the actual ancestor cannot satisfy this branch; the input is omitted from :disabled and subsequently matches :enabled. Disabled state should not change with the chosen search subtree, so this ancestor check needs to run without that context restriction.
Useful? React with 👍 / 👎.
| optgroup[disabled] > option, | ||
| fieldset[disabled]:not(fieldset[disabled] legend:first-of-type *) | ||
| :is(button, input, select, textarea, fieldset)${disabledByFieldset}, | ||
| :is(optgroup, option):is(select[disabled] *, select${disabledByFieldset} *) |
There was a problem hiding this comment.
For <select disabled><option>X</option></select>, this branch makes the option match :disabled instead of :enabled. Options do not inherit a select's disabled state. An option is disabled by its own attribute or a disabled parent optgroup; an optgroup needs its own attribute.
The same mistake occurs when a fieldset disables the select, so callers get the wrong query results in both cases. Remove this branch and update the tests that expect inherited state.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95cc9e3b17
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| optgroup[disabled] > option, | ||
| fieldset[disabled]:not(fieldset[disabled] legend:first-of-type *) | ||
| :is(button, input, select, textarea, fieldset)${disabledByFieldset}, | ||
| :is(optgroup, option):is(select[disabled] *, select${disabledByFieldset} *) |
There was a problem hiding this comment.
For <select disabled>...</select>, HTML only makes the select itself disabled: an optgroup needs its own disabled attribute, while an option needs its own attribute or a disabled parent optgroup. This branch newly makes both descendants match :disabled (and disappear from :enabled) for every disabled select, including selects disabled by a fieldset, reversing the prior and browser behavior; retain fieldset inheritance on the select without propagating it to these descendants.
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
For
<fieldset disabled><input></fieldset>,:disabledcurrently matches only the fieldset and:enabledmatches the input. Apply fieldset inheritance outside each ancestor's first direct legend, including when the disabled ancestor is above the query root. Custom pseudo aliases keep their existing context behavior.Options and optgroups also inherit their select's disabled state, as specified by the current HTML Standard. The installed Chromium differs for this case, so those expectations follow the standard.
Regressions were verified before their respective fixes. All 227 tests pass (one existing skip); ESLint, TypeScript, build, and changed-file Biome pass. Full Biome has existing CRLF formatting failures. Tests cover nested legends, explicit disabled states, scoped queries, and custom aliases. Firefox, WebKit, and benchmarks were not run.
Summary by CodeRabbit
:disabledmatching to account for disabled fieldsets and their supported form controls, including nested fieldsets, select options, and optgroups. Controls inside the first direct legend remain exempt from inherited disabled state, while explicitly disabled fieldsets are still matched.:enabledmatching for options and ensured disabled-state results remain consistent in scoped queries and element checks.