Skip to content

Fix button type getter steps #11047

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

Merged
merged 6 commits into from
Feb 20, 2025
Merged

Fix button type getter steps #11047

merged 6 commits into from
Feb 20, 2025

Conversation

lukewarlow
Copy link
Member

@lukewarlow lukewarlow commented Feb 19, 2025

Fix button type getter steps

Buttons in the Auto state that are submit buttons now reflect to submit correctly

Fixes #11046

  • At least two implementers are interested (and none opposed):
  • Tests are written and can be reviewed and commented upon at:
  • Implementation bugs are filed:
    • Chromium: …
    • Gecko: …
    • WebKit: …
    • Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): …
    • Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): …
  • Corresponding HTML AAM & ARIA in HTML issues & PRs:
  • MDN issue is filed: …
  • The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)


/form-elements.html ( diff )

Buttons in the Auto state that are submit buttons now reflect to submit correctly
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I think it would be clearer to return "submit" if it's a submit button first. Then "button" if it's in the Auto state. And then the value corresponding to state. Possibly with an assert that state cannot be the Submit Button state in there.

@lukewarlow lukewarlow requested a review from annevk February 20, 2025 09:21
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I also moved the Assert up. Looks good to me now. Can you confirm it still looks good to you @lukewarlow?

@annevk annevk merged commit 7568930 into whatwg:main Feb 20, 2025
2 checks passed
@lukewarlow
Copy link
Member Author

I think we maybe both reordered it and it got confused it seems to be below the state var still but looks good to me

@annevk
Copy link
Member

annevk commented Feb 20, 2025

Well it should be below the state variable initialization, because it uses that. But it doesn't have to be below the Auto state check.

@lukewarlow
Copy link
Member Author

Ah apologies I was confusing the assert and the submit button check.

AtkinsSJ added a commit to AtkinsSJ/ladybird that referenced this pull request Feb 20, 2025
Corresponds to part of whatwg/html#9841 and then
whatwg/html#11047

Adding `Auto` as a type state feels a little odd, as it's not an actual
type allowed in HTML. However, it's the default state when the value is
missing or invalid, which works out the same, as long as we never
serialize "auto", which we don't.
AtkinsSJ added a commit to AtkinsSJ/ladybird that referenced this pull request Feb 21, 2025
Corresponds to part of whatwg/html#9841 and then
whatwg/html#11047

Adding `Auto` as a type state feels a little odd, as it's not an actual
type allowed in HTML. However, it's the default state when the value is
missing or invalid, which works out the same, as long as we never
serialize "auto", which we don't.
tcl3 pushed a commit to LadybirdBrowser/ladybird that referenced this pull request Feb 22, 2025
Corresponds to part of whatwg/html#9841 and then
whatwg/html#11047

Adding `Auto` as a type state feels a little odd, as it's not an actual
type allowed in HTML. However, it's the default state when the value is
missing or invalid, which works out the same, as long as we never
serialize "auto", which we don't.
F3n67u pushed a commit to F3n67u/ladybird that referenced this pull request Mar 5, 2025
Corresponds to part of whatwg/html#9841 and then
whatwg/html#11047

Adding `Auto` as a type state feels a little odd, as it's not an actual
type allowed in HTML. However, it's the default state when the value is
missing or invalid, which works out the same, as long as we never
serialize "auto", which we don't.
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.

Button type getter ambiguous
3 participants