Only add aria-hidden for scale mode

[krassowski]

Fixes #499. I would appreciate a review from someone working on accessibility.

[fcollonval]

Gentle ping to @gabalafou could you please review this one?

[gabalafou]

Hi @krassowski, thanks for working on this!

I'm not sure I entirely agree with MDN that aria-hidden should not be added when an element has the hidden attribute, or display: none, or visibility: hidden. And the reason is precisely because of content-visibility. As far as I understand, if you style some offscreen node with content-visibility: auto, but inside that node you have a node that is hidden (via display: none or visibility: hidden), then the hidden node will still appear in the accessibility tree, which is most likely undesired. For example:

<style>
.offscreen {
  content-visibility: auto;
}
.hidden {
  display: none;
}
</style>
<div>Above the fold</div>
<div class="offscreen">
  Below the fold.
  <div class="hidden">Hide this content.</div>
</div>

When the above page first loads and the user hasn't scrolled down, then "Hide this content" will appear in the accessibility tree, correct?

Why does the browser have to do a style recalculation when changing the aria-hidden value? Is it because the aria-hidden value can be targeted by CSS via the [aria-hidden] attribute selector?

My other question is, since we're already changing properties on the widget's node when we call show/hide, why does changing aria-hidden at the same time cause additional work? Is it because the browser has to do the style recalculation at the exact moment that we call set/removeAttribute? Is there any way to batch together the node property changes?

One last question, how could I test this across browsers? Just so I understand what the desired behavior is, whenever we have a widget, no matter what HiddenMode it uses, when we call hide on that widget, its node should be removed from the accessibility tree, and when we call show, it should be added to the accessibility tree? Is that right?

Sorry if some of the above doesn't make sense. I had to type it up hastily. Again, thanks for working on this and please let me know how I can help. Happy to write tests or check this functionality on other browsers. Just let me know, thanks! 😄

[krassowski]

We are not using content-visibility: auto though, only content-visibility: hidden.

Why does the browser have to do a style recalculation when changing the aria-hidden value? Is it because the aria-hidden value can be targeted by CSS via the [aria-hidden] attribute selector?

Yes. Some browsers (see Firefox) have very good heuristics for avoiding doing so when not needed, others don't.

My other question is, since we're already changing properties on the widget's node when we call show/hide, why does changing aria-hidden at the same time cause additional work?

Because browsers have fast paths (optimized code) to handle changes of some attributes or scenarios but not for others. We want to make as little DOM changes which invalidate the style cache as possible to trigger as many fast paths as possible.

Is it because the browser has to do the style recalculation at the exact moment that we call set/removeAttribute? Is there any way to batch together the node property changes?

No. Yes, browsers do batch the style recalculation and layout operations together automatically.

In general if you want to learn more see this course.

One last question, how could I test this across browsers? Just so I understand what the desired behavior is, whenever we have a widget, no matter what HiddenMode it uses, when we call hide on that widget, its node should be removed from the accessibility tree, and when we call show, it should be added to the accessibility tree? Is that right?

Yes, this is right. You would need to modify the DOM manually to test it at the moment (until we release).

[fcollonval]

Thanks @krassowski

I let you merge if you think it is ok.