From 61113a96790feae3079cbe564d3aab982fa70bc4 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Wed, 28 Feb 2024 13:54:46 +0530 Subject: [PATCH 1/5] Added support for image srcset capturing --- packages/core/src/config.js | 5 +++ packages/core/src/discovery.js | 6 ++++ packages/core/src/page.js | 20 ++++++----- packages/core/src/snapshot.js | 1 + packages/core/types/index.d.ts | 1 + packages/dom/src/index.js | 2 ++ packages/dom/src/serialize-image-srcset.js | 41 ++++++++++++++++++++++ 7 files changed, 68 insertions(+), 8 deletions(-) create mode 100644 packages/dom/src/serialize-image-srcset.js diff --git a/packages/core/src/config.js b/packages/core/src/config.js index dc69ed8c2..f00012944 100644 --- a/packages/core/src/config.js +++ b/packages/core/src/config.js @@ -190,6 +190,10 @@ export const configSchema = { type: 'boolean', default: false }, + captureSrcset: { + type: 'boolean', + default: false + }, requestHeaders: { type: 'object', normalize: false, @@ -276,6 +280,7 @@ export const snapshotSchema = { authorization: { $ref: '/config/discovery#/properties/authorization' }, disableCache: { $ref: '/config/discovery#/properties/disableCache' }, captureMockedServiceWorker: { $ref: '/config/discovery#/properties/captureMockedServiceWorker' }, + captureSrcset: { $ref: '/config/discovery#/properties/captureSrcset' }, userAgent: { $ref: '/config/discovery#/properties/userAgent' }, devicePixelRatio: { $ref: '/config/discovery#/properties/devicePixelRatio' } } diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index b22055612..3487ebdbd 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -55,6 +55,7 @@ function debugSnapshotOptions(snapshot) { debugProp(snapshot, 'discovery.authorization', JSON.stringify); debugProp(snapshot, 'discovery.disableCache'); debugProp(snapshot, 'discovery.captureMockedServiceWorker'); + debugProp(snapshot, 'discovery.captureSrcset'); debugProp(snapshot, 'discovery.userAgent'); debugProp(snapshot, 'clientInfo'); debugProp(snapshot, 'environmentInfo'); @@ -171,6 +172,11 @@ async function* captureSnapshotResources(page, snapshot, options) { yield page.evaluate(snapshot.execute.afterNavigation); } + if (discovery.captureSrcset) { + await page.insertPercyDom(); + yield page.eval('window.PercyDOM.serializeImageSrcSet()'); + } + // iterate over additional snapshots for proper DOM capturing for (let additionalSnapshot of [baseSnapshot, ...additionalSnapshots]) { let isBaseSnapshot = additionalSnapshot === baseSnapshot; diff --git a/packages/core/src/page.js b/packages/core/src/page.js index fbed6ea83..0b8b8db53 100644 --- a/packages/core/src/page.js +++ b/packages/core/src/page.js @@ -132,6 +132,17 @@ export class Page { for (let script of scripts) await this.eval(script); } + async insertPercyDom() { + // inject @percy/dom for serialization by evaluating the file contents which adds a global + // PercyDOM object that we can later check against + /* istanbul ignore next: no instrumenting injected code */ + if (await this.eval(() => !window.PercyDOM)) { + this.log.debug('Inject @percy/dom', this.meta); + let script = await fs.promises.readFile(PERCY_DOM, 'utf-8'); + await this.eval(new Function(script)); /* eslint-disable-line no-new-func */ + } + } + // Takes a snapshot after waiting for any timeout, waiting for any selector, executing any // scripts, and waiting for the network idle. Returns all other provided snapshot options along // with the captured URL and DOM snapshot. @@ -165,14 +176,7 @@ export class Page { // wait for any final network activity before capturing the dom snapshot await this.network.idle(); - // inject @percy/dom for serialization by evaluating the file contents which adds a global - // PercyDOM object that we can later check against - /* istanbul ignore next: no instrumenting injected code */ - if (await this.eval(() => !window.PercyDOM)) { - this.log.debug('Inject @percy/dom', this.meta); - let script = await fs.promises.readFile(PERCY_DOM, 'utf-8'); - await this.eval(new Function(script)); /* eslint-disable-line no-new-func */ - } + this.insertPercyDom(); // serialize and capture a DOM snapshot this.log.debug('Serialize DOM', this.meta); diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js index be5f37ae6..1f6ffecd8 100644 --- a/packages/core/src/snapshot.js +++ b/packages/core/src/snapshot.js @@ -117,6 +117,7 @@ function getSnapshotOptions(options, { config, meta }) { authorization: config.discovery.authorization, disableCache: config.discovery.disableCache, captureMockedServiceWorker: config.discovery.captureMockedServiceWorker, + captureSrcset: config.discovery.captureSrcset, userAgent: config.discovery.userAgent } }, options], (path, prev, next) => { diff --git a/packages/core/types/index.d.ts b/packages/core/types/index.d.ts index 1b8eec50b..fecedd2e7 100644 --- a/packages/core/types/index.d.ts +++ b/packages/core/types/index.d.ts @@ -19,6 +19,7 @@ interface DiscoveryOptions { allowedHostnames?: string[]; disableCache?: boolean; captureMockedServiceWorker?: boolean; + captureSrcset?: boolean; } interface ScopeOptions { diff --git a/packages/dom/src/index.js b/packages/dom/src/index.js index 9860612f6..4367bab66 100644 --- a/packages/dom/src/index.js +++ b/packages/dom/src/index.js @@ -4,3 +4,5 @@ export { // namespace alias serializeDOM as serialize } from './serialize-dom'; + +export { serializeImageSrcSet } from './serialize-image-srcset'; diff --git a/packages/dom/src/serialize-image-srcset.js b/packages/dom/src/serialize-image-srcset.js new file mode 100644 index 000000000..338b3d838 --- /dev/null +++ b/packages/dom/src/serialize-image-srcset.js @@ -0,0 +1,41 @@ +function getSrcsets(dom) { + const links = new Set(); + + for (let img of dom.querySelectorAll('img[srcset]')) { + handleSrcSet(img.srcset, links); + } + + for (let picture of dom.querySelectorAll('picture')) { + for (let source of picture.querySelectorAll('source')) { + handleSrcSet(source.srcset, links); + } + } + return Array.from(links); +} + +function handleSrcSet(srcSet, links) { + srcSet = srcSet.split(/,\s+/); + for (let src of srcSet) { + src = src.trim(); + src = src.split(' ')[0]; + links.add(getFormattedLink(src)); + } +} +function getFormattedLink(src) { + const anchor = document.createElement('a'); + anchor.href = src; + return anchor.href; +} + +export function serializeImageSrcSet() { + const allImgTags = []; + const links = getSrcsets(document); + for (const link of links) { + const img = document.createElement('img'); + img.src = link; + allImgTags.push(img); + } + // Adding to window so GC won't abort request + window.allImgTags = allImgTags; +} +export default serializeImageSrcSet; From b62f1749a306e9c8916eabc1306ccf870525f17a Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Wed, 28 Feb 2024 14:45:10 +0530 Subject: [PATCH 2/5] Added tests + addressed comments --- packages/core/src/discovery.js | 5 +- packages/core/test/discovery.test.js | 103 ++++++++++++++++++ packages/dom/src/index.js | 2 +- packages/dom/src/serialize-image-srcset.js | 5 +- .../dom/test/serialize-image-srcset.test.js | 51 +++++++++ 5 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 packages/dom/test/serialize-image-srcset.test.js diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 3487ebdbd..aa83369b8 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -172,9 +172,12 @@ async function* captureSnapshotResources(page, snapshot, options) { yield page.evaluate(snapshot.execute.afterNavigation); } + // Running before page idle since this will trigger many network calls + // so need to run as early as possible. plus it is just reading urls from dom srcset + // which will be already loaded after navigation complete if (discovery.captureSrcset) { await page.insertPercyDom(); - yield page.eval('window.PercyDOM.serializeImageSrcSet()'); + yield page.eval('window.PercyDOM.loadAllSrcsetLinks()'); } // iterate over additional snapshots for proper DOM capturing diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index a1d6aa7b5..9a2ca323c 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -2039,4 +2039,107 @@ describe('Discovery', () => { ])); }); }); + + describe('Capture image srcset =>', () => { + it('make request call to capture resource', async () => { + let responsiveDOM = dedent` + + + +

Hello Percy!

+ + + + `; + server.reply('/img-fromsrcset.png', () => [200, 'image/png', pixel]); + server.reply('/img-already-captured.png', () => [200, 'image/png', pixel]); + server.reply('/img-throwserror.gif', () => [404]); + server.reply('/img-withdifferentcontenttype.gif', () => [200, 'image/gif', pixel]); + + let capturedResource = { + url: 'http://localhost:8000/img-already-captured.png', + content: 'R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==', + mimetype: 'image/png' + }; + await percy.snapshot({ + name: 'test responsive', + url: 'http://localhost:8000', + domSnapshot: { + html: responsiveDOM, + resources: [capturedResource] + }, + discovery: { + captureSrcset: true + } + }); + + await percy.idle(); + + let resource = path => jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': `http://localhost:8000${path}` + }) + }); + + let paths = server.requests.map(r => r[0]); + expect(paths).toContain('/img-fromsrcset.png'); + expect(paths).toContain('/img-withdifferentcontenttype.gif'); + expect(paths).toContain('/img-throwserror.gif'); + expect(captured[0]).toEqual(jasmine.arrayContaining([ + resource('/img-fromsrcset.png'), + resource('/img-withdifferentcontenttype.gif') + ])); + }); + + it('using snapshot command capture srcset', async () => { + let responsiveDOM = dedent` + + + +

Hello Percy!

+ + + + `; + + server.reply('/', () => [200, 'text/html', responsiveDOM]); + server.reply('/img-fromsrcset.png', () => [200, 'image/png', pixel]); + server.reply('/img-already-captured.png', () => [200, 'image/png', pixel]); + server.reply('/img-throwserror.jpeg', () => [404]); + server.reply('/img-withdifferentcontenttype.gif', () => [200, 'image/gif', pixel]); + + await percy.snapshot({ + name: 'image srcset', + url: 'http://localhost:8000', + widths: [1024], + discovery: { + captureSrcset: true + } + }); + + await percy.idle(); + + let resource = path => jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': `http://localhost:8000${path}` + }) + }); + + expect(captured[0]).toEqual(jasmine.arrayContaining([ + resource('/img-fromsrcset.png'), + resource('/img-withdifferentcontenttype.gif'), + resource('/img-already-captured.png') + ])); + + expect(captured[0]).not.toContain(jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': 'https://remote.resource.com/img-shouldnotcaptured.png' + }) + })); + }); + }); }); diff --git a/packages/dom/src/index.js b/packages/dom/src/index.js index 4367bab66..3370031eb 100644 --- a/packages/dom/src/index.js +++ b/packages/dom/src/index.js @@ -5,4 +5,4 @@ export { serializeDOM as serialize } from './serialize-dom'; -export { serializeImageSrcSet } from './serialize-image-srcset'; +export { loadAllSrcsetLinks } from './serialize-image-srcset'; diff --git a/packages/dom/src/serialize-image-srcset.js b/packages/dom/src/serialize-image-srcset.js index 338b3d838..68b8ed045 100644 --- a/packages/dom/src/serialize-image-srcset.js +++ b/packages/dom/src/serialize-image-srcset.js @@ -27,7 +27,7 @@ function getFormattedLink(src) { return anchor.href; } -export function serializeImageSrcSet() { +export function loadAllSrcsetLinks() { const allImgTags = []; const links = getSrcsets(document); for (const link of links) { @@ -37,5 +37,6 @@ export function serializeImageSrcSet() { } // Adding to window so GC won't abort request window.allImgTags = allImgTags; + return allImgTags; } -export default serializeImageSrcSet; +export default loadAllSrcsetLinks; diff --git a/packages/dom/test/serialize-image-srcset.test.js b/packages/dom/test/serialize-image-srcset.test.js new file mode 100644 index 000000000..c128d4523 --- /dev/null +++ b/packages/dom/test/serialize-image-srcset.test.js @@ -0,0 +1,51 @@ +import { withExample, platforms } from './helpers'; + +import { loadAllSrcsetLinks } from '@percy/dom'; + +describe('loadAllSrcsetLinks', () => { + let imgTags; + + platforms.forEach((platform) => { + it(`${platform}: capture url from img srcset`, async () => { + withExample(` + + `); + + imgTags = loadAllSrcsetLinks(); + + expect(imgTags.map(s => s.src)).toEqual(['http://localhost:9876/base/test/assets/example.webp', 'http://localhost:9876/base/test/assets/example.png']); + }); + + it(`${platform}: capture url from source of picture`, async () => { + withExample(` + + + + + `); + + imgTags = loadAllSrcsetLinks(); + + expect(imgTags.map(s => s.src)).toEqual([ + 'http://locahost:9876/base/test/assets/example.webp', + 'http://localhost:9876/base/test/assets/example.png', + 'http://locahost:9876/base/test/assets/example.jpeg', + 'http://locahost:9876/base/test/assets/example1.jpeg' + ]); + }); + + it(`${platform}: srcset inside shadowroot`, () => { + withExample(` + + `, { withShadow: true }); + + imgTags = loadAllSrcsetLinks(); + + expect(imgTags.map(s => s.src)).toEqual([ + 'http://localhost:9876/base/test/assets/example.webp', + 'http://localhost:9876/base/test/assets/example.png', + 'http://localhost:9876/base/test/assets/example1.png' + ]); + }); + }); +}); From 9877cc345242052a0a7306c3790ee283da48c5bd Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Wed, 28 Feb 2024 15:32:01 +0530 Subject: [PATCH 3/5] Fix breaking tests --- packages/core/src/page.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/page.js b/packages/core/src/page.js index 0b8b8db53..ca0db063a 100644 --- a/packages/core/src/page.js +++ b/packages/core/src/page.js @@ -176,7 +176,7 @@ export class Page { // wait for any final network activity before capturing the dom snapshot await this.network.idle(); - this.insertPercyDom(); + await this.insertPercyDom(); // serialize and capture a DOM snapshot this.log.debug('Serialize DOM', this.meta); From a645e18134801a64ff9d59b09d7c3a808a915452 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Thu, 29 Feb 2024 14:08:41 +0530 Subject: [PATCH 4/5] Remove default value captureSrcset --- packages/core/src/config.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/src/config.js b/packages/core/src/config.js index f00012944..d715c4cad 100644 --- a/packages/core/src/config.js +++ b/packages/core/src/config.js @@ -191,8 +191,7 @@ export const configSchema = { default: false }, captureSrcset: { - type: 'boolean', - default: false + type: 'boolean' }, requestHeaders: { type: 'object', From 6b895929c6ab82e1883867e28679d7206eb985f1 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Sun, 3 Mar 2024 01:37:09 +0530 Subject: [PATCH 5/5] Fix edge case --- packages/dom/src/serialize-image-srcset.js | 10 +++++++++- packages/dom/test/serialize-image-srcset.test.js | 10 ++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/dom/src/serialize-image-srcset.js b/packages/dom/src/serialize-image-srcset.js index 68b8ed045..3b863d18a 100644 --- a/packages/dom/src/serialize-image-srcset.js +++ b/packages/dom/src/serialize-image-srcset.js @@ -14,7 +14,15 @@ function getSrcsets(dom) { } function handleSrcSet(srcSet, links) { - srcSet = srcSet.split(/,\s+/); + let pattern = /,\s+/; + + // We found couple of combination of srcset which needs different regex. + // example - https://url.com?param=a,b <--- here only separeting with , will cause incorrect capture. + // srcset = https://abc.com 320w,https://abc.com/a 400 <--- here srcset doesnot have space after comm. + if (!srcSet.match(pattern)) { + pattern = /,/; + } + srcSet = srcSet.split(pattern); for (let src of srcSet) { src = src.trim(); src = src.split(' ')[0]; diff --git a/packages/dom/test/serialize-image-srcset.test.js b/packages/dom/test/serialize-image-srcset.test.js index c128d4523..ab92b1d20 100644 --- a/packages/dom/test/serialize-image-srcset.test.js +++ b/packages/dom/test/serialize-image-srcset.test.js @@ -16,6 +16,16 @@ describe('loadAllSrcsetLinks', () => { expect(imgTags.map(s => s.src)).toEqual(['http://localhost:9876/base/test/assets/example.webp', 'http://localhost:9876/base/test/assets/example.png']); }); + it(`${platform}: capture url from img srcset where there is no space after ,`, async () => { + withExample(` + + `); + + imgTags = loadAllSrcsetLinks(); + + expect(imgTags.map(s => s.src)).toEqual(['http://localhost:9876/base/test/assets/example.webp', 'http://localhost:9876/base/test/assets/example.png']); + }); + it(`${platform}: capture url from source of picture`, async () => { withExample(`