fix: gracefully handle unloaded iframes #133
Conversation
AdnoC
force-pushed
the
fix-unloaded-iframe
branch
from
2273530 to
78dbaec
Compare
| try | ||
| { | ||
| string partialRes = (string)_webDriver.ExecuteAsyncScript(EmbeddedResourceProvider.ReadEmbeddedFile("runPartial.js"), context, options); | ||
| // Important to deserialize because we want to reserialize as an |
There was a problem hiding this comment.
Shouldn't this comment be preserved?
packages/selenium/src/AxeBuilder.cs
Outdated
|
|
||
|
|
||
| // Don't go any deeper if we are just doing top-level iframe | ||
| if (runOptions.Iframes != false) |
There was a problem hiding this comment.
I liked the return early approach this had before
packages/selenium/src/AxeBuilder.cs
Outdated
| { | ||
| // get the proper selector the frame and switch to it | ||
| var selector = _webDriver.ExecuteScript(EmbeddedResourceProvider.ReadEmbeddedFile("shadowSelect.js"), JsonConvert.SerializeObject(context.Selector, AxeJsonSerializerSettings.Default)); | ||
| _webDriver.SwitchTo().Frame(selector as IWebElement); |
There was a problem hiding this comment.
Normally the switchTo of an unloaded iframe is the thing that puts selenium into a bad state, which means trying to switch back to the parent iframe doesn't work. This is the point where the code is needed to switch to the main window then down into the nested frames again.
Since that is not happening and this catches the error and does not rethrow it for the nested call to RunPartialRecursive, does that mean that selenium can handle unloaded iframes without erroring without all this code change?
| frameStack | ||
| )); | ||
| } | ||
| catch (WebDriverTimeoutException) |
There was a problem hiding this comment.
This catch will probably never be hit as you've added try/catches around almost all other other bits of this code that don't rethrow the error.
| foreach (var frameSelector in frameStack) | ||
| { | ||
| var selector = _webDriver.ExecuteScript(EmbeddedResourceProvider.ReadEmbeddedFile("shadowSelect.js"), JsonConvert.SerializeObject(frameSelector, AxeJsonSerializerSettings.Default)); | ||
| if (selector is IWebElement el) |
There was a problem hiding this comment.
What happens if this if statement is false? You never switch to the frame so the next frameSelector is not valid for the next frame. Do we expect this to ever be false?
Closes Issue: #88