fix: Revert DOM clean up methods (#160)

* Revert DOM clean up methods

Given this JSBin: https://jsbin.com/?html,console,output

When we serialize the CSSOM into the `<style>` tag that the in-memory
styles own and then _remove_ those styles, it triggers `deleteRule` on
those styles in the CSSOM. Which means we're removing the text we
injected AND all of the styles that live in memory. This breaks their
app.

With the input cleaning, when we remove the value or
checked attribute on the element, it's possible those elements lose
their values entirely. For example, when we remove the `checked`
attribute from checkboxes in the customers DOM, this will make the
checkbox _no longer checked_, which is changing the state of their
application. It would be better, in this world without an AST, to
leave the mutations in place. Removing the mutations causes more
issues than it tries to solve.

* Update test cases

src/percy-agent-client/percy-agent.ts

@@ -1,8 +1,8 @@
 import Constants from '../services/constants'
 import {ClientOptions} from './client-options'
 import {PercyAgentClient} from './percy-agent-client'
-import {cleanSerializedCssOm, serializeCssOm} from './serialize-cssom'
-import {cleanSerializedInputElements, serializeInputElements} from './serialize-input'
+import {serializeCssOm} from './serialize-cssom'
+import {serializeInputElements} from './serialize-input'
 import {SnapshotOptions} from './snapshot-options'
 
 export default class PercyAgent {
@@ -71,13 +71,6 @@ export default class PercyAgent {
 
     const snapshotString = doctype + domClone.outerHTML
 
-    // Remove all the additions we've made to the original DOM.
-    // Ideally we would make a deep clone of the original DOM at the start of this
-    // method, and operate on that, but this turns out to be hard to do while
-    // retaining CSS OM and input element state. Instead, we carefully remove what we added.
-    cleanSerializedCssOm(documentObject)
-    cleanSerializedInputElements(documentObject)
-
     return snapshotString
   }
 

src/percy-agent-client/serialize-cssom.ts

@@ -1,5 +1,4 @@
 const DATA_ATTRIBUTE = 'data-percy-cssom-serialized'
-const START_COMMENT = '/* Start of Percy serialized CSSOM */'
 
 // Take all the CSS created in the CSS Object Model (CSSOM), and inject it
 // into the DOM so Percy can render it safely in our browsers.
@@ -18,7 +17,7 @@ export function serializeCssOm(document: HTMLDocument) {
         .call(styleSheet.cssRules)
         .reduce((prev: string, cssRule: CSSRule) => {
           return prev + cssRule.cssText
-        }, `${START_COMMENT}\n`)
+        }, '')
 
       // Append the serialized styles to the styleSheet's ownerNode to minimize
       // the chances of messing up the cascade order.
@@ -29,19 +28,3 @@ export function serializeCssOm(document: HTMLDocument) {
 
   return document
 }
-
-export function cleanSerializedCssOm(document: HTMLDocument) {
-  // IMPORTANT: querySelectorAll(...) will not always work. In particular, in certain
-  // cases with malformed HTML (e.g. a <style> tag inside of another one), some of
-  // the elements we are looking for will not be returned. In that case, we will
-  // leave traces of ourselves in the underlying DOM.
-  const nodes = document.querySelectorAll(`[${DATA_ATTRIBUTE}]`)
-  Array.from(nodes).forEach((node: Element) => {
-    node.removeAttribute(DATA_ATTRIBUTE)
-    const startOfSerialized = node.innerHTML.indexOf(START_COMMENT)
-    if (startOfSerialized < 0) {
-      return
-    }
-    node.innerHTML = node.innerHTML.substring(0, startOfSerialized)
-  })
-}

src/percy-agent-client/serialize-input.ts

@@ -32,20 +32,3 @@ export function serializeInputElements(doc: HTMLDocument): HTMLDocument {
 
   return doc
 }
-
-export function cleanSerializedInputElements(doc: HTMLDocument) {
-  doc.querySelectorAll(`[${DATA_ATTRIBUTE_CHECKED}]`).forEach((el: Element) => {
-    el.removeAttribute('checked')
-    el.removeAttribute(DATA_ATTRIBUTE_CHECKED)
-  })
-  doc.querySelectorAll(`[${DATA_ATTRIBUTE_TEXTAREA_INNERTEXT}]`).forEach((el: Element)  => {
-    const originalInnerText: string = el.getAttribute(DATA_ATTRIBUTE_TEXTAREA_INNERTEXT) || ''
-    const textArea = el as HTMLTextAreaElement
-    textArea.innerText = originalInnerText
-    el.removeAttribute(DATA_ATTRIBUTE_TEXTAREA_INNERTEXT)
-  })
-  doc.querySelectorAll(`[${DATA_ATTRIBUTE_VALUE}]`).forEach((el: Element) => {
-    el.removeAttribute('value')
-    el.removeAttribute(DATA_ATTRIBUTE_VALUE)
-  })
-}

test/integration/agent-integration.test.ts

@@ -67,8 +67,8 @@ describe('Integration test', () => {
       const domSnapshot = await snapshot(page, 'Buildkite snapshot')
       expect(domSnapshot).contains('Buildkite')
     })
-  })
 
+  })
   describe('on local test cases', () => {
     const testCaseDir = `${__dirname}/testcases`
     const PORT = 8000
@@ -96,33 +96,36 @@ describe('Integration test', () => {
         await page.goto(`http://localhost:${PORT}/stabilize-dom.html`)
       })
 
-      it('serializes CSSOM', async () => {
-        await page.evaluate(() => {
-          const jsStyledDiv = document.getElementById('jsStyled') as HTMLInputElement
-          jsStyledDiv.style.background = 'mediumorchid'
-        })
-
-        const domSnapshot = await snapshot(page, 'Serialize CSSOM')
-        expect(domSnapshot).to.contain('data-percy-cssom-serialized')
-        expect(domSnapshot).to.contain('background: mediumorchid')
-      })
-
       it('serializes input elements', async () => {
         await page.type('#testInputText', 'test input value')
+        await page.type('#testTextarea', 'test textarea value')
         await page.click('#testCheckbox')
         await page.click('#testRadioButton')
 
         const domSnapshot = await snapshot(page, 'Serialize input elements')
         expect(domSnapshot).to.contain('test input value')
         expect(domSnapshot).to.contain('type="checkbox" checked')
         expect(domSnapshot).to.contain('type="radio" checked')
+        expect(domSnapshot).to.contain('test textarea value')
       })
+    })
 
-      it('serializes textarea elements', async () => {
-        await page.type('#testTextarea', 'test textarea value')
+    describe('stablizes CSSOM', () => {
+      before(async () => {
+        await page.goto(`http://localhost:${PORT}/stabilize-cssom.html`)
+      })
+
+      it('serializes the CSSOM', async () => {
+        const domSnapshot = await snapshot(page, 'Serialize CSSOM')
+
+        expect(domSnapshot).to.contain('data-percy-cssom-serialized')
+        expect(domSnapshot).to.contain('.box { height: 500px; width: 500px; background-color: green; }')
+
+        // we want to ensure mutiple snapshots are successful
+        const secondDomSnapshot = await snapshot(page, 'Serialize CSSOM twice')
+        expect(secondDomSnapshot).to.contain('data-percy-cssom-serialized')
+        expect(secondDomSnapshot).to.contain('.box { height: 500px; width: 500px; background-color: green; }')
 
-        const domSnapshot = await snapshot(page, 'Serialize textarea elements')
-        expect(domSnapshot).to.contain('test textarea value')
       })
     })
   })

test/integration/testcases/stabilize-cssom.html

@@ -0,0 +1,13 @@
+<html>
+  <head>
+    <title>CSSOM testing</title>
+    <style type="text/css"></style>
+    <script>
+     let styleSheet = document.styleSheets[0];
+     styleSheet.insertRule('.box { height: 500px; width: 500px; background-color: green; }')
+    </script>
+  </head>
+  <body>
+    <div class="box"></div>
+  </body>
+</html>

test/integration/testcases/stabilize-dom.html

@@ -15,9 +15,5 @@
       <textarea id="testTextarea" name="testTextArea" rows="10"></textarea>
     </p>
     <!-- End of test elements for input element serialization. -->
-
-    <!-- Test element for CSSOM serialization. -->
-    <div id="jsStyled">Magically styled ✨</div>
-
   </body>
 </html>

test/percy-agent-client/percy-agent.test.ts

@@ -58,21 +58,5 @@ describe('PercyAgent', () => {
       expect(requestBody.widths).to.eql([320, 1024])
       expect(requestBody.minHeight).to.equal(512)
     })
-
-    it('does not alter the DOM being snapshotted', () => {
-      const originalHTML = htmlWithoutSelector(document, '#mocha')
-
-      subject.snapshot('a snapshot')
-
-      const postSnapshotHTML = htmlWithoutSelector(document, '#mocha')
-      expect(postSnapshotHTML).to.eq(originalHTML)
-      expect(postSnapshotHTML).to.not.contain('data-percy')
-    })
-
-    it('multiple snapshots produce the same result', () => {
-      const firstDOMSnapshot = subject.snapshot('a snapshot')
-      const secondDOMSnapshot = subject.snapshot('a second snapshot')
-      expect(secondDOMSnapshot).to.eq(firstDOMSnapshot)
-    })
   })
 })

test/percy-agent-client/serialize-cssom.test.ts

@@ -18,14 +18,5 @@ describe('serializeCssOm', () => {
       const parsedDoc = (new DOMParser()).parseFromString(domSnapshot, 'text/html')
       expect(parsedDoc.getElementById('jsStyled')!.style.background).to.contain('red')
     })
-
-    it('cleans up after itself', () => {
-      subject.snapshot('test snapshot')
-
-      const postSnapshotHTML = htmlWithoutSelector(document, '#mocha')
-
-      expect(postSnapshotHTML).to.not.contain('data-percy-cssom-serialized')
-      expect(postSnapshotHTML).to.not.contain('Start of Percy serialized CSSOM')
-    })
   })
 })

test/percy-agent-client/serialize-input.test.ts

@@ -32,16 +32,4 @@ describe('serializeInputElements', () => {
 
     expect(domSnapshot).to.contain('checked')
   })
-
-  it('cleans up after itself', () => {
-    const preSnapshotHTML = htmlWithoutSelector(document, '#mocha')
-
-    subject.snapshot('test snapshot')
-
-    const postSnapshotHTML = htmlWithoutSelector(document, '#mocha')
-
-    expect(postSnapshotHTML).to.eq(preSnapshotHTML)
-    expect(postSnapshotHTML).to.not.contain('data-percy-input-serialized')
-    expect(postSnapshotHTML).to.not.contain('checked')
-  })
 })