Skip to content

Commit f3fed11

Browse files
octo-patchocto-patch
octo-patch
and
octo-patch
authored
fix: open side panel synchronously and harden menu actions (#963)
Fix #857. Open the side panel before async tab queries, so Chrome preserves the user gesture required by sidePanel.open(). Also guard missing tab and sidePanel contexts, and observe menu/command action promises to avoid unhandled background rejections. Co-authored-by: Octopus <liyuan851277048@icloud.com> Co-authored-by: octo-patch <octo-patch@github.com>
1 parent 74b0077 commit f3fed11

3 files changed

Lines changed: 164 additions & 32 deletions

File tree

‎src/background/commands.mjs‎

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,58 @@ export function registerCommands() {
1212

1313
if (command in menuConfig) {
1414
if (menuConfig[command].action) {
15-
menuConfig[command].action(true, tab)
15+
// The action may return a Promise (e.g. openSidePanel returns the
16+
// chrome.sidePanel.open() Promise). Keep the call synchronous so the
17+
// user-gesture context is preserved, but observe the Promise so a
18+
// rejection does not become an unhandled rejection in the background.
19+
// Also wrap in try/catch because Browser.commands.onCommand documents
20+
// `tab` as optional, so an action that dereferences tab.* (e.g. the
21+
// openSidePanel call) can throw synchronously.
22+
let result
23+
try {
24+
result = menuConfig[command].action(true, tab)
25+
} catch (error) {
26+
console.error(`failed to run command action "${command}"`, error)
27+
return
28+
}
29+
if (result && typeof result.catch === 'function') {
30+
result.catch((error) => {
31+
console.error(`failed to run command action "${command}"`, error)
32+
})
33+
}
1634
}
1735

1836
if (menuConfig[command].genPrompt) {
19-
const currentTab = (await Browser.tabs.query({ active: true, currentWindow: true }))[0]
20-
Browser.tabs.sendMessage(currentTab.id, {
21-
type: 'CREATE_CHAT',
22-
data: message,
23-
})
37+
// Mirror the pattern in menus.mjs so no step here can leak an
38+
// unhandled rejection in the background:
39+
// - Browser.tabs.query() can reject (permission errors, etc.) —
40+
// observe via try/catch.
41+
// - tabs[0] may be undefined when no active tab exists — guard
42+
// before dereferencing currentTab.id.
43+
// - Browser.tabs.sendMessage() (via webextension-polyfill) rejects
44+
// in normal extension usage (no content script listening,
45+
// restricted pages like chrome://, stale content scripts after
46+
// extension reload) — attach a .catch().
47+
let tabs
48+
try {
49+
tabs = await Browser.tabs.query({ active: true, currentWindow: true })
50+
} catch (error) {
51+
console.error(`failed to query active tab for command "${command}"`, error)
52+
return
53+
}
54+
const currentTab = tabs && tabs[0]
55+
if (!currentTab) {
56+
console.debug(`command "${command}" triggered but no active tab found, skipping`)
57+
return
58+
}
59+
Browser.tabs
60+
.sendMessage(currentTab.id, {
61+
type: 'CREATE_CHAT',
62+
data: message,
63+
})
64+
.catch((error) => {
65+
console.error(`failed to send CREATE_CHAT message for command "${command}"`, error)
66+
})
2467
}
2568
}
2669
})

‎src/background/menus.mjs‎

Lines changed: 96 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,33 +5,107 @@ import { config as menuConfig } from '../content-script/menu-tools/index.mjs'
55

66
const menuId = 'ChatGPTBox-Menu'
77
const onClickMenu = (info, tab) => {
8-
Browser.tabs.query({ active: true, currentWindow: true }).then((tabs) => {
9-
const currentTab = tabs[0]
10-
const message = {
11-
itemId: info.menuItemId.replace(menuId, ''),
12-
selectionText: info.selectionText,
13-
useMenuPosition: tab.id === currentTab.id,
14-
}
15-
console.debug('menu clicked', message)
8+
const itemId = info.menuItemId.replace(menuId, '')
169

17-
if (defaultConfig.selectionTools.includes(message.itemId)) {
18-
Browser.tabs.sendMessage(currentTab.id, {
19-
type: 'CREATE_CHAT',
20-
data: message,
10+
// sidePanel.open() must be called synchronously within the user gesture handler.
11+
// Calling it inside a Promise callback (e.g. Browser.tabs.query().then()) breaks
12+
// Chrome's user gesture requirement and causes the error:
13+
// "sidePanel.open() may only be called in response to a user gesture."
14+
if (itemId === 'openSidePanel' && menuConfig.openSidePanel?.action) {
15+
// Keep the call synchronous to preserve the user-gesture requirement,
16+
// but observe the returned Promise so a rejected sidePanel.open() does
17+
// not become an unhandled rejection in the background script.
18+
// Also wrap in try/catch because contextMenus.onClicked documents `tab`
19+
// as optional ("If the click did not take place in a tab, this parameter
20+
// will be missing"), so the openSidePanel action that dereferences
21+
// tab.windowId/tab.id can throw synchronously.
22+
let result
23+
try {
24+
result = menuConfig.openSidePanel.action(true, tab)
25+
} catch (error) {
26+
console.error('failed to open side panel', error)
27+
return
28+
}
29+
if (result && typeof result.catch === 'function') {
30+
result.catch((error) => {
31+
console.error('failed to open side panel', error)
2132
})
22-
} else if (message.itemId in menuConfig) {
23-
if (menuConfig[message.itemId].action) {
24-
menuConfig[message.itemId].action(true, tab)
33+
}
34+
return
35+
}
36+
37+
Browser.tabs
38+
.query({ active: true, currentWindow: true })
39+
.then((tabs) => {
40+
const currentTab = tabs && tabs[0]
41+
if (!currentTab) {
42+
console.debug('menu clicked but no active tab found, skipping')
43+
return
2544
}
2645

27-
if (menuConfig[message.itemId].genPrompt) {
28-
Browser.tabs.sendMessage(currentTab.id, {
29-
type: 'CREATE_CHAT',
30-
data: message,
31-
})
46+
// contextMenus.onClicked documents `tab` as optional ("If the click did
47+
// not take place in a tab, this parameter will be missing"), so guard
48+
// before dereferencing tab.id when computing useMenuPosition.
49+
const message = {
50+
itemId,
51+
selectionText: info.selectionText,
52+
useMenuPosition: tab ? tab.id === currentTab.id : false,
3253
}
33-
}
34-
})
54+
console.debug('menu clicked', message)
55+
56+
if (defaultConfig.selectionTools.includes(message.itemId)) {
57+
// Browser.tabs.sendMessage() (via webextension-polyfill) returns a
58+
// Promise that commonly rejects (no content script listening, restricted
59+
// pages such as chrome://, stale content scripts after extension reload)
60+
// — observe it so we don't leak unhandled rejections in the background.
61+
Browser.tabs
62+
.sendMessage(currentTab.id, {
63+
type: 'CREATE_CHAT',
64+
data: message,
65+
})
66+
.catch((error) => {
67+
console.error(`failed to send CREATE_CHAT message for "${message.itemId}"`, error)
68+
})
69+
} else if (message.itemId in menuConfig) {
70+
if (menuConfig[message.itemId].action) {
71+
// Several actions in menuConfig are async (e.g. tabs/windows calls)
72+
// and can throw synchronously or return a rejected Promise. Mirror
73+
// the handling already used for openSidePanel above and in
74+
// commands.mjs so neither path leaks an unhandled rejection in the
75+
// background script.
76+
let actionResult
77+
try {
78+
actionResult = menuConfig[message.itemId].action(true, tab)
79+
} catch (error) {
80+
console.error(`failed to run menu action "${message.itemId}"`, error)
81+
}
82+
if (actionResult && typeof actionResult.catch === 'function') {
83+
actionResult.catch((error) => {
84+
console.error(`failed to run menu action "${message.itemId}"`, error)
85+
})
86+
}
87+
}
88+
89+
if (menuConfig[message.itemId].genPrompt) {
90+
// Same rationale as the sendMessage call above — observe the Promise
91+
// so a rejected sendMessage (no content script, restricted page, etc.)
92+
// doesn't surface as an unhandled rejection in the background.
93+
Browser.tabs
94+
.sendMessage(currentTab.id, {
95+
type: 'CREATE_CHAT',
96+
data: message,
97+
})
98+
.catch((error) => {
99+
console.error(`failed to send CREATE_CHAT message for "${message.itemId}"`, error)
100+
})
101+
}
102+
}
103+
})
104+
.catch((error) => {
105+
// Browser.tabs.query() can reject (e.g. on permission errors); make sure
106+
// it does not become an unhandled promise rejection in the background.
107+
console.error('failed to query active tab for menu click', error)
108+
})
35109
}
36110
export function refreshMenu() {
37111
if (Browser.contextMenus.onClicked.hasListener(onClickMenu))

‎src/content-script/menu-tools/index.mjs‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,29 @@ export const config = {
5959
},
6060
openSidePanel: {
6161
label: 'Open Side Panel',
62-
action: async (fromBackground, tab) => {
62+
action: (fromBackground, tab) => {
6363
console.debug('action is from background', fromBackground)
6464
if (fromBackground) {
6565
// eslint-disable-next-line no-undef
66-
chrome.sidePanel.open({ windowId: tab.windowId, tabId: tab.id })
67-
} else {
68-
// side panel is not supported
66+
if (typeof chrome === 'undefined' || !chrome.sidePanel?.open) {
67+
// sidePanel API is not available in this browser (e.g. Firefox)
68+
return Promise.reject(new Error('chrome.sidePanel API is not available'))
69+
}
70+
// contextMenus.onClicked / commands.onCommand document `tab` as
71+
// optional, and even when present the tab may not have an id or
72+
// windowId (e.g. clicks outside a normal browser tab). Guard here so
73+
// callers do not have to wrap every invocation in try/catch just to
74+
// avoid a TypeError from dereferencing tab.windowId / tab.id.
75+
if (!tab || tab.windowId == null || tab.id == null) {
76+
return Promise.reject(
77+
new Error('chrome.sidePanel.open requires a tab with windowId and id'),
78+
)
79+
}
80+
// eslint-disable-next-line no-undef
81+
return chrome.sidePanel.open({ windowId: tab.windowId, tabId: tab.id })
6982
}
83+
// side panel is not supported
84+
return undefined
7085
},
7186
},
7287
closeAllChats: {

0 commit comments

Comments
 (0)