Skip to content

Fix bidi screenshot quality=0 being treated as 80%#39744

Merged
yury-s merged 1 commit intomicrosoft:mainfrom
karesansui-u:fix-bidi-quality-zero
Mar 19, 2026
Merged

Fix bidi screenshot quality=0 being treated as 80%#39744
yury-s merged 1 commit intomicrosoft:mainfrom
karesansui-u:fix-bidi-quality-zero

Conversation

@karesansui-u
Copy link
Contributor

Summary

bidiPage.ts uses a truthy check (quality ? quality / 100 : 0.8) to convert the screenshot quality parameter from 0–100 scale to 0–1 scale for the BiDi protocol. When quality === 0, the check fails (0 is falsy) and the value becomes 0.8 (80%) instead of 0.0.

  • Root cause: PR feat: default screenshot jpeg quality to 80 #22966 added quality ?? 80 as the default in screenshotter.ts, but the bidi implementation was not updated to match — it still uses its own 0.8 fallback via a truthy check.
  • Chromium/Firefox: pass quality through directly, so 0 works correctly.
  • Fix: quality !== undefined ? quality / 100 : undefined
    • Correctly handles quality === 0
    • Removes the redundant 0.8 fallback (already handled upstream by ?? 80)
    • Omits quality for PNG screenshots (quality is optional in the BiDi protocol)

Test plan

  • Added test: quality option should work for jpeg — verifies quality: 0 produces a smaller file than quality: 100
  • Existing screenshot tests continue to pass

bidiPage.ts uses a truthy check (quality ? quality / 100 : 0.8) to
convert the screenshot quality parameter. When quality === 0, this
evaluates to 0.8 (80%) instead of 0.0.

Change to quality !== undefined ? quality / 100 : undefined, which
correctly handles zero and omits quality for PNG screenshots.
@github-actions
Copy link
Contributor

Test results for "MCP"

5481 passed, 343 skipped


Merge workflow run.

@github-actions
Copy link
Contributor

Test results for "tests 1"

1 failed
❌ [playwright-test] › reporter-html.spec.ts:267 › merged › should include image diffs for same expectation @macos-latest-node20

5 flaky ⚠️ [chromium-library] › library/trace-viewer.spec.ts:1223 › should display language-specific locators `@ubuntu-22.04-chromium-tip-of-tree`
⚠️ [chromium-library] › library/popup.spec.ts:261 › should not throw when click closes popup `@chromium-ubuntu-22.04-node24`
⚠️ [firefox-library] › library/trace-viewer.spec.ts:1223 › should display language-specific locators `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-page] › page/page-emulate-media.spec.ts:144 › should keep reduced motion and color emulation after reload `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-page] › page/page-wait-for-function.spec.ts:104 › should work with strict CSP policy `@firefox-ubuntu-22.04-node20`

38822 passed, 845 skipped


Merge workflow run.

@yury-s yury-s merged commit d7d8e5b into microsoft:main Mar 19, 2026
38 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants