fix: use frame query that supports shadow dom #492
Conversation
lib/core/public/cleanup-plugins.js
Outdated
| @@ -23,7 +23,7 @@ function cleanupPlugins(resolve, reject) { | |||
| }); | |||
| }); | |||
|
|
|||
| axe.utils.toArray(document.querySelectorAll('frame, iframe')).forEach(function (frame) { | |||
| axe.utils.getFrames().forEach(function (frame) { | |||
There was a problem hiding this comment.
You should be able to use axe.utils.querySelectorAll(axe._tree, 'frame, iframe') instead.
There was a problem hiding this comment.
Thanks. I'll give that a try.
isner
changed the title
|
This enables |
| @@ -23,14 +23,18 @@ function cleanupPlugins(resolve, reject) { | |||
| }); | |||
| }); | |||
|
|
|||
| axe.utils.toArray(document.querySelectorAll('frame, iframe')).forEach(function (frame) { | |||
| var flattenedTree = axe.utils.getFlattenedTree(document.body) | |||
There was a problem hiding this comment.
I had to create a new flattened tree to support this functionality. The only other place in which a flattened tree is created is during the audit itself. Since axe.cleanup can be called at any arbitrary time, we can't reuse the same tree.
There was a problem hiding this comment.
Turns out there was a lint error in this line (missing ;). Very strange this didn't get picked up in circle.
lib/core/public/cleanup-plugins.js
Outdated
| command: 'cleanup-plugin' | ||
| }, res, rej); | ||
| }); | ||
| }); | ||
|
|
||
| flattenedTree = undefined |
There was a problem hiding this comment.
We don't need this. Garbage collector will do this for you.
There was a problem hiding this comment.
Will do. I'm curious, though, why we need to manually free up the memory here: https://github.com/dequelabs/axe-core/blob/develop/lib/core/base/audit.js#L182
There was a problem hiding this comment.
Because it's a global object. Here it's just a local variable. Once nothing can reference it anymore, it's picked up by the garbage collector.
| q.defer(function (res, rej) { | ||
| return axe.utils.sendCommandToFrame(frame, { | ||
| return axe.utils.sendCommandToFrame(node.actualNode, { |
There was a problem hiding this comment.
This is super tweaky, but I wonder if it would be more intuitive to write virtualNode.actualNode or vNode.actualNode so it communicates what that top level object is. That would help in other places too (but changing it everywhere is obvs outside the scope of this PR)
…js (dequelabs#492) Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Supports [WWD-845] https://dequesrc.atlassian.net/browse/WWD-845