From c9a2398f83b1541845678c79bb9f882dcdfdd3af Mon Sep 17 00:00:00 2001 From: Grace Kind Date: Sun, 16 Aug 2026 00:58:43 -0500 Subject: [PATCH] Move permission request to button press --- src/js/push/courierPushService.js | 4 -- src/js/views/settings/notifications.view.js | 17 ++++- tests/e2e/mockServer.js | 9 +++ .../views/settingsNotifications.view.test.js | 70 +++++++++++++++++++ 4 files changed, 95 insertions(+), 5 deletions(-) diff --git a/src/js/push/courierPushService.js b/src/js/push/courierPushService.js index b34d07bb..c41bc80e 100644 --- a/src/js/push/courierPushService.js +++ b/src/js/push/courierPushService.js @@ -232,10 +232,6 @@ export class CourierPushService { } async _subscribeAndRegister(config) { - const permission = await Notification.requestPermission(); - if (permission !== "granted") { - throw new Error(permission === "denied" ? "denied" : "dismissed"); - } const registration = await navigator.serviceWorker.register(SW_PATH); await navigator.serviceWorker.ready; let subscription = await registration.pushManager.getSubscription(); diff --git a/src/js/views/settings/notifications.view.js b/src/js/views/settings/notifications.view.js index 2a8d1661..4b733e00 100644 --- a/src/js/views/settings/notifications.view.js +++ b/src/js/views/settings/notifications.view.js @@ -75,11 +75,26 @@ export default async function settingsNotificationsView({ } return; } + let permission = null; const confirmed = await confirmModal( "You'll be sent to the notification service to authorize a separate, read-only grant for delivering push notifications. You can turn this off again at any time.", - { title: "Enable push notifications?", confirmButtonText: "Continue" }, + { + title: "Enable push notifications?", + confirmButtonText: "Continue", + onConfirm: () => { + permission = Notification.requestPermission(); + return permission; + }, + }, ); if (!confirmed) return; + if ((await permission) !== "granted") { + showToast( + "Notifications are blocked for this site. Re-enable them in your browser's site settings.", + { style: "error" }, + ); + return; + } try { await courierPushService.startEnableFlow({ chatPreviews: state.$chatPreviews.get(), diff --git a/tests/e2e/mockServer.js b/tests/e2e/mockServer.js index 9e9c6037..914a1124 100644 --- a/tests/e2e/mockServer.js +++ b/tests/e2e/mockServer.js @@ -726,6 +726,15 @@ export class MockServer { }); }, ); + // The service's auth handoff. Real courier would run OAuth and redirect + // back; this just stands still so tests can assert what was sent to it. + await page.route(`${notificationService.authUrl}*`, (route) => + route.fulfill({ + status: 200, + contentType: "text/html", + body: "Authorize", + }), + ); await page.route( `${notificationService.endpoint}/.well-known/notif-service.json`, (route) => diff --git a/tests/e2e/specs/views/settingsNotifications.view.test.js b/tests/e2e/specs/views/settingsNotifications.view.test.js index 8067f290..4ee932ca 100644 --- a/tests/e2e/specs/views/settingsNotifications.view.test.js +++ b/tests/e2e/specs/views/settingsNotifications.view.test.js @@ -26,6 +26,14 @@ async function stubNotificationPermission( constructor() {} close() {} } + // Records when permission was asked for, so tests can assert it happened + // before the handoff navigation rather than after it. + window.__permissionRequests = 0; + const request = MockNotification.requestPermission.bind(MockNotification); + MockNotification.requestPermission = async () => { + window.__permissionRequests += 1; + return request(); + }; window.Notification = MockNotification; }, { initialPermission: initial, promptResult: onPrompt }, @@ -222,6 +230,68 @@ test.describe("Settings > Notifications view", () => { ).toBeVisible(); }); + test("asks for permission before leaving for the handoff", async ({ + page, + }) => { + const mockServer = new MockServer(); + mockServer.setNotificationServiceDid(notificationService.did); + await mockServer.setup(page); + await stubNotificationPermission(page, { initial: "default" }); + await login(page); + await page.goto("/settings/notifications"); + + const toggle = page.locator('[data-testid="push-notifications-toggle"]'); + await expect(toggle).not.toHaveAttribute("disabled", "", { + timeout: 10000, + }); + expect(await page.evaluate(() => window.__permissionRequests)).toBe(0); + + await toggle.click(); + // iOS Safari only raises the prompt inside a user gesture, and the + // handoff return has none — so it must be asked for on this press. + await page + .locator( + '[data-testid="confirm-modal"] [data-testid="modal-confirm-button"]', + ) + .click(); + + await expect + .poll(() => page.evaluate(() => window.__permissionRequests)) + .toBeGreaterThan(0); + await expect(page).toHaveURL( + new RegExp( + notificationService.authUrl.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"), + ), + ); + }); + + test("a denied prompt stops before the handoff", async ({ page }) => { + const mockServer = new MockServer(); + mockServer.setNotificationServiceDid(notificationService.did); + await mockServer.setup(page); + await stubNotificationPermission(page, { + initial: "default", + onPrompt: "denied", + }); + await login(page); + await page.goto("/settings/notifications"); + + const toggle = page.locator('[data-testid="push-notifications-toggle"]'); + await expect(toggle).not.toHaveAttribute("disabled", "", { + timeout: 10000, + }); + await toggle.click(); + await page + .locator( + '[data-testid="confirm-modal"] [data-testid="modal-confirm-button"]', + ) + .click(); + + await expect(page.locator('[data-testid="toast"]')).toBeVisible(); + // Never sent through the handoff, so the page never left settings. + await expect(page).toHaveURL(/\/settings\/notifications$/); + }); + test("with a service named, the toggle is usable", async ({ page }) => { const mockServer = new MockServer(); mockServer.setNotificationServiceDid(notificationService.did); -- 2.51.2