From 5a5bb36d28d00a6e0cbdac4ed04fce5153a2e37a Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Fri, 6 May 2022 18:54:17 -0600 Subject: [PATCH] chore: nuke "fonts" screenshot option (#14004) It was never released since it wasn't working as expected on WebKit WPE. Fixes #12839 --- docs/src/api/class-locatorassertions.md | 2 - docs/src/api/class-pageassertions.md | 2 - docs/src/api/params.md | 8 ---- docs/src/test-api/class-testconfig.md | 1 - docs/src/test-api/class-testproject.md | 1 - .../playwright-core/src/protocol/channels.ts | 6 --- .../playwright-core/src/protocol/protocol.yml | 5 -- .../playwright-core/src/protocol/validator.ts | 3 -- .../src/server/screenshotter.ts | 22 +++------ tests/config/experimental.d.ts | 47 ------------------- tests/page/page-screenshot.spec.ts | 35 -------------- .../to-have-screenshot.spec.ts | 2 - 12 files changed, 6 insertions(+), 128 deletions(-) diff --git a/docs/src/api/class-locatorassertions.md b/docs/src/api/class-locatorassertions.md index 5592ae01c0..5d81b8f4a7 100644 --- a/docs/src/api/class-locatorassertions.md +++ b/docs/src/api/class-locatorassertions.md @@ -1024,8 +1024,6 @@ await expect(locator).toHaveScreenshot(); ### option: LocatorAssertions.toHaveScreenshot.caret = %%-screenshot-option-caret-%% -### option: LocatorAssertions.toHaveScreenshot.fonts = %%-screenshot-option-fonts-%% - ### option: LocatorAssertions.toHaveScreenshot.mask = %%-screenshot-option-mask-%% ### option: LocatorAssertions.toHaveScreenshot.omitBackground = %%-screenshot-option-omit-background-%% diff --git a/docs/src/api/class-pageassertions.md b/docs/src/api/class-pageassertions.md index 4c3c63bb29..10702a3d85 100644 --- a/docs/src/api/class-pageassertions.md +++ b/docs/src/api/class-pageassertions.md @@ -137,8 +137,6 @@ await expect(page).toHaveScreenshot(); ### option: PageAssertions.toHaveScreenshot.clip = %%-screenshot-option-clip-%% -### option: PageAssertions.toHaveScreenshot.fonts = %%-screenshot-option-fonts-%% - ### option: PageAssertions.toHaveScreenshot.fullPage = %%-screenshot-option-full-page-%% ### option: PageAssertions.toHaveScreenshot.mask = %%-screenshot-option-mask-%% diff --git a/docs/src/api/params.md b/docs/src/api/params.md index d29cf842f7..537f39296a 100644 --- a/docs/src/api/params.md +++ b/docs/src/api/params.md @@ -1009,13 +1009,6 @@ An object which specifies clipping of the resulting image. Should have the follo When set to `"css"`, screenshot will have a single pixel per each css pixel on the page. For high-dpi devices, this will keep screenshots small. Using `"device"` option will produce a single pixel per each device pixel, so screenhots of high-dpi devices will be twice as large or even larger. Defaults to `"device"`. -## screenshot-option-fonts -* langs: js -* experimental -- `fonts` <[ScreenshotFonts]<"ready"|"nowait">> - -When set to `"ready"`, screenshot will wait for [`document.fonts.ready`](https://developer.mozilla.org/en-US/docs/Web/API/FontFaceSet/ready) promise to resolve in all frames. Defaults to `"nowait"`. - ## screenshot-option-caret - `caret` <[ScreenshotCaret]<"hide"|"initial">> @@ -1027,7 +1020,6 @@ When set to `"hide"`, screenshot will hide text caret. When set to `"initial"`, - %%-screenshot-option-quality-%% - %%-screenshot-option-path-%% - %%-screenshot-option-scale-%% -- %%-screenshot-option-fonts-%% - %%-screenshot-option-caret-%% - %%-screenshot-option-type-%% - %%-screenshot-option-mask-%% diff --git a/docs/src/test-api/class-testconfig.md b/docs/src/test-api/class-testconfig.md index 956e3792d1..5d5ed38180 100644 --- a/docs/src/test-api/class-testconfig.md +++ b/docs/src/test-api/class-testconfig.md @@ -42,7 +42,6 @@ export default config; - `maxDiffPixelRatio` ?<[float]> an acceptable ratio of pixels that are different to the total amount of pixels, between `0` and `1` , unset by default. - `animations` ?<[ScreenshotAnimations]<"allow"|"disable">> See [`option: animations`] in [`method: Page.screenshot`]. Defaults to `"disable"`. - `caret` ?<[ScreenshotCaret]<"hide"|"initial">> See [`option: caret`] in [`method: Page.screenshot`]. Defaults to `"hide"`. - - `fonts` ?<[ScreenshotFonts]<"ready"|"nowait">> See [`option: fonts`] in [`method: Page.screenshot`]. Defaults to `"ready"`. - `scale` ?<[ScreenshotScale]<"css"|"device">> See [`option: scale`] in [`method: Page.screenshot`]. Defaults to `"css"`. - `toMatchSnapshot` ?<[Object]> Configuration for the [`method: ScreenshotAssertions.toMatchSnapshot#1`] method. - `threshold` ?<[float]> an acceptable perceived color difference in the [YIQ color space](https://en.wikipedia.org/wiki/YIQ) between the same pixel in compared images, between zero (strict) and one (lax). Defaults to `0.2`. diff --git a/docs/src/test-api/class-testproject.md b/docs/src/test-api/class-testproject.md index b3f8dc96d1..0279681da8 100644 --- a/docs/src/test-api/class-testproject.md +++ b/docs/src/test-api/class-testproject.md @@ -113,7 +113,6 @@ export default config; - `maxDiffPixelRatio` ?<[float]> an acceptable ratio of pixels that are different to the total amount of pixels, between `0` and `1` , unset by default. - `animations` ?<[ScreenshotAnimations]<"allow"|"disable">> See [`option: animations`] in [`method: Page.screenshot`]. Defaults to `"disable"`. - `caret` ?<[ScreenshotCaret]<"hide"|"initial">> See [`option: caret`] in [`method: Page.screenshot`]. Defaults to `"hide"`. - - `fonts` ?<[ScreenshotFonts]<"ready"|"nowait">> See [`option: fonts`] in [`method: Page.screenshot`]. Defaults to `"ready"`. - `scale` ?<[ScreenshotScale]<"css"|"device">> See [`option: scale`] in [`method: Page.screenshot`]. Defaults to `"css"`. - `toMatchSnapshot` ?<[Object]> Configuration for the [`method: ScreenshotAssertions.toMatchSnapshot#1`] method. - `threshold` ?<[float]> an acceptable perceived color difference in the [YIQ color space](https://en.wikipedia.org/wiki/YIQ) between the same pixel in compared images, between zero (strict) and one (lax). Defaults to `0.2`. diff --git a/packages/playwright-core/src/protocol/channels.ts b/packages/playwright-core/src/protocol/channels.ts index 6f6c89df4d..0e019c352e 100644 --- a/packages/playwright-core/src/protocol/channels.ts +++ b/packages/playwright-core/src/protocol/channels.ts @@ -1536,7 +1536,6 @@ export type PageExpectScreenshotParams = { caret?: 'hide' | 'initial', animations?: 'disabled' | 'allow', scale?: 'css' | 'device', - fonts?: 'ready' | 'nowait', mask?: { frame: FrameChannel, selector: string, @@ -1562,7 +1561,6 @@ export type PageExpectScreenshotOptions = { caret?: 'hide' | 'initial', animations?: 'disabled' | 'allow', scale?: 'css' | 'device', - fonts?: 'ready' | 'nowait', mask?: { frame: FrameChannel, selector: string, @@ -1586,7 +1584,6 @@ export type PageScreenshotParams = { caret?: 'hide' | 'initial', animations?: 'disabled' | 'allow', scale?: 'css' | 'device', - fonts?: 'ready' | 'nowait', mask?: { frame: FrameChannel, selector: string, @@ -1602,7 +1599,6 @@ export type PageScreenshotOptions = { caret?: 'hide' | 'initial', animations?: 'disabled' | 'allow', scale?: 'css' | 'device', - fonts?: 'ready' | 'nowait', mask?: { frame: FrameChannel, selector: string, @@ -2922,7 +2918,6 @@ export type ElementHandleScreenshotParams = { caret?: 'hide' | 'initial', animations?: 'disabled' | 'allow', scale?: 'css' | 'device', - fonts?: 'ready' | 'nowait', mask?: { frame: FrameChannel, selector: string, @@ -2936,7 +2931,6 @@ export type ElementHandleScreenshotOptions = { caret?: 'hide' | 'initial', animations?: 'disabled' | 'allow', scale?: 'css' | 'device', - fonts?: 'ready' | 'nowait', mask?: { frame: FrameChannel, selector: string, diff --git a/packages/playwright-core/src/protocol/protocol.yml b/packages/playwright-core/src/protocol/protocol.yml index 20d8074083..046de94b06 100644 --- a/packages/playwright-core/src/protocol/protocol.yml +++ b/packages/playwright-core/src/protocol/protocol.yml @@ -328,11 +328,6 @@ CommonScreenshotOptions: literals: - css - device - fonts: - type: enum? - literals: - - ready - - nowait mask: type: array? items: diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index 4a5ab124a0..0e4576fae4 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -569,7 +569,6 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { caret: tOptional(tEnum(['hide', 'initial'])), animations: tOptional(tEnum(['disabled', 'allow'])), scale: tOptional(tEnum(['css', 'device'])), - fonts: tOptional(tEnum(['ready', 'nowait'])), mask: tOptional(tArray(tObject({ frame: tChannel('Frame'), selector: tString, @@ -586,7 +585,6 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { caret: tOptional(tEnum(['hide', 'initial'])), animations: tOptional(tEnum(['disabled', 'allow'])), scale: tOptional(tEnum(['css', 'device'])), - fonts: tOptional(tEnum(['ready', 'nowait'])), mask: tOptional(tArray(tObject({ frame: tChannel('Frame'), selector: tString, @@ -1091,7 +1089,6 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { caret: tOptional(tEnum(['hide', 'initial'])), animations: tOptional(tEnum(['disabled', 'allow'])), scale: tOptional(tEnum(['css', 'device'])), - fonts: tOptional(tEnum(['ready', 'nowait'])), mask: tOptional(tArray(tObject({ frame: tChannel('Frame'), selector: tString, diff --git a/packages/playwright-core/src/server/screenshotter.ts b/packages/playwright-core/src/server/screenshotter.ts index 58792dae8c..eebf4ab8c1 100644 --- a/packages/playwright-core/src/server/screenshotter.ts +++ b/packages/playwright-core/src/server/screenshotter.ts @@ -23,7 +23,7 @@ import type { Frame } from './frames'; import type { ParsedSelector } from './isomorphic/selectorParser'; import type * as types from './types'; import type { Progress } from './progress'; -import { assert, experimentalFeaturesEnabled } from '../utils'; +import { assert } from '../utils'; import { MultiMap } from '../utils/multimap'; declare global { @@ -41,7 +41,6 @@ export type ScreenshotOptions = { fullPage?: boolean, clip?: Rect, scale?: 'css' | 'device', - fonts?: 'ready' | 'nowait', caret?: 'hide' | 'initial', }; @@ -87,7 +86,7 @@ export class Screenshotter { return this._queue.postTask(async () => { progress.log('taking page screenshot'); const { viewportSize } = await this._originalViewportSize(progress); - await this._preparePageForScreenshot(progress, options.caret !== 'initial', options.animations === 'disabled', options.fonts === 'ready'); + await this._preparePageForScreenshot(progress, options.caret !== 'initial', options.animations === 'disabled'); progress.throwIfAborted(); // Avoid restoring after failure - should be done by cleanup. if (options.fullPage) { @@ -116,7 +115,7 @@ export class Screenshotter { progress.log('taking element screenshot'); const { viewportSize } = await this._originalViewportSize(progress); - await this._preparePageForScreenshot(progress, options.caret !== 'initial', options.animations === 'disabled', options.fonts === 'ready'); + await this._preparePageForScreenshot(progress, options.caret !== 'initial', options.animations === 'disabled'); progress.throwIfAborted(); // Do not do extra work. await handle._waitAndScrollIntoViewIfNeeded(progress, true /* waitForVisible */); @@ -140,13 +139,11 @@ export class Screenshotter { }); } - async _preparePageForScreenshot(progress: Progress, hideCaret: boolean, disableAnimations: boolean, waitForFonts: boolean) { + async _preparePageForScreenshot(progress: Progress, hideCaret: boolean, disableAnimations: boolean) { if (disableAnimations) progress.log(' disabled all CSS animations'); - if (waitForFonts) - progress.log(' waiting for fonts to load...'); await Promise.all(this._page.frames().map(async frame => { - await frame.nonStallingEvaluateInExistingContext('(' + (async function(hideCaret: boolean, disableAnimations: boolean, waitForFonts: boolean) { + await frame.nonStallingEvaluateInExistingContext('(' + (async function(hideCaret: boolean, disableAnimations: boolean) { const styleTag = document.createElement('style'); if (hideCaret) { styleTag.textContent = ` @@ -223,12 +220,8 @@ export class Screenshotter { delete window.__cleanupScreenshot; }; - if (waitForFonts) - await document.fonts.ready; - }).toString() + `)(${hideCaret}, ${disableAnimations}, ${waitForFonts})`, false, 'utility').catch(() => {}); + }).toString() + `)(${hideCaret}, ${disableAnimations})`, false, 'utility').catch(() => {}); })); - if (waitForFonts) - progress.log(' fonts in all frames are loaded'); progress.cleanupWhenAborted(() => this._restorePageAfterScreenshot()); } @@ -322,9 +315,6 @@ function trimClipToSize(clip: types.Rect, size: types.Size): types.Rect { } export function validateScreenshotOptions(options: ScreenshotOptions): 'png' | 'jpeg' { - if (options.fonts && !experimentalFeaturesEnabled()) - throw new Error(`To use the experimental option "fonts", set PLAYWRIGHT_EXPERIMENTAL_FEATURES=1 enviroment variable.`); - let format: 'png' | 'jpeg' | null = null; // options.type takes precedence over inferring the type from options.path // because it may be a 0-length file with no extension created beforehand (i.e. as a temp file). diff --git a/tests/config/experimental.d.ts b/tests/config/experimental.d.ts index 8e48c6f8a3..f10f8895d8 100644 --- a/tests/config/experimental.d.ts +++ b/tests/config/experimental.d.ts @@ -8361,13 +8361,6 @@ export interface ElementHandle extends JSHandle { */ caret?: "hide"|"initial"; - /** - * When set to `"ready"`, screenshot will wait for - * [`document.fonts.ready`](https://developer.mozilla.org/en-US/docs/Web/API/FontFaceSet/ready) promise to resolve in all - * frames. Defaults to `"nowait"`. - */ - fonts?: "ready"|"nowait"; - /** * Specify locators that should be masked when the screenshot is taken. Masked elements will be overlayed with a pink box * `#FF00FF` that completely covers its bounding box. @@ -16289,13 +16282,6 @@ export interface LocatorScreenshotOptions { */ caret?: "hide"|"initial"; - /** - * When set to `"ready"`, screenshot will wait for - * [`document.fonts.ready`](https://developer.mozilla.org/en-US/docs/Web/API/FontFaceSet/ready) promise to resolve in all - * frames. Defaults to `"nowait"`. - */ - fonts?: "ready"|"nowait"; - /** * Specify locators that should be masked when the screenshot is taken. Masked elements will be overlayed with a pink box * `#FF00FF` that completely covers its bounding box. @@ -16473,13 +16459,6 @@ export interface PageScreenshotOptions { height: number; }; - /** - * When set to `"ready"`, screenshot will wait for - * [`document.fonts.ready`](https://developer.mozilla.org/en-US/docs/Web/API/FontFaceSet/ready) promise to resolve in all - * frames. Defaults to `"nowait"`. - */ - fonts?: "ready"|"nowait"; - /** * When true, takes a screenshot of the full scrollable page, instead of the currently visible viewport. Defaults to * `false`. @@ -17195,12 +17174,6 @@ interface TestConfig { */ caret?: "hide"|"initial"; - /** - * See `fonts` in [page.screenshot([options])](https://playwright.dev/docs/api/class-page#page-screenshot). Defaults to - * `"ready"`. - */ - fonts?: "ready"|"nowait"; - /** * See `scale` in [page.screenshot([options])](https://playwright.dev/docs/api/class-page#page-screenshot). Defaults to * `"css"`. @@ -20071,13 +20044,6 @@ interface LocatorAssertions { */ caret?: "hide"|"initial"; - /** - * When set to `"ready"`, screenshot will wait for - * [`document.fonts.ready`](https://developer.mozilla.org/en-US/docs/Web/API/FontFaceSet/ready) promise to resolve in all - * frames. Defaults to `"nowait"`. - */ - fonts?: "ready"|"nowait"; - /** * Specify locators that should be masked when the screenshot is taken. Masked elements will be overlayed with a pink box * `#FF00FF` that completely covers its bounding box. @@ -20256,13 +20222,6 @@ interface PageAssertions { height: number; }; - /** - * When set to `"ready"`, screenshot will wait for - * [`document.fonts.ready`](https://developer.mozilla.org/en-US/docs/Web/API/FontFaceSet/ready) promise to resolve in all - * frames. Defaults to `"nowait"`. - */ - fonts?: "ready"|"nowait"; - /** * When true, takes a screenshot of the full scrollable page, instead of the currently visible viewport. Defaults to * `false`. @@ -20579,12 +20538,6 @@ interface TestProject { */ caret?: "hide"|"initial"; - /** - * See `fonts` in [page.screenshot([options])](https://playwright.dev/docs/api/class-page#page-screenshot). Defaults to - * `"ready"`. - */ - fonts?: "ready"|"nowait"; - /** * See `scale` in [page.screenshot([options])](https://playwright.dev/docs/api/class-page#page-screenshot). Defaults to * `"css"`. diff --git a/tests/page/page-screenshot.spec.ts b/tests/page/page-screenshot.spec.ts index 3e5c9d327e..c5237f6fcb 100644 --- a/tests/page/page-screenshot.spec.ts +++ b/tests/page/page-screenshot.spec.ts @@ -767,40 +767,5 @@ it.describe('page screenshot animations', () => { 'onfinish', 'animationend' ]); }); - - it('should respect fonts option', async ({ page, server, isWindows, browserName, isLinux }) => { - it.fixme(browserName === 'webkit' && isLinux, 'https://github.com/microsoft/playwright/issues/12839'); - it.fixme(isWindows, 'This requires a windows-specific test expectations. https://github.com/microsoft/playwright/issues/12707'); - await page.setViewportSize({ width: 500, height: 500 }); - let serverRequest, serverResponse; - // Stall font loading. - server.setRoute('/webfont/iconfont.woff2', (req, res) => { - serverRequest = req; - serverResponse = res; - }); - await page.goto(server.PREFIX + '/webfont/webfont.html', { - waitUntil: 'domcontentloaded', // 'load' will not happen if webfont is pending - }); - // Make sure we can take screenshot. - const noIconsScreenshot = await page.screenshot(); - // Make sure screenshot times out while webfont is stalled. - const error = await page.screenshot({ - fonts: 'ready', - timeout: 200, - }).catch(e => e); - expect(error.message).toContain('waiting for fonts to load...'); - expect(error.message).toContain('Timeout 200ms exceeded'); - const [iconsScreenshot] = await Promise.all([ - page.screenshot({ fonts: 'ready' }), - server.serveFile(serverRequest, serverResponse), - ]); - expect(iconsScreenshot).toMatchSnapshot('screenshot-web-font.png', { - maxDiffPixels: 3, - }); - expect(noIconsScreenshot).not.toMatchSnapshot('screenshot-web-font.png', { - maxDiffPixels: 3, - }); - }); - }); diff --git a/tests/playwright-test/to-have-screenshot.spec.ts b/tests/playwright-test/to-have-screenshot.spec.ts index 5951329a06..b04af58cae 100644 --- a/tests/playwright-test/to-have-screenshot.spec.ts +++ b/tests/playwright-test/to-have-screenshot.spec.ts @@ -372,7 +372,6 @@ test('should compile with different option combinations', async ({ runTSC }) => maxDiffPixels: 10, maxDiffPixelRatio: 0.2, animations: "allow", - fonts: "ready", caret: "hide", scale: "css", }, @@ -392,7 +391,6 @@ test('should compile with different option combinations', async ({ runTSC }) => maxDiffPixelRatio: 0.2, animations: "disabled", omitBackground: true, - fonts: "nowait", caret: "initial", scale: "device", timeout: 1000,