chore: nuke "fonts" screenshot option (#14004)

It was never released since it wasn't working as expected on WebKit WPE.

Fixes #12839
This commit is contained in:
Andrey Lushnikov 2022-05-06 18:54:17 -06:00 committed by GitHub
parent dc36b0158a
commit 5a5bb36d28
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 6 additions and 128 deletions

View file

@ -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-%%

View file

@ -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-%%

View file

@ -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-%%

View file

@ -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`.

View file

@ -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`.

View file

@ -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,

View file

@ -328,11 +328,6 @@ CommonScreenshotOptions:
literals:
- css
- device
fonts:
type: enum?
literals:
- ready
- nowait
mask:
type: array?
items:

View file

@ -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,

View file

@ -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).

View file

@ -8361,13 +8361,6 @@ export interface ElementHandle<T=Node> extends JSHandle<T> {
*/
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"`.

View file

@ -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,
});
});
});

View file

@ -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,