fix: skip unloaded iframes #330
Conversation
AdnoC
force-pushed
the
fix-unloaded-iframe
branch
from
fb3fff1 to
3105bc4
Compare
selenium/src/main/java/com/deque/html/axecore/selenium/AxeBuilder.java
Outdated
Show resolved
Hide resolved
AdnoC
force-pushed
the
fix-unloaded-iframe
branch
from
7f6cd95 to
aba88c3
Compare
| @@ -18,9 +18,8 @@ | |||
| }, | |||
| "node_modules/ansi-styles": { | |||
| "version": "4.3.0", | |||
| "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-4.3.0.tgz", | |||
There was a problem hiding this comment.
Why was this file changed so much? I don't think package-lock should be removing the resolved and integrity values?
| webDriver.manage().timeouts().scriptTimeout(Duration.ofSeconds(timeout)); | ||
| Duration pageTimeout = webDriver.manage().timeouts().getPageLoadTimeout(); | ||
| try { | ||
| webDriver.manage().timeouts().pageLoadTimeout(FRAME_LOAD_TIMEOUT); |
There was a problem hiding this comment.
I think we only need to set the page load timeout if we're using analyzePost43x. Im not sure we want to change the legacy version to use a shorter page load timeout since it was only causing problems when trying to switchTo the iframe
| try { | ||
| Object frameContext = AxeReporter.serialize(fc.getFrameContext()); | ||
| Object frameSelector = AxeReporter.serialize(fc.getFrameSelector()); | ||
| frameStack.push(frameSelector); |
There was a problem hiding this comment.
Should only push to the frameStack when we've successfully switchTo the frame. Then you don't have to pop it later
There was a problem hiding this comment.
Also missing the test that verifies we reset page load timeout to user setting
Co-authored-by: Stephen Mathieson <571265+stephenmathieson@users.noreply.github.com>
AdnoC
force-pushed
the
fix-unloaded-iframe
branch
from
aba88c3 to
b658a5d
Compare
AdnoC
dismissed
straker’s stale review
Made page load changes. Added test. Reverted package-lock change
| ArrayList<String> morePartialResults = | ||
| runPartialRecursive(webDriver, options, frameContext, false); | ||
| partialResults.addAll(morePartialResults); | ||
| frameStack.pop(); |
There was a problem hiding this comment.
Shouldn't need this anymore, and in fact this would probably cause an issue since you're popping the parent frame before all children frame have been processed:
For example, if you had this as the frame stack ['ancestor', 'parent'] and are currently under parent frame. If parent frame had 2 subframes and the first one is unloaded, you would switch to the parent frame in the catch statement but then pop parent off the frame stack and would have a bad frame stack when trying to switch to the 2nd child.
There was a problem hiding this comment.
The catch statement ends with a continue. The pop only occurs if we get to the end of the loop without exceptions
Issue: #323