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.

Sign up for GitHub

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

Jump to bottom

axe.run crashes if an iframe's contentDocument is used #3864

Closed
1 task done
TastyPi opened this issue Jan 19, 2023 · 3 comments · Fixed by #4072
Closed
1 task done

axe.run crashes if an iframe's contentDocument is used #3864

TastyPi opened this issue Jan 19, 2023 · 3 comments · Fixed by #4072
Assignees
@straker
Labels
core Issues in the core code (lib/core) fix Bug fixes pr A pr has been created for the issue
Milestone
Axe-core 4.8

Comments

@TastyPi
Copy link

TastyPi commented Jan 19, 2023

Product

axe-core

Product Version

4.6.2

Latest Version

  • I have tested the issue with the latest version of the product

Issue Description

Expectation

When calling axe.run with an iframe's contentDocument I expect it to perform the checks on just the iframe's document.

Actual

Throws an error:

Uncaught RangeError: Maximum call stack size exceeded
    at clone (axe.js:6044:14)
    at clone (axe.js:6059:26)
    at clone (axe.js:6059:26)
    at clone (axe.js:6059:26)
    at clone (axe.js:6059:26)
    at clone (axe.js:6059:26)
    at clone (axe.js:6059:26)
    at clone (axe.js:6059:26)
    at clone (axe.js:6059:26)
    at clone (axe.js:6059:26)

How to Reproduce

  1. Create a HTML file with the following content:
<!DOCTYPE html>
<html>
  <head>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/axe-core/4.6.2/axe.js"></script>
    <script>
      function runAxe() {
        const iframe = document.getElementById("iframe")
        
        axe.run(iframe).then(result => console.log("Running on the element succeeds"))
        axe.run(iframe.contentDocument).then(result => console.log("You won't see this"))
      }
    </script>
  </head>
  <body>
    <iframe id="iframe"></iframe>
    <button onclick="runAxe()">Run</button>
  </body>
</html>
  1. Server it locally with a basic HTTP server (e.g. python -m http.server)
  2. Visit the file in a browser and click the "Run" button
  3. See the stack trace in the console

Additional context

The clone function avoids cloning HTML elements by checking if it is an instance of window.Node, however because the iframe's context is different Node instances from inside the iframe are not instances of window.Node, they are instances of iframe.contentWindow.Node.

The solution to this might just be to say "you can't do that, if you want to test an iframe just pass the iframe's element and make sure the iframe option is true", but I figured it might be possible to find an easy fix if the clone function can detect Node instances from iframes.
@TastyPi TastyPi added the ungroomed Ticket needs a maintainer to prioritize and label label Jan 19, 2023
@WilcoFiers
Copy link
Contributor

WilcoFiers commented Jan 20, 2023 •
edited
Loading

you can't do that, if you want to test an iframe just pass the iframe's element and make sure the iframe option is true

That sounds about right! Although if you wanted to only test the iframe content, this is how you'd do it:

// Make sure to load axe into the iframe first. This is required.
const { document, axe } = iframe.contentWindow;
axe.run(document).then(result => console.log(result));

Or, you could use a selector (also requires loading axe into the frame):

// Test everything inside `#my-iframe`:
axe.run({ fromFrames: ['#my-iframe', ':root'] })

There are some other issues with your example. The first is that you're running axe a second time without waiting for it to complete. It can't do that. And the second is that by testing the iframe you get its content tested too. It does require that you load axe into that iframe, but that's necessary regardless. Axe-core isn't built to do what you're trying to accomplish here. I'd wager there are more things that don't work. We'd have to read through the entire code base to figure out all the many ways in which this might not work. For something that's easy to work around, I'm not sure thats worth the effort.

What I think we should do is make sure axe doesn't get into that maximum call stack situation. We should sniff out situations like this and throw with an error message that makes the issue clear.

@WilcoFiers WilcoFiers added fix Bug fixes core Issues in the core code (lib/core) and removed ungroomed Ticket needs a maintainer to prioritize and label labels Jan 20, 2023
@WilcoFiers WilcoFiers added this to the Axe-core 4.8 milestone Jan 20, 2023
@TastyPi
Copy link
Author

TastyPi commented Jan 20, 2023

I figured out your first suggestion shortly after making this issue, and definitely think it's the nicest solution. My actual code doesn't run axe.run in quick succession like the example, though because of the maximum call stack issue it doesn't actually get to the part where async work starts, so they actually are run one after the other 😛.

I think the ideal fix to this would be to detect it is a node from an iframe and essentially fallback on the first suggestion, but adding something that detects self-references in clone and throws an error or something would be good.

Either way, I'm definitely fine with using the first suggestion so I don't actually need anything to be fixed, it would just be a QOL improvement.

@straker straker mentioned this issue Jun 28, 2023
Merged
@straker straker added the pr A pr has been created for the issue label Jun 28, 2023
@straker straker self-assigned this Jun 28, 2023
@padmavemulapati
Copy link

Verified with the latest axe-core develop branch code base and console log (using axe4.7.2.min.js source)

Not seeing any stack trace for error log, getting proper axe.run() logs

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@straker straker

Labels
core Issues in the core code (lib/core) fix Bug fixes pr A pr has been created for the issue
Projects
None yet
Milestone
Axe-core 4.8
Development

Successfully merging a pull request may close this issue.

fix(utils/clone): don't try to clone elements from different window context dequelabs/axe-core
4 participants
@WilcoFiers @TastyPi @straker @padmavemulapati