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

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

Open
greenc05 opened this issue Dec 15, 2021 · 4 comments
Open
Assignees
Labels
a11y Accessibility-related task bug Something isn't working
Projects

Comments

@greenc05
Copy link

greenc05 commented Dec 15, 2021

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.

@greenc05 greenc05 added bug Something isn't working a11y Accessibility-related task labels Dec 15, 2021
@greenc05 greenc05 added this to Coming Soon in Simorgh via automation Dec 15, 2021
@greenc05 greenc05 changed the title Investigation: Why does NVDA read out 'clickable' when reading some images Investigation: Why does NVDA read out 'clickable' when reading images Dec 15, 2021
@HarveyPeachey HarveyPeachey moved this from Coming Soon to Backlog (Ready for dev) in Simorgh Dec 16, 2021
@DarioR01 DarioR01 self-assigned this Jan 10, 2022
@DarioR01 DarioR01 moved this from Backlog (Ready for dev) to Issue in Progress [WIP 12] in Simorgh Jan 10, 2022
@DarioR01
Copy link
Contributor

DarioR01 commented Jan 20, 2022

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

DarioR01 commented Jan 25, 2022

Updates:

  • Facebook/React closed out the ticket as it was a duplicate.
  • We closed the W3C/aria ticket because we discovered new information about clickable elements we believe the bug should be fixed by NVDA or Firefox.
  • NVDA suggested a solution where role="presentation" solves the bug we are experiencing. After some tests:
<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
Copy link
Contributor

DarioR01 commented Jan 28, 2022

React are going to recommend adding role="presentation" natively in the root div facebook/react#20895 (comment)

@DarioR01 DarioR01 moved this from Issue in Progress [WIP 12] to Blocked in Simorgh Jan 31, 2022
@DarioR01
Copy link
Contributor

We can make the changes ourselves by #9828

@bbc bbc deleted a comment from karinathomasbbc Jun 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Accessibility-related task bug Something isn't working
Projects
No open projects
Simorgh
Blocked
Development

Successfully merging a pull request may close this issue.

3 participants