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

False positive: "aria-required-children" & "aria-required-parent" fail for nested buttons with roles "menu" and "menuitem" #932

Closed
iamrafan opened this issue May 30, 2018 · 10 comments
Labels

Comments

@iamrafan
Copy link
Contributor

Example:

    <button type="button" role="menu" title="name-1">
        <div role="none">
            <button role="menuitem">
                <i class="fa fa-external-link help-dropdown-image"></i>
                <span>text</span>
            </button>
        </div>
    </button>

axe-core version: 3.0.0-beta.3
axe-Coconut version: 3.3.0-beta.0

@WilcoFiers WilcoFiers added the fix Bug fixes label Jun 9, 2018
@marcysutton
Copy link
Contributor

marcysutton commented Jun 19, 2018

I'm not sure that axe-core should pass this–can you add more details about the scripting and styles involved?

In Safari and Voiceover, the focusable outer button is announced as "name-1, group". In Chrome and Voiceover, it is announced as "menu name-1, zero items". The focusable menuitem/button inside is announced as "text menuitem" in both, but there's no menuitem count like with DIV elements. I'd definitely want to see more information before changing axe-core across the board.

In JAWS 2018 and IE11, the menu is announced as "menu. to navigate, press up or down arrow". The menu item is announced as "text" and that's it–hence wanting to see more about the scripting involved. NVDA and Firefox works well; the buttons are announced as "name-1 menu" and "text, 1 of 1". But support is sorta all over the place.

@dylanb
Copy link
Contributor

dylanb commented Jun 19, 2018

@marcysutton change role="none" to role="presentation" and see what happens. I think the support problem might be with the none role and not with the rest of the markup

@marcysutton
Copy link
Contributor

marcysutton commented Jun 19, 2018

@dylanb I did that, and it didn't change anything. Here's a Codepen for testing. https://codepen.io/marcysutton/pen/QxQpbG

@dylanb
Copy link
Contributor

dylanb commented Jun 19, 2018

Ok, I see why - it is not allowed to have a button within a button, so the browser corrects the markup by hoisting the inner button element out of the outer button element and you end up with the following DOM.

image

So while the markup is correct, the resulting DOM is not.

@marcysutton
Copy link
Contributor

Ha, that makes sense–and it explains the inconsistent tab/shift-tab behavior I found. It should actually fail the rule I proposed then, for nested interactive elements: #601

@dylanb
Copy link
Contributor

dylanb commented Jun 20, 2018

@marcysutton I don't think it will fail that rule because of the browser's hoisting of the one button out of the other button, but the bottom line is that this is not a false positive

@iamrafan can you confirm this is the same behavior you are seeing so we can close this issue?

@marcysutton
Copy link
Contributor

@dylanb that might be a better rule for eslint/AST tooling since it could check pre-rendering...?

@dylanb
Copy link
Contributor

dylanb commented Jun 20, 2018

@marcysutton There are some violations of the #601 type that will not be hoisted - so it is still a valuable rule for the DOM. You are right though, it will surface more issues in a pre-rendered linter.

@iamrafan
Copy link
Contributor Author

@marcysutton @dylanb thanks for looking into this issue. I confirm that I am seeing the same behaviour

@marcysutton
Copy link
Contributor

Ok, I'm going to close this issue since it isn't a recommended technique! Thanks for posting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants