bbc / simorgh

The BBC's Open Source Web Application. Contributions welcome! Used on some of our biggest websites, e.g.
https://www.bbc.com/pidgin
Other
1.41k stars 225 forks source link

Investigation: Why does NVDA read out 'clickable' when reading images #9746

Open greenc05 opened 2 years ago

greenc05 commented 2 years ago

Describe the bug When using NVDA with Firefox ESR on Windows 10 it reads out 'clickable' when I read images. Why is this?

To Reproduce Given I am using NVDA on Windows 10 with Firefox ERS When I press the down arrow key to read an image Then 'clickable' is announced along with the alt text and image semantics

Expected behaviour Clickable should not be announced when I read an image.

DarioR01 commented 2 years ago

Investigation report When we started investigating this bug we immediately noticed that if we open the accessibility tree in Firefox we can notice the keyboard's warnings attached to all graphic elements that are announced as clickable. We also discovered that every image that was being announced as clickable had a Click action visible from the properties dropdown:

Screenshot 2022-01-20 at 16 03 45

From this StackOverflow's article we understood that this event was being passed down from the parent elements. But this did not explain why only graphics were announcing clickable.

We decided to go back to past PR's and understand when we introduced the bug. We noticed how the Glass Promo component was the only component with a graphic element that was not announcing "clickable graphic" in past PR's. This motivated us for searching the PR that introduced the bug in the FrostedGlassPromo component. We identified the following PR being the one that introduced the bug. After further investigation we discovered that if we replace this line with const Wrapper = styled.a` ,and this line with <Wrapper href="url" data-testid={ `frosted-promo-${index}`> NVDA will make it stop to announce "clickable graphic" . My assumption is that the click event gets overwritten by the basic click event of the anchor tag. That would also explain why this fix only works when we have an href parameter passed to it. However, this has no use for us, because it makes the clickable image a link, which is not the behaviour we want for all images.

Finally, we discovered the following issue reported in 2021 to facebook React. After some testing I managed to reproduce the described bug in Simorgh by modifying the clint.js and components.js as shown from the following images:

client.js: image (1)

components.js image (2)

I have essentially added 2 tests image, one outside the "root" division and one just inside the root div but before any other element. What we observed is that the one outside presented no bug, but the one inside was still announcing "clickable graphics":

image (1)

And we also observed that we have no keyboard warning appearing in the graphic element outside the root div:

image

We also made a minor discovery where the keyboard warning attached to the graphic element seems to be affected by the LTR and RTL logic 😂 perhaps in Arabic service, we would observe:

Screenshot 2022-01-19 at 11 57 45

Conclusion

React 17 had changes to event delegation https://reactjs.org/blog/2020/08/10/react-v17-rc.html#changes-to-event-delegation In React 16, the events were attached to the document instead of the root v16 - document.addEventListener('click', () => { console.log('click') }) v17 - document.querySelector('#root').addEventListener('click', () => { console.log('click') })

Take this simple HTML as an example:

<!DOCTYPE html>
<html lang='en'>
    <head>
        <title>Basic HTML5 document</title>
        <meta charset='utf-8'>
    </head>
    <body>
        <div id="root">
            <img src="https://picsum.photos/200/300" alt="test text"/>
        </div>

        <script>
            // event delegation
            document.querySelector('#root').addEventListener('click', () => { console.log('click') })
        </script>
    </body>
</html>

If we run the above and inspect with NVDA we can observe the bug. But if we attach the listener to the document instead of the root, the bug goes away.

We reported this behaviour with Facebook React, NVDA, and W3C/aria.

DarioR01 commented 2 years ago

Updates:

<div role="presentation">
        <img src="https://picsum.photos/200/300" alt="div with presentation" />
</div>

<figure role="presentation">
        <img
          src="https://picsum.photos/200/300"
          alt="figure with presentation"
        />
</figure>

<img
role="presentation"
src="https://picsum.photos/200/300"
alt="No wrap and presentation"
/>

We discovered that all of the above fail to fix our issue for various reasons.

However, putting role="presentation" directly in the root div seems to fix the graphic clickable bug. We will take this into consideration as a last resource.

DarioR01 commented 2 years ago

React are going to recommend adding role="presentation" natively in the root div https://github.com/facebook/react/issues/20895#issuecomment-1021175889

DarioR01 commented 2 years ago

We can make the changes ourselves by https://github.com/bbc/simorgh/pull/9828