From 2275c3c0c801d42514d6de127b9b1537d20356a9 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Wed, 13 Feb 2019 19:47:14 -0800 Subject: [PATCH] fix(firefox): properly round clip when doing element screenshots (#4001) Do clipping the same way we do it in Chromium. --- experimental/puppeteer-firefox/lib/JSHandle.js | 8 ++++---- experimental/puppeteer-firefox/lib/Page.js | 12 +++++++++++- .../screenshot-element-fractional-offset.png | Bin 0 -> 113 bytes test/puppeteer.spec.js | 4 ++-- test/screenshot.spec.js | 4 ++-- 5 files changed, 19 insertions(+), 9 deletions(-) create mode 100644 test/golden-firefox/screenshot-element-fractional-offset.png diff --git a/experimental/puppeteer-firefox/lib/JSHandle.js b/experimental/puppeteer-firefox/lib/JSHandle.js index 3861e6290d2..fb735eff9f2 100644 --- a/experimental/puppeteer-firefox/lib/JSHandle.js +++ b/experimental/puppeteer-firefox/lib/JSHandle.js @@ -169,10 +169,10 @@ class ElementHandle extends JSHandle { return await this._frame._page.screenshot(Object.assign({}, options, { clip: { - x: Math.round(clip.x), - y: Math.round(clip.y), - width: Math.round(clip.width), - height: Math.round(clip.height), + x: clip.x, + y: clip.y, + width: clip.width, + height: clip.height, }, })); } diff --git a/experimental/puppeteer-firefox/lib/Page.js b/experimental/puppeteer-firefox/lib/Page.js index 034777c7c93..5dbeb3c90d6 100644 --- a/experimental/puppeteer-firefox/lib/Page.js +++ b/experimental/puppeteer-firefox/lib/Page.js @@ -379,12 +379,22 @@ class Page extends EventEmitter { const {data} = await this._session.send('Page.screenshot', { mimeType: getScreenshotMimeType(options), fullPage: options.fullPage, - clip: options.clip, + clip: processClip(options.clip), }); const buffer = options.encoding === 'base64' ? data : Buffer.from(data, 'base64'); if (options.path) await writeFileAsync(options.path, buffer); return buffer; + + function processClip(clip) { + if (!clip) + return undefined; + const x = Math.round(clip.x); + const y = Math.round(clip.y); + const width = Math.round(clip.width + clip.x - x); + const height = Math.round(clip.height + clip.y - y); + return {x, y, width, height}; + } } async evaluate(pageFunction, ...args) { diff --git a/test/golden-firefox/screenshot-element-fractional-offset.png b/test/golden-firefox/screenshot-element-fractional-offset.png new file mode 100644 index 0000000000000000000000000000000000000000..f554b1d62c4ab19291c200519abdf2b7d12ac1f8 GIT binary patch literal 113 zcmeAS@N?(olHy`uVBq!ia0vp^ra&yt!3HE(H?jKyDGN^*$B>BDx91&s84Ltk9RAGS z_wQ9B?_>pr2I-#YOH*8f#e5@8QZJ$~>a*6e8_l=hFd<5IBhU;6Pgg&ebxsLQ0B}kn Ay#N3J literal 0 HcmV?d00001 diff --git a/test/puppeteer.spec.js b/test/puppeteer.spec.js index 6fa08891b23..bbd3e228f31 100644 --- a/test/puppeteer.spec.js +++ b/test/puppeteer.spec.js @@ -23,8 +23,8 @@ const YELLOW_COLOR = '\x1b[33m'; const RESET_COLOR = '\x1b[0m'; module.exports.addTests = ({testRunner, product, puppeteer, Errors, DeviceDescriptors}) => { - const {describe, xdescribe, fdescribe, describe_fails_ffox} = testRunner; - const {it, fit, xit, it_fails_ffox} = testRunner; + const {describe, xdescribe, fdescribe} = testRunner; + const {it, fit, xit} = testRunner; const {beforeAll, beforeEach, afterAll, afterEach} = testRunner; const CHROME = product === 'Chromium'; diff --git a/test/screenshot.spec.js b/test/screenshot.spec.js index 8f28bddc594..a427abaf645 100644 --- a/test/screenshot.spec.js +++ b/test/screenshot.spec.js @@ -15,7 +15,7 @@ */ module.exports.addTests = function({testRunner, expect, product}) { - const {describe, xdescribe, fdescribe, describe_fails_ffox} = testRunner; + const {describe, xdescribe, fdescribe} = testRunner; const {it, fit, xit, it_fails_ffox} = testRunner; const {beforeAll, beforeEach, afterAll, afterEach} = testRunner; @@ -230,7 +230,7 @@ module.exports.addTests = function({testRunner, expect, product}) { const screenshot = await elementHandle.screenshot(); expect(screenshot).toBeGolden('screenshot-element-fractional.png'); }); - it_fails_ffox('should work for an element with an offset', async({page}) => { + it('should work for an element with an offset', async({page}) => { await page.setContent('
'); const elementHandle = await page.$('div'); const screenshot = await elementHandle.screenshot();