chore: use pngs by default for screenshots (#797)

1. Use PNG by default.
1. Increase JPG quality from `50` -> `90`.

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.

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 })

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',
       },
     ],