-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Create wai-aria/hidden/aria-hidden-tested-via-label.html #45694
base: master
Are you sure you want to change the base?
Conversation
@cookiecrook @scottaohara A couple outstanding questions for this test:
|
oh that's interesting. per your first point, chroimum used to re-expose the focused button/its text. but yah, testing today that doesn't seem to be happening anymore. with jaws/nvda either the last focused item is re-announced, or 'blank' is said instead. i'm not sure we should include that into the tests yet - since this behavior is opposite what i thought was going to get done/standardized. i can't answer your second bullet. so i'll leave that to james. if i recall, what is the 'document' vs not in an html file is maybe not as clear cut as the spec text states. Unrelated: but the bleh.... this will definitely cause issues... (again, unrelated to this test) |
Co-authored-by: Scott O'Hara <[email protected]>
<!doctype html> | ||
<html> | ||
<head> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding test methodology, your current approach of using computedrole
may not get you what you need. In theory, any of these implementations may have the element marked as "hidden" but have the backing object retain the role it would have if it were not hidden.... So these "expected role" tests as written could have both false positives and false negatives.
Another approach that may yield more consistent results between engines is to check the computedlabel
of a rendered container element, that includes its label from contents. That container could have a mix of rendered or unrendered elements with mixed states of aria-hidden
true and false.
As an example that may cause a failure in recent Safari builds at the moment (recently resolved in current WebKit source but not yet shipped, IIRC), since it implemented the original definition of aria-hidden=false
, not the newest draft PR that the spec aligned on.
<h1 data-expectedlabel="one"><!-- not "one three" -->
one <!-- part of the label in all implementations -->
<span aria-hidden="true">
two <!-- should not be part of the label in any implementation -->
<span aria-hidden="false">
three <!-- was part of the label in at least one older implementation before recent spec change proposal -->
</span>
</span>
</h1>
Likewise, aria-hidden=false
should not be able to "undo" or "invert" the hidden-ness of a rendering style like display: none;
or visibility: hidden;
.
<h1 data-expectedlabel="one"><!-- not "one three" -->
one <!-- part of the label in all implementations -->
<span style="display: none;">
two <!-- unrendered contents should not be part of the label in any implementation -->
<span aria-hidden="false">
three <!-- aria-hidden=false should no longer be able to "unhide" an unrendered, hidden element, unless perhaps referenced in accname directly via aria-labelledby -->
</span>
</span>
</h1>
Moving from test methodology to file naming, I'm not sure there is a good reason to have an overall states-and-properties
dir, especially since some of the other features we're testing elsewhere (aria-label
, aria-labelledby
, etc.) are themselves properties.
I do agree that there could be a hidden
or ignored
directory with various files included... I expect there will be a lot more "hidden" or "ignored" tests once more test accessors are available.
Also, this file does not cover everything we'll want to determine about aria-hidden
, so aria-hidden.html
is too general a name, IMO. This might be renamed aria-hidden-tested-via-label.html
or something more specific. Perhaps this file has a further logical split into two or more files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @cookiecrook! Ready for another review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there. Thanks for working on it!
- opacity: 0 | ||
- color: transparent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are both rendered fwiw
- For future general aria-hidden testing that can't currently be tested with WebDriver computedlabel method, such as | ||
WebDriver extension for querying accessibility tree outside of role/name aom #203 https://github.com/WICG/aom/issues/203: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this note suggesting these new to-be-created tests be add to this file? If so, why? If not, I would remove this portion of the comment.
<h2><code>aria-hidden="false"</code> only tested via label</h2> | ||
|
||
<!-- native platform elements with default display properties --> | ||
<button aria-hidden="false" data-testname="button with aria-hidden='false' is not hidden" data-expectedlabel="x" class="ex-label">x</button> | ||
<input type="checkbox" aria-label="x" aria-hidden="false" data-testname="input[checkbox] with aria-hidden='false' labeled via aria-hidden is not hidden" data-expectedlabel="x" class="ex-label"> | ||
<h3 aria-hidden="false" data-testname="h3 with aria-hidden='false' is not hidden" data-expectedlabel="x" class="ex-label">x</h3> | ||
<nav aria-hidden="false" aria-label="x" data-testname="nav with aria-hidden='false' labeled via aria-label is not hidden" data-expectedlabel="x" class="ex-label">x</nav> | ||
|
||
<!-- native platform elements with non-default rendering --> | ||
<button aria-hidden="false" style="display: flex;" data-testname="button with display: contents and aria-hidden='false' is not hidden" data-expectedlabel="x" class="ex-label">x</button> | ||
<input aria-hidden="false" style="display: flex;" aria-label="x" type="checkbox" data-testname="input[checkbox] with display: contents and aria-hidden='false' labeled via aria-hidden is not hidden" data-expectedlabel="x" class="ex-label"> | ||
<h3 aria-hidden="false" style="display: contents;" data-testname="h3 with display: contents, aria-hidden='false' is not hidden" data-expectedlabel="x" class="ex-label">x</h3> | ||
<nav aria-hidden="false" style="display: contents;" aria-label="x" data-testname="nav with display: contents nav and aria-hidden='false' labeled via aria-hidden is not hidden" data-expectedlabel="x" class="ex-label">x</nav> | ||
<button aria-hidden="false" style="transform: scale(0);" data-testname="button with transform: scale(0) and aria-hidden='false' is not hidden" data-expectedlabel="x" class="ex-label">x</button> | ||
|
||
<!-- explicit ARIA roles --> | ||
<div aria-hidden="false" role="button" aria-label="x" tabindex="0" data-testname="div with aria-hidden='false' labeled via aria-label, button role and tabindex='0' is not hidden" data-expectedlabel="x" class="ex-label">x</div> | ||
<div aria-hidden="false" role="heading" aria-level="3" data-testname="div with aria-hidden='false', heading role and aria-level='3' is not hidden" data-expectedlabel="x" class="ex-label">x</div> | ||
<div aria-hidden="false" role="navigation" aria-label="x" data-testname="div with aria-hidden='false' labeled via aria-label, navigation role and aria-label is not hidden" data-expectedlabel="x" class="ex-label">x</div> | ||
<span aria-hidden="false" role="link" tabindex="0" data-testname="span with aria-hidden='false', link role and tabindex='0' is not hidden" data-expectedlabel="x" class="ex-label">x</span> | ||
<span aria-hidden="false" role="region" aria-label="x" data-testname="span with aria-hidden='false', region role and aria-label is not hidden" data-expectedlabel="x" class="ex-label">x</span> | ||
<span aria-hidden="false" role="checkbox" aria-checked="true" aria-label="x" data-testname="span with aria-hidden='false' labeled via aria-label and checkbox role is not hidden" data-expectedlabel="x" class="ex-label">x</span> | ||
<div> | ||
<div role="tablist"> | ||
<div role="tab">x</div> | ||
<div aria-hidden="false" role="tab" aria-selected="true" data-testname="div with aria-hidden='false' and tab role is not hidden" data-expectedlabel="x" class="ex-label">x</div> | ||
</div> | ||
<div aria-hidden="false" role="tabpanel" hidden>x</div> | ||
<div aria-hidden="false" role="tabpanel">x</div> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think any of this first set can be tested reliably with label for the reasons listed in the first review. An implementation is not required to invalidate the label of a hidden element... The hidden rules can only be tested in how the element is incorporated into the label of a different element, like you've done in the next set.
<h2><code>aria-hidden="false"</code> only tested via label</h2> | |
<!-- native platform elements with default display properties --> | |
<button aria-hidden="false" data-testname="button with aria-hidden='false' is not hidden" data-expectedlabel="x" class="ex-label">x</button> | |
<input type="checkbox" aria-label="x" aria-hidden="false" data-testname="input[checkbox] with aria-hidden='false' labeled via aria-hidden is not hidden" data-expectedlabel="x" class="ex-label"> | |
<h3 aria-hidden="false" data-testname="h3 with aria-hidden='false' is not hidden" data-expectedlabel="x" class="ex-label">x</h3> | |
<nav aria-hidden="false" aria-label="x" data-testname="nav with aria-hidden='false' labeled via aria-label is not hidden" data-expectedlabel="x" class="ex-label">x</nav> | |
<!-- native platform elements with non-default rendering --> | |
<button aria-hidden="false" style="display: flex;" data-testname="button with display: contents and aria-hidden='false' is not hidden" data-expectedlabel="x" class="ex-label">x</button> | |
<input aria-hidden="false" style="display: flex;" aria-label="x" type="checkbox" data-testname="input[checkbox] with display: contents and aria-hidden='false' labeled via aria-hidden is not hidden" data-expectedlabel="x" class="ex-label"> | |
<h3 aria-hidden="false" style="display: contents;" data-testname="h3 with display: contents, aria-hidden='false' is not hidden" data-expectedlabel="x" class="ex-label">x</h3> | |
<nav aria-hidden="false" style="display: contents;" aria-label="x" data-testname="nav with display: contents nav and aria-hidden='false' labeled via aria-hidden is not hidden" data-expectedlabel="x" class="ex-label">x</nav> | |
<button aria-hidden="false" style="transform: scale(0);" data-testname="button with transform: scale(0) and aria-hidden='false' is not hidden" data-expectedlabel="x" class="ex-label">x</button> | |
<!-- explicit ARIA roles --> | |
<div aria-hidden="false" role="button" aria-label="x" tabindex="0" data-testname="div with aria-hidden='false' labeled via aria-label, button role and tabindex='0' is not hidden" data-expectedlabel="x" class="ex-label">x</div> | |
<div aria-hidden="false" role="heading" aria-level="3" data-testname="div with aria-hidden='false', heading role and aria-level='3' is not hidden" data-expectedlabel="x" class="ex-label">x</div> | |
<div aria-hidden="false" role="navigation" aria-label="x" data-testname="div with aria-hidden='false' labeled via aria-label, navigation role and aria-label is not hidden" data-expectedlabel="x" class="ex-label">x</div> | |
<span aria-hidden="false" role="link" tabindex="0" data-testname="span with aria-hidden='false', link role and tabindex='0' is not hidden" data-expectedlabel="x" class="ex-label">x</span> | |
<span aria-hidden="false" role="region" aria-label="x" data-testname="span with aria-hidden='false', region role and aria-label is not hidden" data-expectedlabel="x" class="ex-label">x</span> | |
<span aria-hidden="false" role="checkbox" aria-checked="true" aria-label="x" data-testname="span with aria-hidden='false' labeled via aria-label and checkbox role is not hidden" data-expectedlabel="x" class="ex-label">x</span> | |
<div> | |
<div role="tablist"> | |
<div role="tab">x</div> | |
<div aria-hidden="false" role="tab" aria-selected="true" data-testname="div with aria-hidden='false' and tab role is not hidden" data-expectedlabel="x" class="ex-label">x</div> | |
</div> | |
<div aria-hidden="false" role="tabpanel" hidden>x</div> | |
<div aria-hidden="false" role="tabpanel">x</div> | |
</div> |
<h3 data-testname="empty h3 > span with aria-hidden='true' has no label" data-expectedlabel="" class="ex-label"> | ||
<span aria-hidden="true"> | ||
one | ||
</span> | ||
</h3> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Test like this, however I worry about a test requiring an explicitly empty label... It forces implementations to not be permissive in their allowance of "what the author probably intended."
Maybe we should remove this one, too. Or add in another part of the label outside the hidden span, or even a title attribute on the tested element.
<h3 data-testname="empty h3 > span with aria-hidden='false' is labelled" data-expectedlabel="one" class="ex-label"> | ||
<span aria-hidden="false"> | ||
one | ||
</span> | ||
</h3> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
<h3 data-testname="h3 with name from contents > span with aria-hidden='true' > span with aria-hidden='false' is labelled" data-expectedlabel="label" class="ex-label"> | ||
label | ||
<span aria-hidden="true"> | ||
one | ||
<span aria-hidden="false"> | ||
two | ||
</span> | ||
</span> | ||
</h3> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
<h3 data-testname="Empty h3 > span with aria-hidden='true' > span with aria-hidden='false' is labelled" data-expectedlabel="" class="ex-label"> | ||
<span aria-hidden="true"> | ||
one | ||
<span aria-hidden="false"> | ||
two | ||
</span> | ||
</span> | ||
</h3> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest removing or modifying this explicitly empty label test for the same reason listed above. Implementations should be allowed to work around author errors, when it results in an explicitly empty label.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you see something in the AccName spec that disallows this, I'll file an issue, since:
- Users > Authors > Implementors > Spec Editors > Theoretical Purity.
</h3> | ||
|
||
<h3 data-testname="h3 > img with alt, [span with aria-hidden='true' > span with aria-hidden='false'] is labelled" data-expectedlabel="label" class="ex-label"> | ||
<img src="data:," alt="label"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the real single pixel data URI image from the other accessibility tests. Gecko developers raised a bug with the other tests since their implementation is different if an image doesn't load.
<a href="#" data-testname="link with name from contents > img with alt, span with aria-hidden='false' is labelled" data-expectedlabel="link image one" class="ex-label"> | ||
link | ||
<img src="data:," alt="image"> | ||
<span aria-hidden="false">one</span> | ||
</a> | ||
|
||
<a href="#" data-testname="link with name from contents > img with alt, [span with aria-hidden='true' > span with aria-hidden='false'] is labelled" data-expectedlabel="link image" class="ex-label"> | ||
link | ||
<img src="data:," alt="image"> | ||
<span aria-hidden="true"> | ||
one | ||
<span aria-hidden="false"> | ||
two | ||
</span> | ||
</span> | ||
</a> | ||
|
||
<a href="#" data-testname="link with name from contents > img with alt, span with aria-hidden='true' is labelled" data-expectedlabel="link image" class="ex-label"> | ||
link | ||
<img src="data:," alt="image"> | ||
<span aria-hidden="true">one</span> | ||
</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see these as being substantially different from the previous set. I would caution against exhaustive tests whose differences aren't really related to the core matter being tested. Doing so just makes the test data more difficult to maintain.
<a href="#" data-testname="link with name from contents > img with alt, span with aria-hidden='false' is labelled" data-expectedlabel="link image one" class="ex-label"> | |
link | |
<img src="data:," alt="image"> | |
<span aria-hidden="false">one</span> | |
</a> | |
<a href="#" data-testname="link with name from contents > img with alt, [span with aria-hidden='true' > span with aria-hidden='false'] is labelled" data-expectedlabel="link image" class="ex-label"> | |
link | |
<img src="data:," alt="image"> | |
<span aria-hidden="true"> | |
one | |
<span aria-hidden="false"> | |
two | |
</span> | |
</span> | |
</a> | |
<a href="#" data-testname="link with name from contents > img with alt, span with aria-hidden='true' is labelled" data-expectedlabel="link image" class="ex-label"> | |
link | |
<img src="data:," alt="image"> | |
<span aria-hidden="true">one</span> | |
</a> |
<button data-testname="empty button > span with aria-hidden='true' has no label" data-expectedlabel="" class="ex-label"> | ||
<span aria-hidden="true"> | ||
one | ||
</span> | ||
</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto re: empty label above. Suggest modify or remove.
Closes: web-platform-tests/interop-accessibility#88