Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 07e460f461
ℹ️ 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".
| return; | ||
| } | ||
|
|
||
| const pixelRatio = imageSize.width / page.viewportSizeInCss.width; |
There was a problem hiding this comment.
When pixel-ratio validation runs with screenshotMode: "fullpage" (or auto detects a full-page bitmap), capturedImage.uncroppedSize contains the entire document because Camera.captureViewportImage records it before cropping. Dividing that width by the CSS viewport width therefore interprets document extent as DPR; depending on the document aspect ratio, this either throws at the following check or reparses the page with a grossly inflated ratio, breaking assertView for this supported configuration. The calculation must use dimensions corresponding to the captured image or retain whether the source bitmap was full-page.
Useful? React with 👍 / 👎.
| const pixelRatio = imageSize.width / page.viewportSizeInCss.width; | ||
| if (Math.abs(imageSize.height - page.viewportSizeInCss.height * pixelRatio) > 1) { | ||
| throw new Error("Screenshot dimensions do not match the viewport at a consistent pixel ratio"); |
There was a problem hiding this comment.
When the actual screenshot DPR is fractional and differs from the prepared estimate, bitmap width and height are rounded independently, so deriving an exact ratio from width and allowing only one pixel of height error rejects valid images. For example, the repository's Rect.scale rounding maps a 101×201 CSS viewport at DPR 2.625 to 266×528; this code infers 266 / 101 and predicts a height of about 529.37, causing the valid screenshot to throw instead of triggering re-preparation. Compare compatible DPR intervals from both axes or otherwise include the maximum rounding error induced by the viewport aspect ratio.
Useful? React with 👍 / 👎.
1 participant
Backport to v8 of this PR: #1331