fix: use Page.captureScreenshot for exact-tab screenshot capture - #21
Open
DrDexter6000 wants to merge 2 commits into
Open
fix: use Page.captureScreenshot for exact-tab screenshot capture#21DrDexter6000 wants to merge 2 commits into
DrDexter6000 wants to merge 2 commits into
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
captureVisibleTab(windowId) screenshots whichever tab is visually
active in the window, not the requested tabId. When multiple tabs are
open, this produces wrong-tab screenshots.
Switch to chrome.debugger.sendCommand('Page.captureScreenshot') which
targets a specific tab by ID. Activate and focus the target tab first
to ensure the compositor renders it. Fallback to captureVisibleTab if
debugger is unavailable (e.g. permission denied).
Closes different-ai#15
Copy link
Copy Markdown
There was a problem hiding this comment.
Pull request overview
Updates the extension’s screenshot tool to reliably capture the requested tabId (instead of whichever tab is currently foregrounded) by using the Chrome Debugger Protocol Page.captureScreenshot, with a fallback to chrome.tabs.captureVisibleTab.
Changes:
- Activates/focuses the target tab/window before capturing.
- Captures per-tab screenshots via chrome.debugger.sendCommand("Page.captureScreenshot").
- Falls back to chrome.tabs.captureVisibleTab if debugger capture fails.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
Comment thread
extension/background.js
Outdated
Comment on lines
+1168
to
+1179
| if (!alreadyAttached) { | ||
| await chrome.debugger.attach({ tabId: tab.id }, "1.3") | ||
| } | ||
| const result = await chrome.debugger.sendCommand( | ||
| { tabId: tab.id }, | ||
| "Page.captureScreenshot", | ||
| { format: "png" } | ||
| ) | ||
| if (!alreadyAttached) { | ||
| try { await chrome.debugger.detach({ tabId: tab.id }) } catch {} | ||
| } | ||
| return { tabId: tab.id, content: `data:image/png;base64,${result.data}` } |
Copilot
AI
Apr 12, 2026
Copy link
There was a problem hiding this comment.
If chrome.debugger.attach() succeeds but sendCommand("Page.captureScreenshot") throws, the function falls into the catch/fallback path without detaching—leaving the tab debug-attached indefinitely (and potentially blocking DevTools / later attaches). Track whether this call attached, and ensure detach runs in a finally whenever alreadyAttached is false, even on errors.
Suggested change
| if (!alreadyAttached) { | |
| await chrome.debugger.attach({ tabId: tab.id }, "1.3") | |
| } | |
| const result = await chrome.debugger.sendCommand( | |
| { tabId: tab.id }, | |
| "Page.captureScreenshot", | |
| { format: "png" } | |
| ) | |
| if (!alreadyAttached) { | |
| try { await chrome.debugger.detach({ tabId: tab.id }) } catch {} | |
| } | |
| return { tabId: tab.id, content: `data:image/png;base64,${result.data}` } | |
| let didAttach = false | |
| try { | |
| if (!alreadyAttached) { | |
| await chrome.debugger.attach({ tabId: tab.id }, "1.3") | |
| didAttach = true | |
| } | |
| const result = await chrome.debugger.sendCommand( | |
| { tabId: tab.id }, | |
| "Page.captureScreenshot", | |
| { format: "png" } | |
| ) | |
| return { tabId: tab.id, content: `data:image/png;base64,${result.data}` } | |
| } finally { | |
| if (!alreadyAttached && didAttach) { | |
| try { await chrome.debugger.detach({ tabId: tab.id }) } catch {} | |
| } | |
| } |
Sorry, something went wrong.
Copilot uses AI. Check for mistakes.
Comment thread
extension/background.js
Comment on lines
+1153
to
+1182
|
|
||
| // Activate and focus the target tab so the compositor renders it | ||
| await chrome.tabs.update(tab.id, { active: true, highlighted: true }) | ||
| await chrome.windows.update(tab.windowId, { focused: true }) | ||
| // Let the compositor finish the tab swap and give SPA frameworks a moment | ||
| // to flush any pending DOM paint after navigation | ||
| await new Promise(r => setTimeout(r, 600)) | ||
|
|
||
| // Use debugger API for exact tab-specific capture. | ||
| // captureVisibleTab(windowId) captures whichever tab is visually active, | ||
| // not the requested tabId — producing wrong-tab screenshots when multiple | ||
| // tabs are open. | ||
| try { | ||
| const targets = await chrome.debugger.getTargets() | ||
| const alreadyAttached = targets.some(t => t.tabId === tab.id && t.attached) | ||
| if (!alreadyAttached) { | ||
| await chrome.debugger.attach({ tabId: tab.id }, "1.3") | ||
| } | ||
| const result = await chrome.debugger.sendCommand( | ||
| { tabId: tab.id }, | ||
| "Page.captureScreenshot", | ||
| { format: "png" } | ||
| ) | ||
| if (!alreadyAttached) { | ||
| try { await chrome.debugger.detach({ tabId: tab.id }) } catch {} | ||
| } | ||
| return { tabId: tab.id, content: `data:image/png;base64,${result.data}` } | ||
| } catch { | ||
| // Fallback to standard API if debugger is unavailable | ||
| return { tabId: tab.id, content: await chrome.tabs.captureVisibleTab(tab.windowId, { format: "png" }) } |
Copilot
AI
Apr 12, 2026
Copy link
There was a problem hiding this comment.
This changes user-visible state by activating the requested tab and focusing its window, but it never restores the previously active tab/window. That can leave the user on a different tab after the screenshot tool runs. Consider capturing without stealing focus when possible, or save/restore the prior active tab and focused window in a finally block.
Suggested change
| // Activate and focus the target tab so the compositor renders it | |
| await chrome.tabs.update(tab.id, { active: true, highlighted: true }) | |
| await chrome.windows.update(tab.windowId, { focused: true }) | |
| // Let the compositor finish the tab swap and give SPA frameworks a moment | |
| // to flush any pending DOM paint after navigation | |
| await new Promise(r => setTimeout(r, 600)) | |
| // Use debugger API for exact tab-specific capture. | |
| // captureVisibleTab(windowId) captures whichever tab is visually active, | |
| // not the requested tabId — producing wrong-tab screenshots when multiple | |
| // tabs are open. | |
| try { | |
| const targets = await chrome.debugger.getTargets() | |
| const alreadyAttached = targets.some(t => t.tabId === tab.id && t.attached) | |
| if (!alreadyAttached) { | |
| await chrome.debugger.attach({ tabId: tab.id }, "1.3") | |
| } | |
| const result = await chrome.debugger.sendCommand( | |
| { tabId: tab.id }, | |
| "Page.captureScreenshot", | |
| { format: "png" } | |
| ) | |
| if (!alreadyAttached) { | |
| try { await chrome.debugger.detach({ tabId: tab.id }) } catch {} | |
| } | |
| return { tabId: tab.id, content: `data:image/png;base64,${result.data}` } | |
| } catch { | |
| // Fallback to standard API if debugger is unavailable | |
| return { tabId: tab.id, content: await chrome.tabs.captureVisibleTab(tab.windowId, { format: "png" }) } | |
| const previouslyFocusedWindow = await chrome.windows.getLastFocused() | |
| const [previouslyActiveTab] = await chrome.tabs.query({ active: true, windowId: tab.windowId }) | |
| try { | |
| // Activate and focus the target tab so the compositor renders it | |
| await chrome.tabs.update(tab.id, { active: true, highlighted: true }) | |
| await chrome.windows.update(tab.windowId, { focused: true }) | |
| // Let the compositor finish the tab swap and give SPA frameworks a moment | |
| // to flush any pending DOM paint after navigation | |
| await new Promise(r => setTimeout(r, 600)) | |
| // Use debugger API for exact tab-specific capture. | |
| // captureVisibleTab(windowId) captures whichever tab is visually active, | |
| // not the requested tabId — producing wrong-tab screenshots when multiple | |
| // tabs are open. | |
| try { | |
| const targets = await chrome.debugger.getTargets() | |
| const alreadyAttached = targets.some(t => t.tabId === tab.id && t.attached) | |
| if (!alreadyAttached) { | |
| await chrome.debugger.attach({ tabId: tab.id }, "1.3") | |
| } | |
| const result = await chrome.debugger.sendCommand( | |
| { tabId: tab.id }, | |
| "Page.captureScreenshot", | |
| { format: "png" } | |
| ) | |
| if (!alreadyAttached) { | |
| try { await chrome.debugger.detach({ tabId: tab.id }) } catch {} | |
| } | |
| return { tabId: tab.id, content: `data:image/png;base64,${result.data}` } | |
| } catch { | |
| // Fallback to standard API if debugger is unavailable | |
| return { tabId: tab.id, content: await chrome.tabs.captureVisibleTab(tab.windowId, { format: "png" }) } | |
| } | |
| } finally { | |
| if (previouslyActiveTab?.id !== undefined && previouslyActiveTab.id !== tab.id) { | |
| try { | |
| await chrome.tabs.update(previouslyActiveTab.id, { active: true }) | |
| } catch {} | |
| } | |
| if (previouslyFocusedWindow?.id !== undefined && previouslyFocusedWindow.id !== tab.windowId) { | |
| try { | |
| await chrome.windows.update(previouslyFocusedWindow.id, { focused: true }) | |
| } catch {} | |
| } |
Sorry, something went wrong.
Copilot uses AI. Check for mistakes.
Comment thread
extension/background.js
Outdated
Comment on lines
+1157
to
+1159
| // Let the compositor finish the tab swap and give SPA frameworks a moment | ||
| // to flush any pending DOM paint after navigation | ||
| await new Promise(r => setTimeout(r, 600)) |
Copilot
AI
Apr 12, 2026
Copy link
There was a problem hiding this comment.
The 600ms delay is a hard-coded magic number. Please extract this to a named constant (and ideally document why that value is needed), or replace with a more deterministic wait (e.g., wait for activation/update completion) so timing is easier to tune and reason about.
Sorry, something went wrong.
Copilot uses AI. Check for mistakes.
Comment thread
extension/background.js
Outdated
Comment on lines
+1177
to
+1180
| try { await chrome.debugger.detach({ tabId: tab.id }) } catch {} | ||
| } | ||
| return { tabId: tab.id, content: `data:image/png;base64,${result.data}` } | ||
| } catch { |
Copilot
AI
Apr 12, 2026
Copy link
There was a problem hiding this comment.
The catch {} here swallows the underlying failure reason (permission error, protocol error, another debugger attached, etc.), making troubleshooting hard. Consider capturing the error and logging a warning (consistent with ensureDebuggerAttached) before falling back.
Suggested change
| try { await chrome.debugger.detach({ tabId: tab.id }) } catch {} | |
| } | |
| return { tabId: tab.id, content: `data:image/png;base64,${result.data}` } | |
| } catch { | |
| try { | |
| await chrome.debugger.detach({ tabId: tab.id }) | |
| } catch (error) { | |
| console.warn("Failed to detach debugger after screenshot capture", error) | |
| } | |
| } | |
| return { tabId: tab.id, content: `data:image/png;base64,${result.data}` } | |
| } catch (error) { | |
| console.warn("Debugger screenshot capture failed; falling back to captureVisibleTab", error) |
Sorry, something went wrong.
Copilot uses AI. Check for mistakes.
- Use try/finally for debugger detach so it runs even if Page.captureScreenshot throws (was a resource leak) - Save and restore the previously active tab + focused window after capturing so the user isn't left on the wrong tab - Add console.warn in catch blocks instead of silently swallowing errors — makes permission/protocol issues debuggable - Extract the 600ms compositor delay into a named constant
Copy link
Copy Markdown
Author
|
Good catches, thanks. Pushed a follow-up commit that addresses all four points:
|
Sorry, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I was testing the screenshot tool with multiple tabs open and kept getting screenshots of the wrong tab — captureVisibleTab(windowId) captures whichever tab is visually foregrounded, not the tabId you asked for.
Before: toolScreenshot calls chrome.tabs.captureVisibleTab(tab.windowId) — this screenshots whatever the user happens to be looking at, not the tab the agent requested.
After: Activates and focuses the target tab, waits 600ms for the compositor, then uses chrome.debugger.sendCommand("Page.captureScreenshot") which targets a specific tabId. Falls back to captureVisibleTab if the debugger is unavailable.
The extension already has debugger infrastructure (ensureDebuggerAttached, debuggerState) used by toolConsole and toolErrors. This PR uses a lighter-touch debugger attach (check-attach-capture-detach) since screenshots don't need the persistent Runtime.enable session that console/errors require.
Tested on Windows 11 with Chrome + OpenCode Desktop — multiple tabs with different content now produce correct per-tab screenshots.
Closes #15