fix: fix pixel ratio mismatch on pages with out of process iframes - #1315
Conversation
8cf7b61 to
c652087
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c652087a91
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7305272432
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| page.documentHeight = Math.ceil(page.documentHeight * scale); | ||
| page.documentWidth = Math.ceil(page.documentWidth * scale); |
There was a problem hiding this comment.
When a fractional preferred DPR falls back to another DPR, these values have already been outward-rounded by Rect.scale and Math.ceil, so scaling and rounding them again cannot recover the actual geometry. For example, a 101px document at DPR 2.625 is stored as 266px and becomes 102px at DPR 1, while a 50px viewport offset becomes 131px and then 49px; in screenshotMode: "auto" the inflated document size can misclassify a real 101px full-page screenshot as a viewport screenshot, and the shifted viewport then crops the wrong region. Fresh evidence after the prior comment is the new fractional-DPR test itself, which now explicitly expects the incorrect 102px document dimension; preserve logical geometry or rerun screenshot preparation at the detected DPR instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 055e1a9624
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const isHeadlessBrowser = chromeOptions => (chromeOptions?.args || []).some(arg => HEADLESS_CHROME_ARG_RE.test(arg)); | ||
|
|
||
| const getEmulatedPixelRatio = chromeOptions => { | ||
| const pixelRatio = _.get(chromeOptions, "mobileEmulation.deviceMetrics.pixelRatio"); |
There was a problem hiding this comment.
When headful Chrome is configured using the supported mobileEmulation: { deviceName: "..." } form, deviceMetrics is absent, so this returns undefined and the code never installs the preferred-DPR retry path. On an OOPIF page exhibiting the mismatch addressed by this change, prepareScreenshot may use the pre-capture DPR while the screenshot is produced after Chrome switches DPR, leaving named-device users with incorrectly scaled and cropped screenshots. Obtain the initial DPR from the browser or otherwise resolve the named profile rather than requiring explicit deviceMetrics.pixelRatio.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ac59f9df50
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (reprepareScreenshot) { | ||
| const currentPixelRatio = await this._browser.evalScript("window.devicePixelRatio"); |
There was a problem hiding this comment.
With screenshotMode: "fullpage", an emulated-DPR mismatch and a horizontally scrolled page, captureViewportImage(page) processes the first screenshot using the stale, preferred-DPR viewport coordinates before this check runs. Camera._cropAreaToViewport treats the image as full-page and can pass an offset beyond the actual lower-DPR image into Image.crop, which throws a RangeError, so execution never reaches evalScript or the retry. Capture the provisional image without the stale page geometry, or determine the current DPR before asking Camera to crop it.
Useful? React with 👍 / 👎.
2 participants
What's done?
Backport to v8 of this PR: #1313