-
Notifications
You must be signed in to change notification settings - Fork 23
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.
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
feat: iframe serialization #468
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🏁 REALLY good stuff, this is going to make a lot of folks happy. 🔥
`) | ||
|
||
const $frameInput = document.getElementById('frame-input') as HTMLIFrameElement | ||
await when(() => !!$frameInput.contentWindow!.performance.timing.loadEventEnd) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️ Love this for handling async
if (!elem.getAttribute('data-percy-element-id')) { | ||
createUID(elem) | ||
elem.setAttribute('data-percy-element-id', createUID()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, thanks for cleaning up my ugly code 🧹
# [0.22.0](v0.21.0...v0.22.0) (2020-02-21) ### Features * iframe serialization ([#468](#468)) ([0bf23af](0bf23af))
Purpose
Currently, we only capture iframes during the asset discovery process. In which we only capture the response of the
src
attribute. If an iframe has been interacted with, navigated within, or built with JavaScript, we cannot properly capture that iframe.This PR aims to solve those limitations
Approach
During DOM serialization, we can retrieve the DOM of an iframe using
iframe.contentDocument
. We can then recursively serialize that DOM, and once serialized set the content as asrcdoc
attribute. Thesrc
attribute is then removed since it is no longer needed and might cause other issues.While dealing with iframes, we can also remove iframes from the
<head>
. These iframes typically have no visual presentation and when they do, they often break pages since rerendering causes the browser to "fix" the<head>
and pushes everything from the iframe down into an implicit<body>
(and ignore the explicit<body>
).The
contentDocument
property returnsnull
for iframes with security restrictions (CORS), so we have no way to serialize those iframes. We also do not want to serialize iframes that have not finished loading (even with CORS, you can access the DOM before it loads and incorrectly serialize an empty DOM). Firefox does not trigger load-related properties for iframes built with JS, so those iframes are always serialized (as long as JS is not enabled).There are simple tests included here, but as a complex example I manually tested this on a live site where the iframe has internal navigation on form submission. Results here.