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

Create wai-aria/hidden/aria-hidden-tested-via-label.html #45694

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

rahimabdi
Copy link
Contributor

@rahimabdi
Copy link
Contributor Author

rahimabdi commented Apr 13, 2024

@cookiecrook @scottaohara A couple outstanding questions for this test:

  • I see that ignoring aria-hidden on focusable elements is still pending (aria #1255) however, WebKit/Chromium do currently ignore it from my quick testing of a couple elements such as <button aria-hidden="true">. Should we include tests for this although it's not part of ARIA spec yet?
  • I included a test for checking that aria-hidden="false" is ignored on the root HTML element however this is currently failing across engines. Any idea why browsers don't treat document as the implicit ARIA role for <html> (see "html root element with aria-hidden='false' is not hidden" test in CI results)?

@scottaohara
Copy link
Contributor

scottaohara commented Apr 15, 2024

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 visibility: collapse somehow flew under my radar. For the use case of using it for tables - per the MDN demo near the end of the page - that could easily result in (and does in that demo) some broken communication of what the table visually conveys vs how it is exposed to someone using AT. E.g., cell 1.3 looks like it should be in the 3rd column of content - but it will be communicated to AT as if it was in the second column...

bleh.... this will definitely cause issues... (again, unrelated to this test)

Comment on lines 1 to 3
<!doctype html>
<html>
<head>
Copy link
Contributor

@cookiecrook cookiecrook Apr 17, 2024

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.

Copy link
Contributor Author

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.

@rahimabdi rahimabdi changed the title Create wai-aria/states-and-properties/aria-hidden.html Create wai-aria/hidden/aria-hidden-tested-via-label.html Apr 29, 2024
Copy link
Contributor

@cookiecrook cookiecrook left a 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!

Comment on lines +30 to +31
- opacity: 0
- color: transparent
Copy link
Contributor

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

Comment on lines +38 to +39
- 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:
Copy link
Contributor

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.

Comment on lines +51 to +80
<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>
Copy link
Contributor

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.

Suggested change
<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>

Comment on lines +84 to +88
<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>
Copy link
Contributor

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.

Comment on lines +90 to +94
<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>
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Comment on lines +96 to +104
<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>
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Comment on lines +106 to +113
<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>
Copy link
Contributor

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.

Copy link
Contributor

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">
Copy link
Contributor

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.

Comment on lines +145 to +166
<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>
Copy link
Contributor

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.

Suggested change
<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>

Comment on lines +174 to +178
<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>
Copy link
Contributor

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.

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.

Tests for removal of aria-hidden=false
4 participants