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

getElmScrollRecursive fails on SVG in some browsers #525

Closed
fofr opened this issue Sep 15, 2017 · 7 comments
Closed

getElmScrollRecursive fails on SVG in some browsers #525

fofr opened this issue Sep 15, 2017 · 7 comments
Assignees
Labels
core Issues in the core code (lib/core) fix Bug fixes
Milestone

Comments

@fofr
Copy link

fofr commented Sep 15, 2017

When recursing through the DOM to calculate the scroll position here:

function getElmScrollRecursive (root) {
return Array.from(root.children).reduce((scrolls, elm) => {
const scroll = getScroll(elm);
if (scroll) {
scrolls.push(scroll);
}
return scrolls.concat(getElmScrollRecursive(elm));
}, []);
}

If the code encounters an SVG it essentially runs, document.querySelector('svg').children. This doesn't always return an HTMLCollection object which in turn causes Array.from to error.

JS fiddle demonstrating issue:
https://jsfiddle.net/uvftjtre/2/ (version 3.0.0.alpha.4)

Running the above example in EDGE:

Array.from: argument is not an Object

Running in IE11 will return:

TypeError: Array.from requires an array-like object - not null or undefined

This error surfaced when running automated tests through Phantom JS 1.9.7

More context:
https://stackoverflow.com/questions/7935689/what-is-the-difference-between-children-and-childnodes-in-javascript#7935719

fofr added a commit to alphagov/govuk_publishing_components that referenced this issue Sep 15, 2017
See: dequelabs/axe-core#525

If the code encounters an SVG it essentially runs,
document.querySelector('svg').children. This doesn't always return an
HTMLCollection object which in turn causes Array.from to error.
@marcysutton marcysutton added the core Issues in the core code (lib/core) label Sep 28, 2017
@WilcoFiers
Copy link
Contributor

Problem turns out to be that .children is undefined. The work around is to use .childNodes instead and filter them for elements.

@WilcoFiers WilcoFiers added the fix Bug fixes label Aug 22, 2019
@WilcoFiers WilcoFiers modified the milestones: Axe-core 3.4, axe-core 3.5 Aug 22, 2019
@benbcai
Copy link
Contributor

benbcai commented Sep 16, 2019

@WilcoFiers This issue causes our accessibility tests to fail on any page that contains an SVG element. Looks like this is scheduled to be fixed in axe-core 3.4. Do you have a timeframe when 3.4 is scheduled to be released?

@emilyrohrbough
Copy link
Contributor

Do you expect the changes be to add fallback for childNodes like:

- return Array.from(root.children).reduce((scrolls, elm) => { 
+ return Array.from(root.children || root.childNodes || []).reduce((scrolls, elm) => { 

or are the scroll position of SVG children necessary to worry about?

@benbcai
Copy link
Contributor

benbcai commented Sep 17, 2019

Another option could be something like this.

+ var i = 0
+ var node;
+ var nodes = root.childNodes;
+ var children = [];

+ while (node = nodes[i++]) {
+ 	if (node.nodeType === Node.ELEMENT_NODE) {
+ 		children.push(node);
+ 	}
+ }
- return Array.from(root.children).reduce((scrolls, elm) => { 
+ return Array.from(children).reduce((scrolls, elm) => { 

@WilcoFiers @straker What are your thoughts?

@benbcai
Copy link
Contributor

benbcai commented Sep 30, 2019

@WilcoFiers @straker When do you plan to release 3.3.3/3.4.0 so that my project can consume the release with this fix?

@benbcai
Copy link
Contributor

benbcai commented Oct 8, 2019

@WilcoFiers @straker Just wanted to follow up to see when you plan to release the next version of axe-core so my team can consume this fix.

@benbcai
Copy link
Contributor

benbcai commented Oct 8, 2019

@WilcoFiers @straker You may ignore my question above. Just noticed the release date here: https://github.com/dequelabs/axe-core/milestone/11

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) fix Bug fixes
Projects
None yet
Development

No branches or pull requests

6 participants