Skip to content
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

Loosen DOMString? getter to accommodate ARIA-style string reflection #10974

Merged
merged 7 commits into from
Feb 26, 2025

Conversation

rahimabdi
Copy link
Contributor

@rahimabdi rahimabdi commented Jan 31, 2025

Addresses #10037.

The ARIA WG is currently working on improving IDL which includes converting ARIA attributes (where applicable/possible) to be truly enumerated.

As a subtask of this general work, this PR loosens HTML spec to allow DOMString? reflection that is not limited to only known values. This is desirable for non-enumerated ARIA content attributes (e.g., aria-label, role) where the most appropriate IDL attribute type is DOMString? and the absence of the attribute's value is meaningful. This change will also ensure HTML-conformant reflection for future ARIA attributes that require string reflection.

CC @annevk

  • At least two implementers are interested (and none opposed):
    • N/A, already implemented
  • Tests are written and can be reviewed and commented upon at:
    • N/A, already tested
  • Implementation bugs are filed:
    • N/A, already implemented
  • Corresponding HTML AAM & ARIA in HTML issues & PRs: N/A
  • MDN issue is filed: N/A
  • The top of this comment includes a clear commit message to use.

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


/acknowledgements.html ( diff )
/common-dom-interfaces.html ( diff )

Sorry, something went wrong.

@domenic
Copy link
Member

domenic commented Jan 31, 2025

Can you clarify which of the paths outlined in #10037 this follows?

@rahimabdi
Copy link
Contributor Author

@domenic Apologies, please see this latest comment in the issue: #10037 (comment).

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.

Thanks! Couple more nits.

Rahim Abdi added 3 commits February 22, 2025 09:24
…extraneous null assignment removal step
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.

@rahimabdi thanks again for tackling this!

This looks good to me now. With this PR in place ARIA can use DOMString? and use the reflect language for IDL attributes whose content attributes are not defined as enumerated attributes. The meaning of which will be that if the content attribute is absent, the IDL attribute getter will return null. (And the IDL attribute setter will remove the content attribute when given the null value. Though this does not require further changes on the HTML side.)

While this was not the most desirable solution, it is the most reasonable one now that everyone has shipped. And in parallel ARIA folks will continue to investigate migrating their content attributes to become proper enumerated attributes as that would yield additional benefits, such as feature testing.

@domenic any further thoughts from you?

@domenic
Copy link
Member

domenic commented Feb 26, 2025

This looks good, although since we have decided on 1A instead of 2A, I wonder if we should have some sort of normative guidance in https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#using-reflect-in-specifications to avoid using DOMString? for non-enumerated attributes.

@annevk
Copy link
Member

annevk commented Feb 26, 2025

It would probably be good to have some more detailed advice on attribute design in general, but I would not really want to block this PR on that as this seems like a strict improvement over not having anything.

It's also quite nuanced as I don't think we generally prefer DOMString? even if the attribute is enumerated, as generally it seems preferable to have an explicit state over null.

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.

None yet

3 participants