[GENERIC viewer] Try to improve a11y, for search results, in the findbar (issue 14525)

[Snuffleupagus]

Note that it seemed necessary to re-factor the findResultsCount and findMsg element grouping, in the HTML/CSS code, in order those elements to be correctly announced by a11y software in Firefox.

The following MDN articles may be helpful here:

• https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-invalid
• https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-live

[Snuffleupagus]

Using the Accessibility-tab in the DevTools to inspect the result of this patch it doesn't look completely unreasonable (with the caveat that I don't know if "status" is fully correct to use here).

@brendandahl Given that this is specific to the GENERIC viewer, and that the patch is small, can we perhaps just land this?

[scottaohara]

This change unfortunately won't work consistently as the live region role=status has been added to the span elements that toggle between display:none and block depending on whether they contain content or not. Live regions are very finicky and for the most robust behavior, they must always be part of the DOM so that when they are updated, their content can be correctly communicated by the browser's accessibility tree to assistive technologies.

I have provided a number of comments/suggested updates which would help move this PR along . Please let me know if you have any questions. Thanks

[Snuffleupagus]

@scottaohara Thank you for the help/comments here!
Can you please test the new preview, especially in Firefox since that's the primary development target for this library, and confirm that everything works correctly now?

[scottaohara]

@Snuffleupagus so this works exactly as expected with Chromium browsers, but unfortunately I'm getting some odd behavior when testing with NVDA and JAWS with Firefox. Specifically that the old results message is being repeated somehow along with the 'no results found'. This unfortunately seems like it may be a live region implementation issue with Firefox, but that can be mitigated.

Rather, what could be done here is to forego the idea of two live regions, and instead wrap both of those elements in a single <div aria-live=polite>, so:

<div aria-live=polite>
  <div class="findbarMessageContainer">
    <span id="findResultsCount" class="toolbarLabel"></span>
  </div>
  <div class="findbarMessageContainer">
    <span id="findMsg" class="toolbarLabel"></span>
  </div>
</div>

Per the attached screenshot, I added a wrapper div using Firefox's dev tools and there doesn't appear to be any layout side effects from doing this.

I've created a quick demo and tested this in Firefox, Safari and Chrome/Edge https://codepen.io/scottohara/pen/abELNOz and with this approach only the current message is announced.

Again, any questions please let me know.

[calixteman]

@Snuffleupagus so this works exactly as expected with Chromium browsers, but unfortunately I'm getting some odd behavior when testing with NVDA and JAWS with Firefox. Specifically that the old results message is being repeated somehow along with the 'no results found'. This unfortunately seems like it may be a live region implementation issue with Firefox, but that can be mitigated.

@MReschenberg, could you have a look please ?

[Snuffleupagus]

but unfortunately I'm getting some odd behavior when testing with NVDA and JAWS with Firefox.

Considering that this won't really matter for the built-in PDF Viewer in Firefox, since that one uses the regular browser findbar, this is probably less of an issue here.

Per the attached screenshot, I added a wrapper div using Firefox's dev tools and there doesn't appear to be any layout side effects from doing this.

Unfortunately there will be negative side-effects, since that'd prevent the findbar from wrapping correctly when the viewer becomes very narrow. Hence we cannot make those kind of changes here, and it also feels generally wrong to have a11y force a very particular DOM structure.

[MReschenberg]

@Snuffleupagus so this works exactly as expected with Chromium browsers, but unfortunately I'm getting some odd behavior when testing with NVDA and JAWS with Firefox. Specifically that the old results message is being repeated somehow along with the 'no results found'. This unfortunately seems like it may be a live region implementation issue with Firefox, but that can be mitigated.

@MReschenberg, could you have a look please ?

Hi! Happy to check this out -- can you give me some more information on what you mean by new vs. old results message? What's your STR?

[scottaohara]

@MReschenberg I hope the following will add some clarity.

per this build, which was based off my update comments I found that Firefox is treating the live regions differently than Chrome/Edge. For instance:

• When using the search field with NVDA, if i type in 'd' I will hear more than 1000 matches.
• if i type in 'dd' I will hear "more than 1000 matches" and then "1 of 15 matches"
• if i type in 'ddd' I will hear "phrase not found" from the aria-live=assertive element. But then "1 of 15 matches" from the no longer relevant (and emptied) aria-live=polite element.

(JAWS has slightly different behavior in the ordering of the announcements, but the 2 announcements for the 2 live regions are made)

Using NVDA and JAWS with Chrome/Edge only a single announcement is made, which is reflective of the currently visible string if I follow the same 3 steps above.

Without further testing/review of the script being used to populate the live regions, it was my assumption this is a Firefox implementation oddity, as I have worked through testing and logging of such issues in the past.

---

@Snuffleupagus re: your latest comments, I will defer to your team on what further updates make sense here. I was trying to off you the alternative to mitigate against the awkward announcements with Firefox since you mentioned that was the primary dev target. I did not notice any difference in layout for the toolbar at increased browser zoom or narrow viewports when testing the parent div wrapper. It seems these results already reflow to a new line of their own. But, I suspect per your comment there are situations I'm not aware of. Thank you anyway for considering it!

[Snuffleupagus]

It seems these results already reflow to a new line of their own. But, I suspect per your comment there are situations I'm not aware of.

As long as only one of the findResultsCount and findMsg spans are visible things will work, however when both are visible at the same time is where things break (happens when searching wraps at the top/bottom of the document).
Anyway, thanks again for helping out with testing here!

[MReschenberg]

@scottaohara thanks! interestingly I can't reproduce this using VoiceOver on macOS -- I get some other weird behaviour from a known bug, but no duplication of the aria live strings -- which makes me wonder if it's a platform-specific FF bug. Generally, as you noted, I think it's best practise to merge these elements into one live region instead of two.
I'll share this issue with the rest of my team, particularly our dev who works on windows a11y :)

[Snuffleupagus]

As mentioned before, I do find it slightly unfortunate that we have to modify the HTML code in order for this to work.[1]
However, I'd like to get this off my plate so to speak, so if the latest round of changes allow the patch to land sooner rather than later that'd obviously be nice :-)

/botio-linux preview

---

[1] Since it makes the code a bit less consistent, which could make it easier to accidentally break something with any future changes here.

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

Command cmd_preview from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/bbe3f9b3ff18b50/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.241.84.105:8877/bbe3f9b3ff18b50/output.txt

Total script time: 2.54 mins

Published

• Viewer: http://54.241.84.105:8877/bbe3f9b3ff18b50/web/viewer.html
• Viewer (legacy): http://54.241.84.105:8877/bbe3f9b3ff18b50/legacy/web/viewer.html

[scottaohara]

Very much appreciated. There are still some quirks with the announcement, but at least now the "phrase not found" is consistently being announced last with Firefox+NVDA, which will remove the ambiguity that was present in the previous build. As far as I'm concerned this meets what's necessary to close out the issue. Again, thank you.

[jcsteh]

Without further testing/review of the script being used to populate the live regions, it was my assumption this is a Firefox implementation oddity, as I have worked through testing and logging of such issues in the past.

I'm not ruling out a Firefox bug here - I'd have to dive very deep into the DOM mutations this code is making to be sure, which I can't do just now- but it's far more likely to be a difference in timing between the two browser engines. Both coalesce events based on browser ticks. However, where those ticks occur likely differs.

From what I can see, the live regions both contain an additional span with the text inside. I tried replicating that with this test case:

data:text/html,<div aria-live="assertive" id="live"></div><input type="text" onkeypress="live.innerHTML = foo;">

When I type into the input, I only hear one occurrence of "foo".

My guess is that pdf.js is updating the text multiple times in quick succession and Chromium is coalescing that while Firefox isn't due to timing differences.

[Snuffleupagus]

So, what's the status of this PR; can we land it as-is or do we need to wait for any changes/improvements on the Firefox side?

Please note that all of this mostly[1] don't apply to the Firefox PDF Viewer, since it uses the browser findbar.

---

[1] With the exception of PDF documents placed in <iframe> or <embed> elements.

[scottaohara]

FWIW, I think this PR is an improvement and effectively resolves the issue. There could be additional work/investigation into the JS or Firefox live region implementation to determine what else may need to be done for the two live region announcements. But I would submit that can be done as a follow on so as to not hold up the merging of this PR.

Thanks

[calixteman]

So, if it is an overall improvement I guess we can merge it.

[calixteman]

It's an improvement so r=me.
Thank you.