Skip to content

Commit

Permalink
fix: Remove loading attributes from cloned iframes (#555)
Browse files Browse the repository at this point in the history
This attribute causes asset discovery to hang and eventually timeout since we
don't scroll the page. Removing the attribute is ideally what we want (to load
the iframe).
  • Loading branch information
Robdel12 authored Aug 28, 2020
1 parent 45b27ee commit 83185ec
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/percy-agent-client/dom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,6 @@ class DOM {
// rerendered and do not effect the visuals of a page
if (clonedDOM.head.contains(cloned)) {
cloned!.remove()

// if the frame document is accessible, we can serialize it
} else if (frame.contentDocument) {
// js is enabled and this frame was built with js, don't serialize it
Expand All @@ -188,6 +187,9 @@ class DOM {
} else if (!enableJavaScript && builtWithJs) {
cloned!.remove()
}

// Remove any lazy loading attributes, since they hang asset discovery
cloned!.removeAttribute('loading')
}
}

Expand Down
5 changes: 5 additions & 0 deletions test/unit/percy-agent-client/dom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ describe('DOM -', () => {
createExample(`
<iframe id="frame-external" src="https://example.com"></iframe>
<iframe id="frame-input" srcdoc="<input/>"></iframe>
<iframe id="frame-lazy" srcdoc="<input/>" loading="lazy"></iframe>
<iframe id="frame-js" src="javascript:void(
this.document.body.innerHTML = '<p>made with js src</p>'
)"></iframe>
Expand Down Expand Up @@ -386,6 +387,10 @@ describe('DOM -', () => {
expect($dom('#frame-js-inject')).to.not.have.length
})

it('removes loading attributes on iframes', () => {
expect($dom('#frame-lazy').attr('loading')).to.be.undefined
})

it('does not serialize iframes with CORS', () => {
expect($dom('#frame-external').attr('src')).to.equal('https://example.com')
expect($dom('#frame-external').attr('srcdoc')).to.be.undefined
Expand Down

0 comments on commit 83185ec

Please sign in to comment.