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

Color contrast checks result in an error when axe-core is run from an extension #3837

Closed
1 task done
thuey opened this issue Dec 22, 2022 · 4 comments · Fixed by #3838 or #3858
Closed
1 task done

Color contrast checks result in an error when axe-core is run from an extension #3837

thuey opened this issue Dec 22, 2022 · 4 comments · Fixed by #3838 or #3858
Labels
pr A pr has been created for the issue tech debt Technical debt related tasks tests Issues that deal with the unit or integration tests
Milestone

Comments

@thuey
Copy link
Contributor

thuey commented Dec 22, 2022

Product

axe-core

Product Version

= 4.6.0

Latest Version

  • I have tested the issue with the latest version of the product

Issue Description

Expectation

I expect it to correctly identify color contrast issues

Actual

It does not identify color contrast issues. Instead, an error message is logged to the results:

TypeError: setting getter-only property "right"

How to Reproduce

Run axe-core >= 4.6.0 in an extension. (Feel free to reach out, and I can help you get set up with an extension with axe-core >= 4.6.0)

Additional context

I have traced down the issue and am submitting a PR to fix it. The issue is axe-core is trying to set DOMRect.right here:

nodeRect.right = nodeRect.left + nodeRect.width;
. However, DOMRect.right (and DOMRect.bottom) is a read-only property.

Unfortunately, browsers seem to enforce this inconsistently, and it is only actually enforced from the isolated world that an extension content script runs in.

@thuey thuey added the ungroomed Ticket needs a maintainer to prioritize and label label Dec 22, 2022
@WilcoFiers
Copy link
Contributor

An issue like this could easily pop up again if we don't add tests. The way I see it, we either add an eslint rule that prevents ever assigning to a .right and .bottom property, or we override the DOMRect object in our test env to have it throw when those props are assigned.

@WilcoFiers WilcoFiers reopened this Dec 23, 2022
@straker straker added tech debt Technical debt related tasks tests Issues that deal with the unit or integration tests and removed ungroomed Ticket needs a maintainer to prioritize and label labels Jan 9, 2023
@straker straker added this to the Axe-core 4.7 milestone Jan 9, 2023
@straker straker added the pr A pr has been created for the issue label Jan 16, 2023
@padmavemulapati
Copy link

@straker , QA notes please.

@straker
Copy link
Contributor

straker commented Jan 25, 2023

@padmavemulapati For this one it will be latest axe version runs in the extension without issues.

@padmavemulapati
Copy link

per steve, everything it need to work from the extension end , so validated and working as expected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr A pr has been created for the issue tech debt Technical debt related tasks tests Issues that deal with the unit or integration tests
Projects
None yet
4 participants