chore(origin): fix CDP Page.setDocumentContent origin null crashing a…#3912
chore(origin): fix CDP Page.setDocumentContent origin null crashing a…#3912j-mendez wants to merge 1 commit intodequelabs:developfrom j-mendez:develop
Conversation
|
|
|
@j-mendez did you mean to close this? I was looking into this pr and was able to validate the bug and the fix. Was just about to ask for a test if that was possible (was seeing if it was possible to change |
|
@straker Hi, I actually did not mean to close down the PR. Changing the 'null' for a test seems adequate as well. Is there a certain location or file for the test additions? |
|
Alight, talked with @WilcoFiers on how best to test this. What we think we'd like is a new example in the Something like this (you'll need to update the example // doc/examples/puppeteer/set-content.js
const assert = require('assert');
const browser = await puppeteer.launch();
const page = await browser.newPage();
const axeSource = await fs.readFile('./axe.js', 'utf8');
await page.setContent(`
<html lang="en">
<head>
<title>Test Page</title>
</head>
<body>
<main>
<h1>Hello World</h1>
<button id="empty-button"></button>
<iframe title="iframe" srcdoc="<button id='iframe-empty-button'></button>"></iframe>
</main>
</body>
</html>
`);
await page.evaluate(axeSource);
const frames = page.frames();
for (let i = 0; i < frames.length; i++) {
await frames[i].evaluate(axeSource);
}
const results = await page.evaluate(`window.axe.run()`);
assert(results.violations.length) |
|
@straker feel free to drive this one home, currently do not have the bandwidth. |
|
Looks like you deleted your fork that this pr was attached to. I'll have to close this pr and open a new one to make the changes. |
|
@straker you can use the GitHub cli to checkout the pr or git https://cli.github.com/manual/gh_pr_checkout. |
|
It's not possible to checkout your code through the CLI as it no longer exists
|
|
Wo, didn't know the |
no problem, it should be |
|
So now that I've made the changes, how do I push back to this pr? I don't see a |
I think ‘gh pr push’, not sure only since I have only recently used it to checkout a PR for testing. |
|
Hmm, didn't work. This is an interesting problem. |
Ahh, I’m not at the computer right now. Feel free to merge the changes locally to another branch and re push the PR or etc and thank you for taking it over! |
|
Moved pr to #3921. |
Page.setDocumentContent--- Notes
The following issue occurs when using axe-core to test HTML markup created over CDP
Page.setDocumentContentorpuppeteer.setContent.Example test showing the crash repro https://github.com/a11ywatch/a11y/blob/main/tests/basic-axe.ts.
You also need to comment out the following block https://github.com/a11ywatch/a11y/blob/main/lib/runners/axe.ts#L16 that allowing test to run after setting the
originmanually.