From 2fe028cb8880347889f99810c4631aae42c7ba9e Mon Sep 17 00:00:00 2001 From: MinhHaDuong Date: Tue, 22 Sep 2026 11:21:48 +0200 Subject: [PATCH 1/3] Prevent stale Add-ons removal from deleting replacement --- app/scripts/fetch_xulrunner | 25 ++++++ .../addon-list-pending-uninstall.test.mjs | 83 +++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 app/scripts/tests/addon-list-pending-uninstall.test.mjs diff --git a/app/scripts/fetch_xulrunner b/app/scripts/fetch_xulrunner index 60cd2f90ec..c8c62f5b27 100755 --- a/app/scripts/fetch_xulrunner +++ b/app/scripts/fetch_xulrunner @@ -460,6 +460,31 @@ function modify_omni { replace_line 'let \{ BrowserAddonUI \} = windowRoot.window;' '' $file replace_line 'await BrowserAddonUI.promptRemoveExtension' 'promptRemoveExtension' $file + file="chrome/toolkit/content/mozapps/extensions/components/addon-list.mjs" + # The Add-ons window can retain the old add-on wrapper after a replacement. + # Finalizing that wrapper's pending uninstall would remove the new copy by ID. + replace_line ' disconnectedCallback\(\) \{' ' async disconnectedCallback() {' $file + remove_line ' this.pendingUninstallAddons.clear\(\);' $file + replace_line ' for \(const addon of this.pendingUninstallAddons\) \{' \ + ' const pendingUninstalls = [...this.pendingUninstallAddons]; + this.pendingUninstallAddons.clear(); + for (const addon of pendingUninstalls) {' $file + replace_line ' if \(isPending\(addon, "uninstall"\)\) \{' \ + ' const current = await AddonManager.getAddonByID(addon.id); + if (this.isConnected) { + return; + } + if (current === addon && isPending(current, "uninstall")) {' $file + replace_line ' onInstalled\(addon\) \{' ' onInstalled(addon) { + if (!isPending(addon, "uninstall")) { + for (const pending of this.pendingUninstallAddons) { + if (pending.id === addon.id) { + this.pendingUninstallAddons.delete(pending); + this.removePendingUninstallBar(pending); + } + } + }' $file + # Customize empty-list message replace_line 'createEmptyListMessage\(\) {' 'createEmptyListMessage() { var p = document.createElement("p"); diff --git a/app/scripts/tests/addon-list-pending-uninstall.test.mjs b/app/scripts/tests/addon-list-pending-uninstall.test.mjs new file mode 100644 index 0000000000..e6cc6b1d0c --- /dev/null +++ b/app/scripts/tests/addon-list-pending-uninstall.test.mjs @@ -0,0 +1,83 @@ +// Run against the addon-list.mjs extracted from the patched Firefox omni.ja: +// node app/scripts/tests/addon-list-pending-uninstall.test.mjs +import { readFileSync } from 'node:fs'; +import { runInNewContext } from 'node:vm'; +import assert from 'node:assert/strict'; +import { test } from 'node:test'; + +const sourcePath = process.argv.at(-1); +if (!sourcePath?.endsWith('addon-list.mjs')) { + throw new Error('Pass the patched addon-list.mjs path as the last argument'); +} +const source = readFileSync(sourcePath, 'utf8') + .replace(/^import \{[\s\S]*?\} from "\.\.\/aboutaddons-utils\.mjs";\s*/mu, '') + .replace('export class AddonList', 'class AddonList') + .replace('customElements.define("addon-list", AddonList);', 'globalThis.AddonList = AddonList;'); + +function createList(currentById) { + const context = { + HTMLElement: class {}, + ChromeUtils: { + importESModule: () => ({ AddonManager: { + getAddonByID: id => typeof currentById === 'function' + ? currentById(id) : Promise.resolve(currentById.get(id)), + } }), + defineESModuleGetters: () => {}, + }, + isPending: addon => addon.pendingUninstall, + }; + runInNewContext(source, context, { filename: sourcePath }); + const list = new context.AddonList(); + list.removeListener = () => {}; + list.updateAddon = () => {}; + list.removePendingUninstallBar = () => {}; + return list; +} + +test('a replacement clears the pending removal and survives list teardown', async () => { + let uninstalls = 0; + const old = { id: 'plugin@test', pendingUninstall: true, uninstall: () => uninstalls++ }; + const replacement = { id: old.id, pendingUninstall: false }; + const list = createList(new Map([[old.id, replacement]])); + list.pendingUninstallAddons.add(old); + list.onInstalled(replacement); + assert.equal(list.pendingUninstallAddons.size, 0); + await list.disconnectedCallback(); + assert.equal(uninstalls, 0); +}); + +test('list teardown checks the live add-on even before the install event arrives', async () => { + let uninstalls = 0; + const old = { id: 'plugin@test', pendingUninstall: true, uninstall: () => uninstalls++ }; + const replacement = { id: old.id, pendingUninstall: false }; + const list = createList(new Map([[old.id, replacement]])); + list.pendingUninstallAddons.add(old); + await list.disconnectedCallback(); + assert.equal(uninstalls, 0); +}); + +test('a genuinely pending removal still finalizes', async () => { + let uninstalls = 0; + const old = { id: 'plugin@test', pendingUninstall: true, uninstall: () => uninstalls++ }; + const list = createList(new Map([[old.id, old]])); + list.pendingUninstallAddons.add(old); + await list.disconnectedCallback(); + assert.equal(uninstalls, 1); +}); + +test('a reconnect during lookup leaves newly queued removals intact', async () => { + let resolveLookup; + let uninstalls = 0; + const old = { id: 'old@test', pendingUninstall: true, uninstall: () => uninstalls++ }; + const fresh = { id: 'fresh@test', pendingUninstall: true }; + const list = createList(() => new Promise(resolve => { resolveLookup = resolve; })); + list.pendingUninstallAddons.add(old); + const teardown = list.disconnectedCallback(); + assert.equal(list.pendingUninstallAddons.size, 0); + list.isConnected = true; + list.pendingUninstallAddons.add(fresh); + resolveLookup(old); + await teardown; + assert.equal(uninstalls, 0); + assert.equal(list.pendingUninstallAddons.has(fresh), true); +}); From c1bf7a4865ad3bd20a80c31d82de7f9a33e7ed4f Mon Sep 17 00:00:00 2001 From: MinhHaDuong Date: Tue, 22 Sep 2026 11:34:32 +0200 Subject: [PATCH 2/3] Preserve pending removals across list reconnects --- app/scripts/fetch_xulrunner | 16 +++++++-- .../addon-list-pending-uninstall.test.mjs | 34 +++++++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/app/scripts/fetch_xulrunner b/app/scripts/fetch_xulrunner index c8c62f5b27..fbc339d199 100755 --- a/app/scripts/fetch_xulrunner +++ b/app/scripts/fetch_xulrunner @@ -468,10 +468,22 @@ function modify_omni { replace_line ' for \(const addon of this.pendingUninstallAddons\) \{' \ ' const pendingUninstalls = [...this.pendingUninstallAddons]; this.pendingUninstallAddons.clear(); - for (const addon of pendingUninstalls) {' $file + for (let i = 0; i < pendingUninstalls.length; i++) { + const addon = pendingUninstalls[i];' $file replace_line ' if \(isPending\(addon, "uninstall"\)\) \{' \ - ' const current = await AddonManager.getAddonByID(addon.id); + ' let current; + try { + current = await AddonManager.getAddonByID(addon.id); + } catch (error) { + console.error(error); + this.pendingUninstallAddons.add(addon); + } if (this.isConnected) { + for (const pending of pendingUninstalls.slice(i)) { + if (isPending(pending, "uninstall")) { + this.pendingUninstallAddons.add(pending); + } + } return; } if (current === addon && isPending(current, "uninstall")) {' $file diff --git a/app/scripts/tests/addon-list-pending-uninstall.test.mjs b/app/scripts/tests/addon-list-pending-uninstall.test.mjs index e6cc6b1d0c..8637d3e8ba 100644 --- a/app/scripts/tests/addon-list-pending-uninstall.test.mjs +++ b/app/scripts/tests/addon-list-pending-uninstall.test.mjs @@ -15,7 +15,9 @@ const source = readFileSync(sourcePath, 'utf8') .replace('customElements.define("addon-list", AddonList);', 'globalThis.AddonList = AddonList;'); function createList(currentById) { + const errors = []; const context = { + console: { error: error => errors.push(error) }, HTMLElement: class {}, ChromeUtils: { importESModule: () => ({ AddonManager: { @@ -31,6 +33,7 @@ function createList(currentById) { list.removeListener = () => {}; list.updateAddon = () => {}; list.removePendingUninstallBar = () => {}; + list.errors = errors; return list; } @@ -81,3 +84,34 @@ test('a reconnect during lookup leaves newly queued removals intact', async () = assert.equal(uninstalls, 0); assert.equal(list.pendingUninstallAddons.has(fresh), true); }); + +test('a reconnect mid-loop preserves the unprocessed pending removal', async () => { + let resolveSecond; + let firstUninstalls = 0; + let secondUninstalls = 0; + const first = { id: 'first@test', pendingUninstall: true, + uninstall: () => firstUninstalls++ }; + const second = { id: 'second@test', pendingUninstall: true, + uninstall: () => secondUninstalls++ }; + const list = createList(id => id === first.id ? Promise.resolve(first) + : new Promise(resolve => { resolveSecond = resolve; })); + list.pendingUninstallAddons.add(first); + list.pendingUninstallAddons.add(second); + const teardown = list.disconnectedCallback(); + await new Promise(resolve => setImmediate(resolve)); + assert.equal(firstUninstalls, 1); + list.isConnected = true; + resolveSecond(second); + await teardown; + assert.equal(secondUninstalls, 0); + assert.equal(list.pendingUninstallAddons.has(second), true); +}); + +test('a failed current-add-on lookup retains the pending removal', async () => { + const old = { id: 'plugin@test', pendingUninstall: true }; + const list = createList(() => Promise.reject(new Error('lookup failed'))); + list.pendingUninstallAddons.add(old); + await list.disconnectedCallback(); + assert.equal(list.pendingUninstallAddons.has(old), true); + assert.equal(list.errors.length, 1); +}); From 66c1f8a89891b93a25b0338d51281ba1bf3e259a Mon Sep 17 00:00:00 2001 From: MinhHaDuong Date: Tue, 22 Sep 2026 11:37:07 +0200 Subject: [PATCH 3/3] Keep pending removals live during asynchronous teardown --- app/scripts/fetch_xulrunner | 22 +++++++------------ .../addon-list-pending-uninstall.test.mjs | 18 ++++++++++++++- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/app/scripts/fetch_xulrunner b/app/scripts/fetch_xulrunner index fbc339d199..ebfb43de85 100755 --- a/app/scripts/fetch_xulrunner +++ b/app/scripts/fetch_xulrunner @@ -464,29 +464,23 @@ function modify_omni { # The Add-ons window can retain the old add-on wrapper after a replacement. # Finalizing that wrapper's pending uninstall would remove the new copy by ID. replace_line ' disconnectedCallback\(\) \{' ' async disconnectedCallback() {' $file - remove_line ' this.pendingUninstallAddons.clear\(\);' $file replace_line ' for \(const addon of this.pendingUninstallAddons\) \{' \ - ' const pendingUninstalls = [...this.pendingUninstallAddons]; - this.pendingUninstallAddons.clear(); - for (let i = 0; i < pendingUninstalls.length; i++) { - const addon = pendingUninstalls[i];' $file + ' for (const addon of [...this.pendingUninstallAddons]) {' $file replace_line ' if \(isPending\(addon, "uninstall"\)\) \{' \ ' let current; try { current = await AddonManager.getAddonByID(addon.id); } catch (error) { console.error(error); - this.pendingUninstallAddons.add(addon); - } - if (this.isConnected) { - for (const pending of pendingUninstalls.slice(i)) { - if (isPending(pending, "uninstall")) { - this.pendingUninstallAddons.add(pending); - } - } return; } - if (current === addon && isPending(current, "uninstall")) {' $file + if (this.isConnected) { + return; + } + if (this.pendingUninstallAddons.has(addon) && current === addon && isPending(current, "uninstall")) {' $file + replace_line ' addon.uninstall\(\);' \ + ' addon.uninstall(); + this.pendingUninstallAddons.delete(addon);' $file replace_line ' onInstalled\(addon\) \{' ' onInstalled(addon) { if (!isPending(addon, "uninstall")) { for (const pending of this.pendingUninstallAddons) { diff --git a/app/scripts/tests/addon-list-pending-uninstall.test.mjs b/app/scripts/tests/addon-list-pending-uninstall.test.mjs index 8637d3e8ba..5241afc889 100644 --- a/app/scripts/tests/addon-list-pending-uninstall.test.mjs +++ b/app/scripts/tests/addon-list-pending-uninstall.test.mjs @@ -76,7 +76,7 @@ test('a reconnect during lookup leaves newly queued removals intact', async () = const list = createList(() => new Promise(resolve => { resolveLookup = resolve; })); list.pendingUninstallAddons.add(old); const teardown = list.disconnectedCallback(); - assert.equal(list.pendingUninstallAddons.size, 0); + assert.equal(list.pendingUninstallAddons.has(old), true); list.isConnected = true; list.pendingUninstallAddons.add(fresh); resolveLookup(old); @@ -85,6 +85,22 @@ test('a reconnect during lookup leaves newly queued removals intact', async () = assert.equal(list.pendingUninstallAddons.has(fresh), true); }); +test('install notification during lookup removes the stale entry before reconnect', async () => { + let resolveLookup; + let uninstalls = 0; + const old = { id: 'plugin@test', pendingUninstall: true, uninstall: () => uninstalls++ }; + const replacement = { id: old.id, pendingUninstall: false }; + const list = createList(() => new Promise(resolve => { resolveLookup = resolve; })); + list.pendingUninstallAddons.add(old); + const teardown = list.disconnectedCallback(); + list.onInstalled(replacement); + list.isConnected = true; + resolveLookup(replacement); + await teardown; + assert.equal(uninstalls, 0); + assert.equal(list.pendingUninstallAddons.has(old), false); +}); + test('a reconnect mid-loop preserves the unprocessed pending removal', async () => { let resolveSecond; let firstUninstalls = 0;