Skip to content
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

fix: Revert DOM clean up methods #160

Merged
merged 2 commits into from
Apr 18, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions src/percy-agent-client/percy-agent.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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
}

Expand Down
19 changes: 1 addition & 18 deletions src/percy-agent-client/serialize-cssom.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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.
Expand All @@ -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)
})
}
17 changes: 0 additions & 17 deletions src/percy-agent-client/serialize-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
35 changes: 19 additions & 16 deletions test/integration/agent-integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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')
})
})
})
Expand Down
13 changes: 13 additions & 0 deletions test/integration/testcases/stabilize-cssom.html
Original file line number Diff line number Diff line change
@@ -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>
4 changes: 0 additions & 4 deletions test/integration/testcases/stabilize-dom.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
16 changes: 0 additions & 16 deletions test/percy-agent-client/percy-agent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
9 changes: 0 additions & 9 deletions test/percy-agent-client/serialize-cssom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
})
})
12 changes: 0 additions & 12 deletions test/percy-agent-client/serialize-input.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
})