fix(🐛): Remove inaccessible JS frames when serializing (#512)

* 🐛 Remove inaccessible JS frames when serializing * 🚨 Appease tsc
percy · May 21, 2020 · 86cd887 · 86cd887
1 parent 71d7b97
commit 86cd887
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 4 deletions.
diff --git a/src/percy-agent-client/dom.ts b/src/percy-agent-client/dom.ts
@@ -158,9 +158,12 @@ class DOM {
   }
 
   private serializeFrameElements(clonedDOM: HTMLDocument) {
+    const { enableJavaScript } = this.options
+
     for (const frame of this.originalDOM.querySelectorAll('iframe')) {
       const percyElementId = frame.getAttribute('data-percy-element-id')
       const cloned = clonedDOM.querySelector(`[data-percy-element-id="${percyElementId}"]`)
+      const builtWithJs = !frame.srcdoc && (!frame.src || frame.src.split(':')[0] === 'javascript')
 
       // delete frames within the head since they usually break pages when
       // rerendered and do not effect the visuals of a page
@@ -169,11 +172,8 @@ class DOM {
 
       // if the frame document is accessible, we can serialize it
       } else if (frame.contentDocument) {
-        const builtWithJs = !frame.srcdoc && (!frame.src || frame.src.split(':')[0] === 'javascript')
-
         // js is enabled and this frame was built with js, don't serialize it
-        if (this.options.enableJavaScript && builtWithJs) { continue }
-
+        if (enableJavaScript && builtWithJs) { continue }
         // the frame has yet to load and wasn't built with js, it is unsafe to serialize
         if (!builtWithJs && !frame.contentWindow!.performance.timing.loadEventEnd) { continue }
 
@@ -182,6 +182,11 @@ class DOM {
         cloned!.setAttribute('srcdoc', frameDOM.snapshotString())
         // srcdoc cannot exist in tandem with src
         cloned!.removeAttribute('src')
+
+      // delete inaccessible frames built with js when js is disabled because
+      // they break asset discovery by creating non-captured requests that hang
+      } else if (!enableJavaScript && builtWithJs) {
+        cloned!.remove()
       }
     }
   }

diff --git a/test/unit/percy-agent-client/dom.test.ts b/test/unit/percy-agent-client/dom.test.ts
@@ -349,6 +349,11 @@ describe('DOM -', () => {
         const $frameJS = document.getElementById('frame-js-no-src') as HTMLIFrameElement
         $frameJS.contentDocument!.body.innerHTML = '<p>generated iframe</p>'
 
+        const $frameInject = document.createElement('iframe') as HTMLIFrameElement
+        $frameInject.setAttribute('src', 'javascript:false')
+        $frameInject.setAttribute('id', 'frame-js-inject')
+        document.querySelector('.container')!.appendChild($frameInject)
+
         $dom = cheerio.load(new DOM(document).snapshotString())
       })
 
@@ -360,6 +365,7 @@ describe('DOM -', () => {
           '</body></html>',
         ].join(''))
 
+
         expect($dom('#frame-js-no-src').attr('src')).to.be.undefined
         expect($dom('#frame-js-no-src').attr('srcdoc')).to.equal([
           '<!DOCTYPE html><html><head></head><body>',
@@ -376,6 +382,10 @@ describe('DOM -', () => {
         ].join('')))
       })
 
+      it('removes inaccessible JS iframes', () => {
+        expect($dom('#frame-js-inject')).to.not.have.length
+      })
+
       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