fix(desktop): repair the Windows notification shim's false denial on read - #5600
Open
Joxyko wants to merge 1 commit into
Open
fix(desktop): repair the Windows notification shim's false denial on read#5600Joxyko wants to merge 1 commit into
Joxyko wants to merge 1 commit into
Conversation
…read On Windows the notification plugin's injected shim stamps `denied` at startup without ever consulting the backend, whose `permission_state()` returns `Granted` unconditionally on desktop. Its bootstrap short-circuits on the `__TEMPLATE_windows__` literal and compares its own freshly initialised `'default'` against `'granted'`. `getDesktopNotificationPermissionState()` trusted that value, so the mount-time read reported `denied` and the settings effect turned `desktopEnabled` off and persisted it on every launch -- which is why fixing only the enable path leaves a Windows user re-enabling alerts after each restart. Repair the value at the read instead of reporting it. Inside the Windows Tauri app a `denied` reading is never a real denial, and `requestPermission()` is the only call that reaches the backend and rewrites the shim's cached value. It is self-limiting: the next read short-circuits as `granted` and never requests again. Repairing at the read rather than at the toggle also covers `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, and a denial that survives the request is still reported as `denied`. Refs block#2445 Signed-off-by: VSCteam <144167621+Joxyko@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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):__TEMPLATE_windows__is replaced with the literaltrueon Windows, so the||short-circuits and the shim compares its own freshly-initialised'default'against'granted'--false-- and stampsdenied. The line that would ask the backend is unreachable, and the backend would have said yes:permission_state()returnsPermissionState::Grantedunconditionally on desktop.So on every launch of the Windows app,
window.Notification.permissionisdeniedand nothing in the OS is denying anything.Why the toggle is only half of it
getDesktopNotificationPermissionState()trusts that value, sorefreshPermission()reportsdeniedat mount, and this effect inhooks.tsruns before the user touches anything:It writes
desktopEnabled: falseand persists it. Measured on a real install: with alerts successfully enabled anddesktopEnabledpersisted astrue, a clean relaunch comes back with it persisted asfalse.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.requestPermission()is the only call that reaches the backend, and on Windows it raises no prompt, returnsGranted, and rewrites the shim's cached value -- so this is self-limiting: the next read short-circuits asgrantedand 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.deniedstays terminal on macOS, on Linux, and on ordinary Windows web pages.denied.isWindowsPlatform()toshared/lib/platform.ts, alongside the existingisMacPlatform()/isLinuxPlatform().hooks.tsor 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:
The OS was permissive throughout: no
ToastEnabledoverride, no notification policy keys under HKCU or HKLM, and the app present underHKCU\...\Notifications\Settings\xyz.block.buzz.app. Full detail in this comment.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 inlib/desktop.test.mjs:Plus
shared/lib/platform.test.mjsforisWindowsPlatform()acrossWin32/Win64/WindowsandMacIntel/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 itsensureDesktopNotificationPermissionhelper 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, sincesetDesktopEnabledcallsrefreshPermission()first and findsgranted.The two overlap: both add
isWindowsPlatform()toplatform.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 fromrefreshPermissionas well as fromsetDesktopEnabled-- 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.