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

LibWeb: Sync AriaRoles.json w/ Characteristics data in current spec #3640

Merged

Conversation

sideshowbarker
Copy link
Contributor

@sideshowbarker sideshowbarker commented Feb 20, 2025

So I did a complete audit (manually) of our AriaRoles.json file against every single one of the Characteristics tables in the ARIA spec — at, for example, https://w3c.github.io/aria/#application — and patched the AriaRoles.json file to match the current data in the spec.

When I started on it, I had optimistically imagined I was only going to find a small number of differences between the file and the spec. But I ended up finding a lot more than a few.

Most of the changes are to match the Supported States and Properties and Inherited States and Properties data — but some changes are to match the Name From data and Accessible Name Required data.

I also intentionally removed all states and properties the Characteristics tables list as “deprecated on this role in ARIA 1.2”.

The rationale for dropping the deprecated states and properties is just that developers shouldn’t be using those — and so we don’t need or want to be tracking data for them in our AriaRoles.json file. It’s just a waste of time all around.


In more detail:

  • The Characteristics tables in the spec don’t state UA requirements for implementors but instead state document/authoring-conformance requirements.

  • So the corresponding data in the AriaRoles.json file — because it’s just based on the Characteristics tables in the spec — has no affect on any of the actual application behavior of Ladybird as far as handling of web content.

  • Instead that data in the AriaRoles.json file is used only in our devtools — for the convenience of web developers,so that they can view the information at point-of-use within devtools, rather than needing to go look at the spec.

Given all that, we don’t want or need to be exposing information about deprecated states and properties to developers in our devtools — because it’s pointless, confusing, non-convenient, and counter-productive to developers to be exposing information to them about stuff that they’re not meant to be using anyway.

@sideshowbarker sideshowbarker force-pushed the aria-roles-data-audit branch 2 times, most recently from b7f9922 to 471af79 Compare February 20, 2025 07:13
This change updates the contents of the AriaRoles.json file to match the
data in the Characteristics tables in the ARIA spec — at, for example,
https://w3c.github.io/aria/#application.

Most of the changes are to match the “Supported States and Properties”
and “Inherited States and Properties” data — but some changes are to
match the “Name From” data and “Accessible Name Required” data.

This change also intentionally removes all states and properties the
Characteristics tables list as “deprecated on this role in ARIA 1.2”.
@Psychpsyo
Copy link
Contributor

Psychpsyo commented Feb 20, 2025

I think having information about deprecated states and properties could make sense.
If a developer is (incorrectly) setting those on an element, the devtools could inform them that "Hey, you should not be doing this."

@AtkinsSJ
Copy link
Member

I guess it depends what this is really for. If it's purely to make Ladybird work, then dropping deprecated stuff makes sense. But if we want it to be a machine-readable form of the ARIA data, then keeping deprecated things in there, marked as deprecated, could be useful. I don't have much opinion either way.

@sideshowbarker
Copy link
Contributor Author

I think having information about deprecated states and properties could make sense. If a developer is (incorrectly) setting those on an element, the devtools could inform them that "Hey, you should not be doing this."

Yeah, it’s imaginable having a tool somewhere which does that could be useful to developers — but I think having devtools in Ladybird be that place would not be a good fit.

So unless you feel strongly what we should change the behavior from what we currently have, to making it do something special for “deprecated” stuff, I’d personally rather we don’t venture down that route.

I mean, "Hey, you should not be doing this" is already what’s meant by the absence of a state or property in the information which devtools shows for a particular role. It simply indicates to developers what they should or should not be using.

So the problem with keeping the deprecated states and properties in the AriaRoles.jsondata is that they currently don’t show up with any special "Hey, you should not be doing this" indicator. They instead just show up in devtools as if they’re states and properties that developers should be using — that is, the very opposite of what should be happening.

Changing the devtools behavior to instead do special-casing for them would take actual new work — new code to have them show up differently, and some kind of new schema for the JSON data that’d add a new field for them.

And because I personally think we actually shouldn’t be doing anything new and special for them in devtools, then I personally at least wouldn’t be enthusiastic at all about being the person to write that code and change the JSON schema and then update the AriaRoles.json file for all these cases.

I think it’s instead actually better to just drop them all from the data — which is why that’s what I did in this patch.

@Psychpsyo
Copy link
Contributor

I think having information about deprecated states and properties could make sense. If a developer is (incorrectly) setting those on an element, the devtools could inform them that "Hey, you should not be doing this."

Yeah, it’s imaginable having a tool somewhere which does that could be useful to developers — but I think having devtools in Ladybird be that place would not be a good fit.

So unless you feel strongly what we should change the behavior from what we currently have, to making it do something special for “deprecated” stuff, I’d personally rather we don’t venture down that route.

I mean, "Hey, you should not be doing this" is already what’s meant by the absence of a state or property in the information which devtools shows for a particular role. It simply indicates to developers what they should or should not be using.

So the problem with keeping the deprecated states and properties in the AriaRoles.jsondata is that they currently don’t show up with any special "Hey, you should not be doing this" indicator. They instead just show up in devtools as if they’re states and properties that developers should be using — that is, the very opposite of what should be happening.

Changing the devtools behavior to instead do special-casing for them would take actual new work — new code to have them show up differently, and some kind of new schema for the JSON data that’d add a new field for them.

And because I personally think we actually shouldn’t be doing anything new and special for them in devtools, then I personally at least wouldn’t be enthusiastic at all about being the person to write that code and change the JSON schema and then update the AriaRoles.json file for all these cases.

I think it’s instead actually better to just drop them all from the data — which is why that’s what I did in this patch.

Fair.
Dropping them for now seems like the correct option then.

@sideshowbarker
Copy link
Contributor Author

I guess it depends what this is really for. If it's purely to make Ladybird work,

Yeah, it’s just that, I believe.

then dropping deprecated stuff makes sense. But if we want it to be a machine-readable form of the ARIA data, then keeping deprecated things in there, marked as deprecated, could be useful.

I think we’d not want it to also scope-creep into a general-purpose source of the ARIA data — especially not something that other projects might end up expecting us to maintain and update and enhance for use cases we ourselves have no need for.

As discussed a bit on Discord: It could make sense for the ARIA WG themselves to maintain some machine-readable thing upstream. And if they ever did, I’d be super happy to re-write our code to consume whatever they might end up producing.

So I’ve gone ahead and raised w3c/aria#2445 to ask the ARIA WG to consider doing that.

Copy link
Member

@AtkinsSJ AtkinsSJ left a comment

Choose a reason for hiding this comment

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

I'm happy merging this. Hopefully Aria do decide to adopt the data, but either way this is an improvement. :^)

@AtkinsSJ AtkinsSJ merged commit 4396dbb into LadybirdBrowser:master Feb 21, 2025
7 checks passed
@sideshowbarker sideshowbarker deleted the aria-roles-data-audit branch February 21, 2025 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants