-
-
Notifications
You must be signed in to change notification settings - Fork 641
fix(plugins): restore ChatGPT formula copy in Edge #911
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export const PLUGIN_CONTENT_SCRIPT_SYNC_MESSAGE = 'gv.plugins.syncContentScripts'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ import { | |
| supportsDynamicContentScriptRegistration, | ||
| supportsOptionalHostPermissions, | ||
| } from '@/core/utils/browser'; | ||
| import { PLUGIN_CONTENT_SCRIPT_SYNC_MESSAGE } from '@/features/plugins/runtime/messages'; | ||
| import { pluginToOriginPatternsForActiveUrl } from '@/features/plugins/runtime/siteRegistration'; | ||
| import { SiteRegistry } from '@/features/plugins/sites/registry'; | ||
| import { | ||
|
|
@@ -27,6 +28,26 @@ import { IconChatGPT, IconClaude } from './WebsiteLogos'; | |
| type EnabledMap = Record<string, boolean>; | ||
| type SettingsMap = Record<string, Record<string, PluginSettingValue>>; | ||
|
|
||
| /** | ||
| * Ask the background service to reconcile dynamic plugin content scripts after | ||
| * an optional host permission grant. This is best-effort because Chrome may | ||
| * close the popup while displaying its permission prompt; the background | ||
| * permissions listener remains the fallback in that case. Returns whether the | ||
| * background confirmed that reconciliation completed. | ||
| */ | ||
| async function requestPluginContentScriptSync(): Promise<boolean> { | ||
| try { | ||
| const response = (await browser.runtime.sendMessage({ | ||
| type: PLUGIN_CONTENT_SCRIPT_SYNC_MESSAGE, | ||
| })) as { ok?: unknown } | null; | ||
| return response?.ok === true; | ||
| } catch { | ||
| // Chrome may close the popup while showing the optional-host prompt. The | ||
| // background permissions.onAdded listener remains the fallback in that case. | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** Logo + default accent per known site id. */ | ||
| const SITE_BADGES: Record<string, { Icon: typeof IconClaude; color: string }> = { | ||
| claude: { Icon: IconClaude, color: '#d97757' }, | ||
|
|
@@ -167,6 +188,10 @@ export interface PluginManagerProps { | |
| readonly activeUrl?: string; | ||
| } | ||
|
|
||
| /** | ||
| * Render the popup's plugin catalog and manage each plugin's enabled state, | ||
| * optional host access, platform-specific settings, and refresh lifecycle. | ||
| */ | ||
| export function PluginManager({ | ||
| manifests, | ||
| loading = false, | ||
|
|
@@ -277,14 +302,21 @@ export function PluginManager({ | |
| if (!alreadyGranted) { | ||
| // Chrome closes extension popups while showing an optional-host | ||
| // prompt. Persist the user's intent BEFORE opening it so a | ||
| // successful grant can be completed by the background | ||
| // permissions.onAdded handler without another popup visit. | ||
| // successful grant can be completed by the background even if | ||
| // the popup is closed before permissions.request resolves. | ||
| setEnabledMap((prev) => ({ ...prev, [plugin.id]: true })); | ||
| await setPluginEnabled(plugin.id, true); | ||
| if (!(await browser.permissions.request({ origins }))) { | ||
| const granted = await browser.permissions.request({ origins }); | ||
| if (!granted) { | ||
| setEnabledMap((prev) => ({ ...prev, [plugin.id]: false })); | ||
| await setPluginEnabled(plugin.id, false); | ||
| setDeniedId(plugin.id); | ||
| } else { | ||
| // Edge can resolve the request without reliably delivering the | ||
| // permissions.onAdded event that normally performs registration. | ||
| // Reconcile explicitly while retaining onAdded as Chrome's | ||
| // popup-close fallback. | ||
| await requestPluginContentScriptSync(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In the new-host enable path, AGENTS.md reference: AGENTS.md:L84-L84 Useful? React with 👍 / 👎. |
||
| } | ||
| return; | ||
| } | ||
|
|
@@ -365,6 +397,7 @@ export function PluginManager({ | |
| setDeniedId(plugin.id); | ||
| return; | ||
| } | ||
| if (!(await requestPluginContentScriptSync())) return; | ||
| setMissingPermissionIds((previous) => { | ||
| const next = new Set(previous); | ||
| next.delete(plugin.id); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
chrome.scripting.registerContentScriptsrejects duringdoSyncPluginContentScripts, that function catches/logs and resolves, so this new branch still sends{ ok: true }. In the Edge permission-repair flow the popup removes thepluginGrantRequiredAccessretry as soon as it sees ok, leaving a granted-but-unregistered plugin (Formula Copy still inert) with no retry path; have the sync return a real success/failure before acknowledging.AGENTS.md reference: AGENTS.md:L84-L84
Useful? React with 👍 / 👎.