From 2a86ac74e31fe16bfd353e0e1fea95fd9a373302 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Thu, 31 Jul 2025 11:03:19 +0200 Subject: [PATCH] chore: use pngs by default for screenshots (#797) 1. Use PNG by default. 1. Increase JPG quality from `50` -> `90`. --- README.md | 2 +- src/tools/screenshot.ts | 6 ++-- tests/screenshot.spec.ts | 76 +++++++++++++++++++++++++++++----------- 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index de48c2d..5e16e69 100644 --- a/README.md +++ b/README.md @@ -544,7 +544,7 @@ http.createServer(async (req, res) => { - Title: Take a screenshot - Description: Take a screenshot of the current page. You can't perform actions based on the screenshot, use browser_snapshot for actions. - Parameters: - - `raw` (boolean, optional): Whether to return without compression (in PNG format). Default is false, which returns a JPEG image. + - `type` (string, optional): Image format for the screenshot. Default is png. - `filename` (string, optional): File name to save the screenshot to. Defaults to `page-{timestamp}.{png|jpeg}` if not specified. - `element` (string, optional): Human-readable element description used to obtain permission to screenshot the element. If not provided, the screenshot will be taken of viewport. If element is provided, ref must be provided too. - `ref` (string, optional): Exact target element reference from the page snapshot. If not provided, the screenshot will be taken of viewport. If ref is provided, element must be provided too. diff --git a/src/tools/screenshot.ts b/src/tools/screenshot.ts index 7df12db..55a8107 100644 --- a/src/tools/screenshot.ts +++ b/src/tools/screenshot.ts @@ -24,7 +24,7 @@ import { generateLocator } from './utils.js'; import type * as playwright from 'playwright'; const screenshotSchema = z.object({ - raw: z.boolean().optional().describe('Whether to return without compression (in PNG format). Default is false, which returns a JPEG image.'), + type: z.enum(['png', 'jpeg']).default('png').describe('Image format for the screenshot. Default is png.'), filename: z.string().optional().describe('File name to save the screenshot to. Defaults to `page-{timestamp}.{png|jpeg}` if not specified.'), element: z.string().optional().describe('Human-readable element description used to obtain permission to screenshot the element. If not provided, the screenshot will be taken of viewport. If element is provided, ref must be provided too.'), ref: z.string().optional().describe('Exact target element reference from the page snapshot. If not provided, the screenshot will be taken of viewport. If ref is provided, element must be provided too.'), @@ -52,11 +52,11 @@ const screenshot = defineTabTool({ }, handle: async (tab, params, response) => { - const fileType = params.raw ? 'png' : 'jpeg'; + const fileType = params.type || 'png'; const fileName = await outputFile(tab.context.config, params.filename ?? `page-${new Date().toISOString()}.${fileType}`); const options: playwright.PageScreenshotOptions = { type: fileType, - quality: fileType === 'png' ? undefined : 50, + quality: fileType === 'png' ? undefined : 90, scale: 'css', path: fileName, ...(params.fullPage !== undefined && { fullPage: params.fullPage }) diff --git a/tests/screenshot.spec.ts b/tests/screenshot.spec.ts index f062f1c..cce0c3b 100644 --- a/tests/screenshot.spec.ts +++ b/tests/screenshot.spec.ts @@ -35,7 +35,7 @@ test('browser_take_screenshot (viewport)', async ({ startClient, server }, testI code: expect.stringContaining(`await page.screenshot`), attachments: [{ data: expect.any(String), - mimeType: 'image/jpeg', + mimeType: 'image/png', type: 'image', }], }); @@ -66,7 +66,7 @@ test('browser_take_screenshot (element)', async ({ startClient, server }, testIn }, { data: expect.any(String), - mimeType: 'image/jpeg', + mimeType: 'image/png', type: 'image', }, ], @@ -90,15 +90,14 @@ test('--output-dir should work', async ({ startClient, server }, testInfo) => { }); expect(fs.existsSync(outputDir)).toBeTruthy(); - const files = [...fs.readdirSync(outputDir)].filter(f => f.endsWith('.jpeg')); + const files = [...fs.readdirSync(outputDir)].filter(f => f.endsWith('.png')); expect(files).toHaveLength(1); - expect(files[0]).toMatch(/^page-\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}-\d{3}Z\.jpeg$/); + expect(files[0]).toMatch(/^page-\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}-\d{3}Z\.png$/); }); -for (const raw of [undefined, true]) { - test(`browser_take_screenshot (raw: ${raw})`, async ({ startClient, server }, testInfo) => { +for (const type of ['png', 'jpeg']) { + test(`browser_take_screenshot (type: ${type})`, async ({ startClient, server }, testInfo) => { const outputDir = testInfo.outputPath('output'); - const ext = raw ? 'png' : 'jpeg'; const { client } = await startClient({ config: { outputDir }, }); @@ -111,35 +110,72 @@ for (const raw of [undefined, true]) { expect(await client.callTool({ name: 'browser_take_screenshot', - arguments: { raw }, + arguments: { type }, })).toEqual({ content: [ { text: expect.stringMatching( - new RegExp(`page-\\d{4}-\\d{2}-\\d{2}T\\d{2}-\\d{2}-\\d{2}\\-\\d{3}Z\\.${ext}`) + new RegExp(`page-\\d{4}-\\d{2}-\\d{2}T\\d{2}-\\d{2}-\\d{2}\\-\\d{3}Z\\.${type}`) ), type: 'text', }, { data: expect.any(String), - mimeType: `image/${ext}`, + mimeType: `image/${type}`, type: 'image', }, ], }); - const files = [...fs.readdirSync(outputDir)].filter(f => f.endsWith(`.${ext}`)); + const files = [...fs.readdirSync(outputDir)].filter(f => f.endsWith(`.${type}`)); expect(fs.existsSync(outputDir)).toBeTruthy(); expect(files).toHaveLength(1); expect(files[0]).toMatch( - new RegExp(`^page-\\d{4}-\\d{2}-\\d{2}T\\d{2}-\\d{2}-\\d{2}-\\d{3}Z\\.${ext}$`) + new RegExp(`^page-\\d{4}-\\d{2}-\\d{2}T\\d{2}-\\d{2}-\\d{2}-\\d{3}Z\\.${type}$`) ); }); } -test('browser_take_screenshot (filename: "output.jpeg")', async ({ startClient, server }, testInfo) => { +test('browser_take_screenshot (default type should be png)', async ({ startClient, server }, testInfo) => { + const outputDir = testInfo.outputPath('output'); + const { client } = await startClient({ + config: { outputDir }, + }); + expect(await client.callTool({ + name: 'browser_navigate', + arguments: { url: server.PREFIX }, + })).toHaveResponse({ + code: `await page.goto('${server.PREFIX}');`, + }); + + expect(await client.callTool({ + name: 'browser_take_screenshot', + })).toEqual({ + content: [ + { + text: expect.stringMatching( + new RegExp(`page-\\d{4}-\\d{2}-\\d{2}T\\d{2}-\\d{2}-\\d{2}\\-\\d{3}Z\\.png`) + ), + type: 'text', + }, + { + data: expect.any(String), + mimeType: 'image/png', + type: 'image', + }, + ], + }); + + const files = [...fs.readdirSync(outputDir)].filter(f => f.endsWith('.png')); + + expect(fs.existsSync(outputDir)).toBeTruthy(); + expect(files).toHaveLength(1); + expect(files[0]).toMatch(/^page-\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}-\d{3}Z\.png$/); +}); + +test('browser_take_screenshot (filename: "output.png")', async ({ startClient, server }, testInfo) => { const outputDir = testInfo.outputPath('output'); const { client } = await startClient({ config: { outputDir }, @@ -154,27 +190,27 @@ test('browser_take_screenshot (filename: "output.jpeg")', async ({ startClient, expect(await client.callTool({ name: 'browser_take_screenshot', arguments: { - filename: 'output.jpeg', + filename: 'output.png', }, })).toEqual({ content: [ { - text: expect.stringContaining(`output.jpeg`), + text: expect.stringContaining(`output.png`), type: 'text', }, { data: expect.any(String), - mimeType: 'image/jpeg', + mimeType: 'image/png', type: 'image', }, ], }); - const files = [...fs.readdirSync(outputDir)].filter(f => f.endsWith('.jpeg')); + const files = [...fs.readdirSync(outputDir)].filter(f => f.endsWith('.png')); expect(fs.existsSync(outputDir)).toBeTruthy(); expect(files).toHaveLength(1); - expect(files[0]).toMatch(/^output\.jpeg$/); + expect(files[0]).toMatch(/^output\.png$/); }); test('browser_take_screenshot (imageResponses=omit)', async ({ startClient, server }, testInfo) => { @@ -231,7 +267,7 @@ test('browser_take_screenshot (fullPage: true)', async ({ startClient, server }, }, { data: expect.any(String), - mimeType: 'image/jpeg', + mimeType: 'image/png', type: 'image', }, ], @@ -285,7 +321,7 @@ test('browser_take_screenshot (viewport without snapshot)', async ({ startClient }, { data: expect.any(String), - mimeType: 'image/jpeg', + mimeType: 'image/png', type: 'image', }, ],