mirror of
https://github.com/block/buzz.git
synced 2026-10-27 04:30:52 +00:00
fix(desktop): repair the Windows notification shim's false denial on read (#5600)
Addresses #2445. Closely related to #2483 -- see "Relationship to #2483" below; I am happy to close this in favour of that one. ## The problem On Windows the notification plugin's injected shim decides its permission at startup like this ([`guest-js/init.ts:12-18`](https://github.com/tauri-apps/plugins-workspace/blob/v2/plugins/notification/guest-js/init.ts)): ```ts if (window.Notification.permission !== 'default' || __TEMPLATE_windows__) { return await Promise.resolve(window.Notification.permission === 'granted') } return await invoke('plugin:notification|is_permission_granted') ``` `__TEMPLATE_windows__` is replaced with the literal `true` on Windows, so the `||` short-circuits and the shim compares its own freshly-initialised `'default'` against `'granted'` -- `false` -- and stamps **`denied`**. The line that would ask the backend is unreachable, and the backend would have said yes: `permission_state()` returns `PermissionState::Granted` unconditionally on desktop. So on every launch of the Windows app, `window.Notification.permission` is `denied` and nothing in the OS is denying anything. ## Why the toggle is only half of it `getDesktopNotificationPermissionState()` trusts that value, so `refreshPermission()` reports `denied` at mount, and this effect in `hooks.ts` runs before the user touches anything: ```ts React.useEffect(() => { if (settings.desktopEnabled && (permission === "denied" || permission === "unsupported")) { setSettings((current) => ({ ...current, desktopEnabled: false })); } }, [permission, settings.desktopEnabled]); ``` It writes `desktopEnabled: false` and persists it. Measured on a real install: with alerts successfully enabled and `desktopEnabled` persisted as `true`, a clean relaunch comes back with it persisted as `false`. That means fixing only the enable path leaves a Windows user re-enabling alerts after every restart. ## What this changes One place: `getDesktopNotificationPermissionState()` repairs the bogus value instead of reporting it. ```ts if ( window.Notification.permission === "denied" && isTauri() && isWindowsPlatform() ) { try { return await requestDesktopNotificationAccess(); } catch { return window.Notification.permission; } } ``` `requestPermission()` is the only call that reaches the backend, and on Windows it raises no prompt, returns `Granted`, and rewrites the shim's cached value -- so this is self-limiting: the next read short-circuits as `granted` and never requests again. There is a test for exactly that. Repairing at the read rather than at the toggle fixes every consumer at once, including `sendDesktopNotification()`, which gates delivery on `=== "granted"` and would otherwise drop notifications at boot. - `denied` stays terminal on macOS, on Linux, and on ordinary Windows web pages. - A denial that survives the request is still reported as `denied`. - A rejected request falls back to the current value instead of propagating; callers do not handle rejections today. - Adds `isWindowsPlatform()` to `shared/lib/platform.ts`, alongside the existing `isMacPlatform()` / `isLinuxPlatform()`. - No new dependencies. No changes to `hooks.ts` or to notification delivery. ## Verification on a real Windows 11 install Buzz Desktop 0.5.9 release build, Windows 11 Home 10.0.26200, WebView2 151.0.4129.72, attached over CDP. Clean boot, nothing injected: ``` window.Notification.permission -> "denied" localStorage[...].desktopEnabled -> false (written by the effect above) await window.Notification.requestPermission() -> "granted" window.Notification.permission -> "granted" new window.Notification("Buzz", { body: ... }) ToastNotificationManager::History .GetHistory("xyz.block.buzz.app") -> 3 -> 4 (real toast delivered) CreateToastNotifier(...).Setting -> Enabled ``` The OS was permissive throughout: no `ToastEnabled` override, no notification policy keys under HKCU or HKLM, and the app present under `HKCU\...\Notifications\Settings\xyz.block.buzz.app`. Full detail in [this comment](https://github.com/block/buzz/pull/2483#issuecomment-5255972424). To be precise about scope: this confirms the mechanism the patch relies on, measured on a stock release build. The patched build itself is covered by the unit tests below, not by a packaged Windows run. ## Tests `pnpm test` -- full desktop suite. New cases in `lib/desktop.test.mjs`: - Windows Tauri repairs the shim's false denied state on read - the repaired state is cached, so later reads do not request again - a denial that survives the request is reported as denied - denied stays terminal outside the Windows Tauri app (Windows web, Linux Tauri, Linux web) - granted is returned untouched and never triggers a request Plus `shared/lib/platform.test.mjs` for `isWindowsPlatform()` across `Win32` / `Win64` / `Windows` and `MacIntel` / `Darwin` / `Linux x86_64`. ## Relationship to #2483 #2483 fixes the same root cause at the toggle: it retries the request inside `setDesktopEnabled`. That makes the toggle work, which is what #2445 reports, and its `ensureDesktopNotificationPermission` helper is a nicer shape than an inline condition. The difference is where the repair happens. Because #2483 leaves the mount-time read as `denied`, the effect quoted above still turns the setting off on every launch, so the fix does not survive a restart. Repairing at the read covers the toggle path too, since `setDesktopEnabled` calls `refreshPermission()` first and finds `granted`. The two overlap: both add `isWindowsPlatform()` to `platform.ts`, so whichever lands second needs a trivial rebase. If maintainers prefer #2483's shape, the equivalent of this PR is to call its helper from `refreshPermission` as well as from `setDesktopEnabled` -- I am glad to close this and send that instead. I did not want to push changes into someone else's open PR uninvited. ## Once this lands #2445 can close. I am running a per-machine launcher workaround for a small Windows team until then, which this makes unnecessary. Signed-off-by: VSCteam <144167621+Joxyko@users.noreply.github.com>
This commit is contained in:
@@ -23,7 +23,55 @@ class ThrowingNotification {
|
||||
|
||||
globalThis.window = { Notification: ThrowingNotification };
|
||||
|
||||
const { sendDesktopNotification } = await import("./desktop.ts");
|
||||
const { sendDesktopNotification, getDesktopNotificationPermissionState } =
|
||||
await import("./desktop.ts");
|
||||
|
||||
// Stands in for the plugin's injected shim: `permission` is a cached value and
|
||||
// only `requestPermission()` reaches the backend and rewrites it.
|
||||
function shimNotification(initialPermission, grantedPermission = "granted") {
|
||||
class ShimNotification {
|
||||
static permission = initialPermission;
|
||||
static requestCount = 0;
|
||||
|
||||
static async requestPermission() {
|
||||
ShimNotification.requestCount += 1;
|
||||
ShimNotification.permission = grantedPermission;
|
||||
return grantedPermission;
|
||||
}
|
||||
|
||||
close() {}
|
||||
}
|
||||
|
||||
return ShimNotification;
|
||||
}
|
||||
|
||||
async function withEnvironment({ platform, isTauri, notification }, callback) {
|
||||
const originalNavigator = Object.getOwnPropertyDescriptor(
|
||||
globalThis,
|
||||
"navigator",
|
||||
);
|
||||
const originalNotification = window.Notification;
|
||||
const originalIsTauri = globalThis.isTauri;
|
||||
|
||||
Object.defineProperty(globalThis, "navigator", {
|
||||
configurable: true,
|
||||
value: { platform, userAgent: "" },
|
||||
});
|
||||
window.Notification = notification;
|
||||
globalThis.isTauri = isTauri;
|
||||
|
||||
try {
|
||||
return await callback();
|
||||
} finally {
|
||||
if (originalNavigator) {
|
||||
Object.defineProperty(globalThis, "navigator", originalNavigator);
|
||||
} else {
|
||||
delete globalThis.navigator;
|
||||
}
|
||||
window.Notification = originalNotification;
|
||||
globalThis.isTauri = originalIsTauri;
|
||||
}
|
||||
}
|
||||
|
||||
test("constructor failure is a delivery miss and does not prevent a later notification", async (t) => {
|
||||
const warnings = [];
|
||||
@@ -50,3 +98,72 @@ test("constructor failure is a delivery miss and does not prevent a later notifi
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
test("Windows Tauri repairs the shim's false denied state on read", async () => {
|
||||
const Notification = shimNotification("denied");
|
||||
|
||||
const permission = await withEnvironment(
|
||||
{ platform: "Win32", isTauri: true, notification: Notification },
|
||||
() => getDesktopNotificationPermissionState(),
|
||||
);
|
||||
|
||||
assert.equal(permission, "granted");
|
||||
assert.equal(Notification.requestCount, 1);
|
||||
});
|
||||
|
||||
test("the repaired state is cached, so later reads do not request again", async () => {
|
||||
const Notification = shimNotification("denied");
|
||||
|
||||
await withEnvironment(
|
||||
{ platform: "Win32", isTauri: true, notification: Notification },
|
||||
async () => {
|
||||
await getDesktopNotificationPermissionState();
|
||||
const second = await getDesktopNotificationPermissionState();
|
||||
assert.equal(second, "granted");
|
||||
},
|
||||
);
|
||||
|
||||
assert.equal(Notification.requestCount, 1);
|
||||
});
|
||||
|
||||
test("a denial that survives the request is reported as denied", async () => {
|
||||
const Notification = shimNotification("denied", "denied");
|
||||
|
||||
const permission = await withEnvironment(
|
||||
{ platform: "Win32", isTauri: true, notification: Notification },
|
||||
() => getDesktopNotificationPermissionState(),
|
||||
);
|
||||
|
||||
assert.equal(permission, "denied");
|
||||
assert.equal(Notification.requestCount, 1);
|
||||
});
|
||||
|
||||
test("denied stays terminal outside the Windows Tauri app", async () => {
|
||||
for (const environment of [
|
||||
{ label: "Windows web", platform: "Win32", isTauri: false },
|
||||
{ label: "Linux Tauri", platform: "Linux x86_64", isTauri: true },
|
||||
{ label: "Linux web", platform: "Linux x86_64", isTauri: false },
|
||||
]) {
|
||||
const Notification = shimNotification("denied");
|
||||
|
||||
const permission = await withEnvironment(
|
||||
{ ...environment, notification: Notification },
|
||||
() => getDesktopNotificationPermissionState(),
|
||||
);
|
||||
|
||||
assert.equal(permission, "denied", environment.label);
|
||||
assert.equal(Notification.requestCount, 0, environment.label);
|
||||
}
|
||||
});
|
||||
|
||||
test("granted is returned untouched and never triggers a request", async () => {
|
||||
const Notification = shimNotification("granted");
|
||||
|
||||
const permission = await withEnvironment(
|
||||
{ platform: "Win32", isTauri: true, notification: Notification },
|
||||
() => getDesktopNotificationPermissionState(),
|
||||
);
|
||||
|
||||
assert.equal(permission, "granted");
|
||||
assert.equal(Notification.requestCount, 0);
|
||||
});
|
||||
|
||||
@@ -6,7 +6,11 @@ import {
|
||||
onAction,
|
||||
requestPermission,
|
||||
} from "@tauri-apps/plugin-notification";
|
||||
import { isLinuxPlatform, isMacPlatform } from "@/shared/lib/platform";
|
||||
import {
|
||||
isLinuxPlatform,
|
||||
isMacPlatform,
|
||||
isWindowsPlatform,
|
||||
} from "@/shared/lib/platform";
|
||||
|
||||
// Backend event emitted when a native Linux notification is clicked or a
|
||||
// queued macOS activation becomes available. See src-tauri notification code.
|
||||
@@ -147,6 +151,25 @@ export async function getDesktopNotificationPermissionState(): Promise<DesktopNo
|
||||
}
|
||||
|
||||
if (window.Notification.permission !== "default") {
|
||||
// On Windows the notification plugin's injected shim stamps `denied` at
|
||||
// startup without ever consulting the backend — `permission_state()`
|
||||
// returns `Granted` unconditionally on desktop — so a `denied` reading
|
||||
// inside the Windows Tauri app is never a real denial. Repair it by
|
||||
// requesting once: on Windows `request_permission()` raises no prompt,
|
||||
// returns `Granted`, and rewrites the shim's cached value, so subsequent
|
||||
// reads short-circuit here as `granted`.
|
||||
if (
|
||||
window.Notification.permission === "denied" &&
|
||||
isTauri() &&
|
||||
isWindowsPlatform()
|
||||
) {
|
||||
try {
|
||||
return await requestDesktopNotificationAccess();
|
||||
} catch {
|
||||
return window.Notification.permission;
|
||||
}
|
||||
}
|
||||
|
||||
return window.Notification.permission;
|
||||
}
|
||||
|
||||
|
||||
38
desktop/src/shared/lib/platform.test.mjs
Normal file
38
desktop/src/shared/lib/platform.test.mjs
Normal file
@@ -0,0 +1,38 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { isWindowsPlatform } from "./platform.ts";
|
||||
|
||||
function withNavigator(navigator, callback) {
|
||||
const original = Object.getOwnPropertyDescriptor(globalThis, "navigator");
|
||||
Object.defineProperty(globalThis, "navigator", {
|
||||
configurable: true,
|
||||
value: navigator,
|
||||
});
|
||||
|
||||
try {
|
||||
callback();
|
||||
} finally {
|
||||
if (original) {
|
||||
Object.defineProperty(globalThis, "navigator", original);
|
||||
} else {
|
||||
delete globalThis.navigator;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
test("Windows detection accepts the platform strings WebView2 reports", () => {
|
||||
for (const platform of ["Win32", "Win64", "Windows"]) {
|
||||
withNavigator({ platform, userAgent: "" }, () => {
|
||||
assert.equal(isWindowsPlatform(), true, platform);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
test("Windows detection rejects the other desktop platforms", () => {
|
||||
for (const platform of ["MacIntel", "Darwin", "Linux x86_64"]) {
|
||||
withNavigator({ platform, userAgent: "" }, () => {
|
||||
assert.equal(isWindowsPlatform(), false, platform);
|
||||
});
|
||||
}
|
||||
});
|
||||
@@ -23,6 +23,15 @@ export function isLinuxPlatform(): boolean {
|
||||
);
|
||||
}
|
||||
|
||||
/** Returns true on Windows desktops. */
|
||||
export function isWindowsPlatform(): boolean {
|
||||
if (typeof navigator === "undefined") {
|
||||
return false;
|
||||
}
|
||||
|
||||
return /^win/i.test(navigator.platform);
|
||||
}
|
||||
|
||||
/**
|
||||
* The platform's normal application-shortcut modifier:
|
||||
* - macOS: Command (Meta)
|
||||
|
||||
Reference in New Issue
Block a user