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

a[href][disabled] is still a focusable element #3315

Closed
WilcoFiers opened this issue Nov 29, 2021 · 4 comments
Closed

a[href][disabled] is still a focusable element #3315

WilcoFiers opened this issue Nov 29, 2021 · 4 comments
Labels
commons Issues in the common code (lib/commons) fix Bug fixes good first issue For first-time contributors pr A pr has been created for the issue

Comments

@WilcoFiers
Copy link
Contributor

Noticed this in reviewing #3165. In determining if an element is focused, axe-core looks at the disabled attribute. However it doesn't check if that attribute is allowed for the node. So the following link is incorrectly determined as not focusable:

<a href="foo.html" disabled>Hello</a>

This bug exists in lib/commons/dom/focus-disabled.js. The fix is to check vNode.props.nodeName before looking for the disabled attribute.

@WilcoFiers WilcoFiers added fix Bug fixes commons Issues in the common code (lib/commons) good first issue For first-time contributors labels Nov 29, 2021
@dan-tripp
Copy link
Contributor

I would like to fix this one. But it might take me a few weeks to get around to it.

@WilcoFiers
Copy link
Contributor Author

@dan-tripp You are welcome to it. Much appreciated!

dan-tripp added a commit to dan-tripp/axe-core that referenced this issue Feb 12, 2022
Now checks "disabled" attribute

Closes issue dequelabs#3315
dan-tripp added a commit to dan-tripp/axe-core that referenced this issue Feb 21, 2022
Now checks "disabled" attribute

Closes issue dequelabs#3315
straker added a commit that referenced this issue Feb 22, 2022
…s as disabled (#3393)

* refactor(checks/navigation): improve `internal-link-present-evaluate`

Make `internal-link-present-evaluate` work with virtualNode rather than actualNode.

Closes issue #2466

* test commit 1

* test commit 2

* test commit 3

* Revert "Merge branch 'dan-test-branch-1' into develop"

This reverts commit 428e015, reversing
changes made to 9f996bc.

* Revert "test commit 1"

This reverts commit 9f996bc.

* fix(rule): allow "tabindex=-1" for rules "aria-text" and "nested-interactive"

Closes issue #2934

* work in progress

* work in progress

* test commit 1

* Revert "test commit 1"

This reverts commit 9f996bc.

* fix(rule): allow "tabindex=-1" for rules "aria-text" and "nested-interactive"

Closes issue #2934

* work in progress

* work in progress

* fix whitespace

* add new case to test test/checks/keyboard/no-focusable-content.js

* change "disabled" test case in test/checks/keyboard/no-focusable-content.js

* fix merge problem

* fix(commons/dom): focusDisabled() behavior

Now checks "disabled" attribute

Closes issue #3315

* Update lib/commons/dom/focus-disabled.js

Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>

* fix typo

Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>
@straker straker added the pr A pr has been created for the issue label Feb 22, 2022
@OmiCoding
Copy link

I was looking for issues to fix and I noticed this issue was already fixed in bb8b5ca, could a maintainer mark this as closed?

@padmavemulapati
Copy link

Verified with the latest dev branch code , role=text is allowing to have "tabindex=-1" as a descendants

and with the nested-interactive/aria-text allowing "tabindex=-1" on elements when role is not there.
and role conflict is appearing, when tabindex=-1 and role=presentation

Image

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commons Issues in the common code (lib/commons) fix Bug fixes good first issue For first-time contributors pr A pr has been created for the issue
Projects
None yet
Development

No branches or pull requests

5 participants