Skip to content

fix: use Page.captureScreenshot for exact-tab screenshot capture #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DrDexter6000 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: use Page.captureScreenshot for exact-tab screenshot capture #21

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b1e3c45
fix: use Page.captureScreenshot for exact-tab screenshot capture
DrDexter6000 Apr 12, 2026
4b85a79
fix: address review feedback — detach safety, focus restore, logging
DrDexter6000 Apr 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions extension/background.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1150,8 +1150,37 @@ async function toolSelect({ selector, value, label, optionIndex, tabId, index =

async function toolScreenshot({ tabId }) {
const tab = await getTabById(tabId)
const png = await chrome.tabs.captureVisibleTab(tab.windowId, { format: "png" })
return { tabId: tab.id, content: png }

// 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))
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

// 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}` }
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {}
}
}

Copilot uses AI. Check for mistakes.
} catch {
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copilot uses AI. Check for mistakes.
// Fallback to standard API if debugger is unavailable
return { tabId: tab.id, content: await chrome.tabs.captureVisibleTab(tab.windowId, { format: "png" }) }
Comment on lines +1153 to +1196
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {}
}

Copilot uses AI. Check for mistakes.
}
}

async function toolSnapshot({ tabId }) {
Expand Down
Loading