-
Notifications
You must be signed in to change notification settings - Fork 3k
Make option disabledness look for ancestor optgroups #11721
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Now that option elements can be descendants of optgroups instead of just children, the disabledness algorithm should look for ancestors intead of just a parent optgroup. Fixes whatwg#11707
I had a peak at Chromium's code for this and |
source
Outdated
attribute is present.</p> | ||
data-x="attr-option-disabled">disabled</code> attribute is present or if it is a | ||
<span>descendant</span> of an <code>optgroup</code> element whose <code | ||
data-x="attr-optgroup-disabled">disabled</code> attribute is present.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it also be disabled when we have <optgroup disabled><optgroup><option>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nested optgroups are something that we decided not to support, so in this case I don't think it should be disabled. I changed the text to look for the nearest ancestor optgroup instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add a test for this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I also have some other reservations. Is <select disabled><option>
not disabled? What about <optgroup disabled><select><option>
? (Not sure what the right answers are here, but it should be clear and we need test coverage, even if they are edge cases.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<select disabled><option>
is not disabled, and <optgroup disabled><select><option>
is also not disabled with the latest commit which aborts if it finds a select element before an optgroup. I will add tests.
Thanks, I am fixing this in my patch which adds the WPT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe at some point we should formally define nearest and furthest, but this seems fine for now. Thanks!
Should this stop going through ancestors if a Also see handling of |
Yes, I pushed a commit to abort if a select element is found. |
Now that option elements can be descendants of optgroups instead of just children, the disabledness algorithm should look for ancestors intead of just a parent optgroup.
Fixes #11707
(See WHATWG Working Mode: Changes for more details.)
/form-elements.html ( diff )