Skip to content

Conversation

@annevk
Copy link
Member

@annevk annevk commented Nov 4, 2025

And also has an unused ancestor variable.


/form-elements.html ( diff )

We don't use this ancestor variable. (The for each loop creates its own.)
@annevk annevk requested a review from josepharhar November 4, 2025 07:46
@annevk annevk changed the title Remove no-op step from selectedcontent post-connection steps selectedcontent post-connection steps did not break out of the loop Nov 4, 2025
@@ -59187,12 +59185,15 @@ interface <dfn interface>HTMLSelectedContentElement</dfn> : <span>HTMLElement</s

<li><p>Otherwise, set <var>selectedcontent</var>'s <span
data-x="selectedcontent-disabled">disabled</span> state to true.</p></li>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should move the below break into this "otherwise" condition. If we always break when the ancestor is a select element, then we can't detect the nested select elements case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also what the chromium implementation does

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I think this is lacking test coverage.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although if we end up finding ourselves in a nested situation, why do we still run update select's selectedcontents on the innermost select?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants