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

Need a bi-directional API for utilizing both node & virtualNode #726

Closed
marcysutton opened this issue Feb 9, 2018 · 2 comments · Fixed by #1503
Closed

Need a bi-directional API for utilizing both node & virtualNode #726

marcysutton opened this issue Feb 9, 2018 · 2 comments · Fixed by #1503
Assignees
Labels
core Issues in the core code (lib/core)

Comments

@marcysutton
Copy link
Contributor

marcysutton commented Feb 9, 2018

In doing the work for #690, I found some checks still use accessibleText instead of accessibleTextVirtual, probably because they're using DOM APIs and filtering on DOM nodes. This makes it difficult to call accessibleTextVirtual without having to redo the work in accessibleText (and sort-of negates the point of doing it).

We talked about creating a bi-directional API so you can move more easily between node and virtualNode–that's what this issue is about. Can we add a virtualNode property to DOM nodes, or are those readonly?

Here's an example from lib/checks/label/explicit.js:

if (node.getAttribute('id')) {
	const root = axe.commons.dom.getRootNode(node);
	const id = axe.commons.utils.escapeSelector(node.getAttribute('id'));
	const label = root.querySelector(`label[for="${id}"]`);

	if (label) {
		return !!axe.commons.text.accessibleText(label);
	}
}
return false;

Because the label element is found by querying the DOM from an explicit ID on the node under test, you can't use accessibleTextVirtual here without getNodeFromTree. Other examples of checks that have this problem include:

accessibleText

  • lib/checks/forms/fieldset.js
  • lib/checks/forms/labelledby.js
  • lib/checks/label/help-same-as-label.js
  • lib/checks/shared/aria-labelledby.js
  • lib/checks/tables/headers-visible-text.js
  • lib/checks/tables/same-caption-summary.js

hasContent

  • lib/checks/navigation/region.js
  • lib/checks/tables/td-has-header.js
  • lib/checks/tables/th-has-data-cells.js
@marcysutton marcysutton changed the title Need a bi-directional api for switching between node and virtualNode Need a bi-directional API for utilizing both node & virtualNode Feb 10, 2018
@dequelabs dequelabs deleted a comment Feb 13, 2018
@dylanb
Copy link
Contributor

dylanb commented Feb 13, 2018

@marcysutton we do have a bi-directional API - we have getNodeFromTree and actualNode

The issue here is more complicated. IDREFS are only valid within the scope of a single root. The code example you list is taking advantage of this by using Element.querySelector instead of axe.utils,querySelectorAll.

We could "fix" the code by using axe.utils.querySelectorAll instead but would have to ensure we have tests to ensure that selectors with [for=X] stay within the scope of the current root.

@WilcoFiers
Copy link
Contributor

I'm taking off the tag. We're not going to get this done before 3.0.

@WilcoFiers WilcoFiers added the core Issues in the core code (lib/core) label Apr 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Issues in the core code (lib/core)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants