Add screenshot support#6
Conversation
Implements the `browser-cli screenshot <session-id>` command with `--output`, `--full-page`, and `--quality` options. The extension uses `chrome.tabs.captureVisibleTab()` to capture the viewport and returns base64-encoded image data. The CLI decodes and writes the image to disk. Full-page capture is accepted but prints a warning as it is not yet implemented. Closes #4
…ed parameter type Use `Parameters<typeof chrome.tabs.captureVisibleTab>[1]` instead of the non-existent `chrome.tabs.CaptureVisibleTabOptions` type.
|
@codex review this pr |
![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: 984dfd5daa
ℹ️ 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".
| } | ||
|
|
||
| // Ensure the tab is active in its window before capturing | ||
| await chrome.tabs.update(session.value.tab_id, { active: true }); |
There was a problem hiding this comment.
handleScreenshot force-activates the target tab (chrome.tabs.update(..., { active: true })) but never restores the previously active tab in that window. When a screenshot is taken for a background session, this permanently steals focus and changes browser state for the user, which is an unexpected side effect of a read-only command and can disrupt ongoing work in other tabs.
Useful? React with 👍 / 👎.
| /// Image quality for JPEG (0-100, default: PNG format) | ||
| #[arg(long)] | ||
| quality: Option<u32>, |
There was a problem hiding this comment.
The CLI help text documents --quality as 0-100, but the argument is parsed as unconstrained Option<u32>, so values like --quality 500 are accepted and forwarded. That creates a contract mismatch and pushes invalid/undefined quality handling into the extension/API layer; adding a clap range constraint here would make behavior deterministic and fail fast with a clear error.
Useful? React with 👍 / 👎.
1 participant
Summary
browser-cli screenshot <session-id>command with--output,--full-page,--quality, and--jsonoptionschrome.tabs.captureVisibleTab()and returns base64-encoded image datascreenshot-<timestamp>.png)base64 = "0.22"dependency,activeTabpermission in manifest, andscreenshotaction constant in the protocolNotes
--full-pageis accepted but prints a warning and falls back to viewport capture (full-page stitching can be added later)--qualityis specified, the image format switches from PNG to JPEGTest plan
browser-cli screenshot <session-id>and verify a PNG file is saved--output custom.pngand verify the custom path is used--quality 80and verify a JPEG file is produced--jsonand verify structured JSON output--full-pageand verify the warning is printedCloses #4