commit 099ce56a644fe786ea963787ea23258f4dc67f64
parent 62ab9e5d620a2ecb6f83fda060198916817bf6ce
Author: Duncan McIntosh <dmcintosh@mozilla.com>
Date: Tue, 7 Oct 2025 20:53:42 +0000
Bug 1990892 - Delete files when the Taskbar Tab is removed even if unpinning fails. r=nrishel
Differential Revision: https://phabricator.services.mozilla.com/D266221
Diffstat:
3 files changed, 97 insertions(+), 32 deletions(-)
diff --git a/browser/components/taskbartabs/TaskbarTabsPin.sys.mjs b/browser/components/taskbartabs/TaskbarTabsPin.sys.mjs
@@ -56,11 +56,20 @@ export const TaskbarTabsPin = {
* @returns {Promise} Resolves once finished.
*/
async unpinTaskbarTab(aTaskbarTab, aRegistry) {
+ let unpinError = null;
+ let removalError = null;
+
try {
lazy.logConsole.info("Unpinning Taskbar Tab from the taskbar.");
let { relativePath } = await generateShortcutInfo(aTaskbarTab);
- lazy.ShellService.unpinShortcutFromTaskbar("Programs", relativePath);
+
+ try {
+ lazy.ShellService.unpinShortcutFromTaskbar("Programs", relativePath);
+ } catch (e) {
+ lazy.logConsole.error(`Failed to unpin shortcut: ${e.message}`);
+ unpinError = e;
+ }
let iconFile = getIconFile(aTaskbarTab);
@@ -76,12 +85,18 @@ export const TaskbarTabsPin = {
}),
IOUtils.remove(iconFile.path),
]);
-
- Glean.webApp.unpin.record({ result: "Success" });
} catch (e) {
- lazy.logConsole.error(`An error occurred while unpinning: ${e.message}`);
- Glean.webApp.unpin.record({ result: e.name ?? "Unknown exception" });
+ lazy.logConsole.error(
+ `An error occurred while uninstalling: ${e.message}`
+ );
+ removalError = e;
}
+
+ const message = e => (e ? (e.name ?? "Unknown exception") : "Success");
+ Glean.webApp.unpin.record({
+ result: message(unpinError),
+ removal_result: message(removalError),
+ });
},
/**
diff --git a/browser/components/taskbartabs/metrics.yaml b/browser/components/taskbartabs/metrics.yaml
@@ -92,13 +92,21 @@ web_app:
description: >
The user unpinned the web app from their taskbar using Firefox.
extra_keys:
+ removal_result:
+ description: >
+ Error message from the cleanup process, i.e. deleting the shortcut
+ and icon, or 'Success' if it succeeds.
+ type: string
result:
- description: Error code from the unpinning process.
+ description: >
+ Error message from the unpinning process, or 'Success' if it succeeds.
type: string
bugs:
- https://bugzilla.mozilla.org/1966476
+ - https://bugzilla.mozilla.org/1990892
data_reviews:
- https://phabricator.services.mozilla.com/D254733
+ - https://phabricator.services.mozilla.com/D266221
notification_emails:
- nrishel@mozilla.com
expires: never
diff --git a/browser/components/taskbartabs/test/browser/browser_taskbarTabs_telemetry.js b/browser/components/taskbartabs/test/browser/browser_taskbarTabs_telemetry.js
@@ -25,11 +25,22 @@ const exposePinResult = () => {
}
};
+// Similarly, we want to fake an error while deleting to check that telemetry
+// reports it.
+let gShortcutDeleteResult = null;
+const exposeDeleteResult = async () => {
+ if (gShortcutDeleteResult !== null) {
+ const error = new Error();
+ error.name = gShortcutDeleteResult;
+ throw error;
+ }
+};
+
const proxyNativeShellService = {
...ShellService.shellService,
createWindowsIcon: sinon.stub().resolves(),
createShortcut: sinon.stub().resolves("dummy_path"),
- deleteShortcut: sinon.stub().resolves("dummy_path"),
+ deleteShortcut: sinon.stub().callsFake(exposeDeleteResult),
pinShortcutToTaskbar: sinon.stub().callsFake(exposePinResult),
unpinShortcutFromTaskbar: sinon.stub().callsFake(exposePinResult),
};
@@ -62,62 +73,93 @@ add_task(async function testInstallAndUninstallMetric() {
is(snapshot.length, 1, "Should have recorded an 'uninstall' event");
});
-add_task(async function testPinAndUnpinMetricSuccess() {
+async function testPinMetricCustom(aPinResult, aPinMessage = null) {
let snapshot;
const taskbarTab = gRegistry.findOrCreateTaskbarTab(PARSED_URL, 0);
Services.fog.testResetFOG();
- gShortcutPinResult = null;
+ gShortcutPinResult = aPinResult;
await TaskbarTabsPin.pinTaskbarTab(taskbarTab, gRegistry);
snapshot = Glean.webApp.pin.testGetValue();
is(snapshot.length, 1, "A single pin event was recorded");
Assert.strictEqual(
snapshot[0].extra.result,
- "Success",
- "The pin event should be successful"
- );
-
- await TaskbarTabsPin.unpinTaskbarTab(taskbarTab, gRegistry);
- snapshot = Glean.webApp.unpin.testGetValue();
- is(snapshot.length, 1, "A single unpin event was recorded");
- Assert.strictEqual(
- snapshot[0].extra.result,
- "Success",
- "The unpin event should be successful"
+ aPinMessage ?? aPinResult ?? "Success",
+ `Should record the pin ${aPinResult ? "exception" : "success"}`
);
gRegistry.removeTaskbarTab(taskbarTab.id);
+}
+
+add_task(async function testPinMetricSuccess() {
+ await testPinMetricCustom(null);
});
-add_task(async function testPinAndUnpinMetricError() {
+add_task(async function testPinMetricFail() {
+ await testPinMetricCustom("Pin fail!");
+});
+
+add_task(async function testPinMetricInvalid() {
+ await testPinMetricCustom(undefined, "Unknown exception");
+});
+
+async function testUnpinMetricCustom(
+ aUnpinResult,
+ aDeleteResult,
+ aUnpinMessage = null,
+ aDeleteMessage = null
+) {
let snapshot;
const taskbarTab = gRegistry.findOrCreateTaskbarTab(PARSED_URL, 0);
Services.fog.testResetFOG();
- gShortcutPinResult = "This test failed!";
+ gShortcutPinResult = aUnpinResult;
+ gShortcutDeleteResult = aDeleteResult;
- await TaskbarTabsPin.pinTaskbarTab(taskbarTab, gRegistry);
- snapshot = Glean.webApp.pin.testGetValue();
- is(snapshot.length, 1, "A single pin event was recorded");
- Assert.strictEqual(
- snapshot[0].extra.result,
- "This test failed!",
- "The pin event shows failure"
- );
+ // We've mocked out so much that calling pinTaskbarTab should be irrelevant.
await TaskbarTabsPin.unpinTaskbarTab(taskbarTab, gRegistry);
snapshot = Glean.webApp.unpin.testGetValue();
is(snapshot.length, 1, "A single unpin event was recorded");
Assert.strictEqual(
snapshot[0].extra.result,
- "This test failed!",
- "The unpin event shows failure"
+ aUnpinMessage ?? aUnpinResult ?? "Success",
+ `Should record the unpin ${aUnpinResult ? "exception" : "success"}`
+ );
+ Assert.strictEqual(
+ snapshot[0].extra.removal_result,
+ aDeleteMessage ?? aDeleteResult ?? "Success",
+ `Should record the deletion ${aDeleteResult ? "exception" : "success"}`
);
gRegistry.removeTaskbarTab(taskbarTab.id);
+}
+
+add_task(async function testPinAndUnpinMetric_UnpinSuccessDeleteSuccess() {
+ await testUnpinMetricCustom(null, null);
+});
+
+add_task(async function testPinAndUnpinMetric_UnpinFailDeleteSuccess() {
+ await testUnpinMetricCustom("Unpin fail!", null);
+});
+
+add_task(async function testPinAndUnpinMetric_UnpinSuccessDeleteFail() {
+ await testUnpinMetricCustom(null, "Deletion fail!");
+});
+
+add_task(async function testPinAndUnpinMetric_UnpinSuccessDeleteFail() {
+ await testUnpinMetricCustom("Unpin fail!", "Deletion fail!");
+});
+
+add_task(async function testPinAndUnpinMetric_UnpinInvalid() {
+ await testUnpinMetricCustom(undefined, null, "Unknown exception", null);
+});
+
+add_task(async function testPinAndUnpinMetric_DeleteInvalid() {
+ await testUnpinMetricCustom(null, undefined, null, "Unknown exception");
});
add_task(async function testActivateWhenWindowOpened() {
