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

Remove aria-relevant tests #43866

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove aria-relevant tests #43866

wants to merge 1 commit into from

Conversation

domenic
Copy link
Member

@domenic domenic commented Jan 5, 2024

This attribute was removed from the ARIA spec in 2020: see w3c/aria#1261 and w3c/aria#1267.

All browsers will fail the new -historical.html tests, so it's not clear whether we should merge this or fix the spec...

This attribute was removed from the ARIA spec in 2020: see w3c/aria#1261 and w3c/aria#1267.
@annevk
Copy link
Member

annevk commented Jan 8, 2024

@cookiecrook @alice thoughts?

@alice
Copy link
Contributor

alice commented Jan 9, 2024

Hm, looks like committing w3c/aria#1261 with ariaRelevant commented out was an accident @cookiecrook? Based on w3c/aria#1261 (comment) it seems like it was waiting on a resolution for the missing value default question in w3c/aria#1265, which has since been resolved.

@cookiecrook
Copy link
Contributor

cookiecrook commented Jan 11, 2024

from the diff in w3c/aria#1261

15-May-2020: Remove nullable from IDL DOMStrings, add enumerated attributes prose and examples, and remove ariaRelevant IDL until Issue w3c/aria#1267 can be resolved.

So not an accident. 1267 still unresolved. I think you were looking at 1265 not 1267?

@@ -58,7 +58,6 @@
"ariaPosInSet",
"ariaPressed",
"ariaReadOnly",
"ariaRelevant",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to comment this one with a pointer to the issue: https://github.com/w3c/aria/issue/1267

Copy link
Contributor

Choose a reason for hiding this comment

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

-  "ariaRelevant",
+  // "ariaRelevant", // blocked by https://github.com/w3c/aria/issue/1267

@@ -47,7 +47,6 @@
testReflectAttribute('ariaPosInSet', 'aria-posinset', 'foo', 'bar', 'ariaPosInSet on Element');
testReflectAttribute('ariaPressed', 'aria-pressed', 'foo', 'bar', 'ariaPressed on Element');
testReflectAttribute('ariaReadOnly', 'aria-readonly', 'foo', 'bar', 'ariaReadOnly on Element');
testReflectAttribute('ariaRelevant', 'aria-relevant', 'foo', 'bar', 'ariaRelevant on Element');
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto here. These are alphabetical lists, so they should include all the known properties, even if some aren't testable yet.

We used a similar pattern in html-aam/roles.html for completeness and pointers to avoid duplication of tests or issues.

Copy link
Contributor

Choose a reason for hiding this comment

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

- testReflectAttribute('ariaRelevant', 'aria-relevant', 'foo', 'bar', 'ariaRelevant on Element');
+ // ariaRelevant blocked by https://github.com/w3c/aria/issue/1267

Copy link
Member Author

Choose a reason for hiding this comment

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

"they should include all the known properties"

How do you define "known properties"? Looking at published specs, I can find nothing that would give me knowledge of ariaRelevant.

Did you mean instead "specified and proposed-for-the-future properties"?

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

Successfully merging this pull request may close these issues.

6 participants