fix(gui): wait for the browser's prompt before closing the permission popup

The deadline on a registration begun from a click counts no time while
the browser's own notification prompt is open, in the permission popup
and in the enable-push dialog. The enable-push dialog keeps one
registration in flight, settles on whenever it resolves while open, and
offers a retry only after a real failure. A permission dialog that ends
in a denial after Allow began a registration turns push back off and
restores the opt-out it cleared.
This commit is contained in:
Juan Castro committed 2026-10-27 16:48:07 -04:00
1 parent 415c179770
commit 6c35780d59
8 files changed
+183 -33

No files matched your search

+28 -22
View File
@@ -20,15 +20,14 @@
import {
browserPushState,
enableBrowserPush,
PUSH_ENABLE_TIMEOUT_MS,
pushClientWithin,
withEnableDeadline,
} from '../helpers/pushClient.js';
import {
guard_input,
identity_html,
INPUT_PROTECTION_MS,
resolve_requesting_entity,
with_timeout,
} from './UIPermissionDialog.js';
/** @typedef {import('../helpers/pushClient.js').BrowserPushState} BrowserPushState */
@@ -60,38 +59,45 @@ async function UIEnablePushDialog (options = {}) {
const $not_now = $(el_dialog).find('.perm-dialog-deny');
const $error = $(el_dialog).find('.perm-dialog-error');
let enabling = false;
// The one enable() in flight; cleared only when it failed, so a retry never overlaps it.
let pending = null;
// True from Turn on until the registration settles or outlasts its deadline.
let waiting = false;
const sync_buttons = () => {
const enabled = !! input_guard?.ready && ! enabling;
$turn_on.prop('disabled', ! enabled);
$not_now.prop('disabled', ! enabled);
const ready = !! input_guard?.ready && ! waiting;
$turn_on.prop('disabled', ! ready || !! pending);
$not_now.prop('disabled', ! ready);
};
sync_buttons();
$turn_on.on('click', async () => {
if ( enabling ) return;
enabling = true;
if ( pending ) return;
pending = enableBrowserPush(client);
pending.then((ok) => {
if ( ok ) settle('on');
});
waiting = true;
$turn_on.addClass('perm-dialog-busy');
sync_buttons();
$error.hide();
const enabled = await with_timeout(enableBrowserPush(client), PUSH_ENABLE_TIMEOUT_MS, false);
if ( enabled ) {
settle('on');
return;
const outcome = await withEnableDeadline(pending, { fallback: null });
if ( outcome === true ) return;
if ( outcome === false ) {
pending = null;
const now = await browserPushState(client);
if ( now !== 'off' ) {
settle(now);
return;
}
}
const now = await browserPushState(client);
if ( now !== 'off' ) {
settle(now);
return;
}
enabling = false;
waiting = false;
if ( ! el_dialog.open ) {
settle('off');
return;
}
$error.text(i18n('push_enable_failed', [], false)).show();
$turn_on.removeClass('perm-dialog-busy');
if ( ! pending ) $turn_on.removeClass('perm-dialog-busy');
sync_buttons();
});
@@ -99,11 +105,11 @@ async function UIEnablePushDialog (options = {}) {
el_dialog.addEventListener('cancel', (e) => {
e.preventDefault();
if ( enabling ) return;
if ( waiting ) return;
settle('off');
});
el_dialog.addEventListener('close', () => {
if ( enabling ) return;
if ( waiting ) return;
settle('off');
});
@@ -121,7 +127,7 @@ async function UIEnablePushDialog (options = {}) {
ms: INPUT_PROTECTION_MS,
on_change: (ready) => {
sync_buttons();
if ( ready && ! enabling ) $not_now.get(0)?.focus();
if ( ready && ! waiting ) $not_now.get(0)?.focus();
},
});
});
+62 -1
View File
@@ -17,7 +17,7 @@
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import jQuery from '../lib/jquery-3.6.1/jquery-3.6.1.min.js';
globalThis.$ = jQuery;
@@ -138,6 +138,67 @@ describe('the enable-push dialog', () => {
debug.mockRestore();
});
describe('with the clock under control', () => {
beforeEach(() => vi.useFakeTimers());
afterEach(() => {
vi.useRealTimers();
delete globalThis.Notification;
});
const shownFake = async () => {
await vi.waitFor(() => expect(document.querySelector('.enable-push-dialog')).not.toBeNull());
await vi.advanceTimersByTimeAsync(INPUT_PROTECTION_MS + 50);
};
it('waits as long as the browser prompt is open, then settles on when it registers', async () => {
globalThis.Notification = { permission: 'default' };
const client = fakeClient({ permission: 'default', registered: false });
let finish;
client.enable.mockImplementation(() => new Promise((resolve) => {
finish = resolve;
}));
registerPushClient(client);
const result = UIEnablePushDialog({ origin: 'https://chat.example' });
await shownFake();
$('.perm-dialog-allow').trigger('click');
await vi.advanceTimersByTimeAsync(60_000);
expect($('.perm-dialog-error').text()).toBe('');
expect($('.perm-dialog-not-now, .perm-dialog-deny').prop('disabled')).toBe(true);
expect(document.querySelector('dialog')).not.toBeNull();
globalThis.Notification.permission = 'granted';
finish({ deviceUid: 'dev-new' });
await vi.advanceTimersByTimeAsync(10);
await expect(result).resolves.toBe('on');
expect(client.enable).toHaveBeenCalledTimes(1);
});
it('keeps the one registration after its deadline and settles on when it resolves', async () => {
globalThis.Notification = { permission: 'granted' };
const client = fakeClient({ permission: 'granted', registered: false });
let finish;
client.enable.mockImplementation(() => new Promise((resolve) => {
finish = resolve;
}));
registerPushClient(client);
const result = UIEnablePushDialog({ origin: 'https://chat.example' });
await shownFake();
$('.perm-dialog-allow').trigger('click');
await vi.advanceTimersByTimeAsync(16_000);
expect($('.perm-dialog-error').text()).toBe('push_enable_failed');
expect($('.perm-dialog-allow').prop('disabled')).toBe(true);
expect($('.perm-dialog-deny').prop('disabled')).toBe(false);
$('.perm-dialog-allow').trigger('click');
expect(client.enable).toHaveBeenCalledTimes(1);
finish({ deviceUid: 'dev-new' });
await vi.advanceTimersByTimeAsync(10);
await expect(result).resolves.toBe('on');
});
});
it('offers a retry when registering failed and push is still off', async () => {
const client = fakeClient({ permission: 'default', registered: false });
client.enable.mockRejectedValueOnce(new Error('no_worker'));
@@ -123,6 +123,26 @@ describe('Allow on a request for notifications:push', () => {
errors.mockRestore();
});
it('turns push back off, and restores the opt-out, when the grant fails and the user then denies', async () => {
const client = fakeClient({ permission: 'default', registered: false });
registerPushClient(client);
localStorage.setItem('puter.push.browser-off:u-1', '1');
fetch.mockResolvedValue({ ok: false, status: 500 });
const errors = vi.spyOn(console, 'error').mockImplementation(() => {});
const decision = UIPermissionDialog({ permissions: ['notifications:push'], app_uid: `app-${apps}` });
await shown();
allow();
await vi.waitFor(() => expect($('.perm-dialog-error').text()).toBe('perm_dialog_error'));
expect(localStorage.getItem('puter.push.browser-off:u-1')).toBeNull();
$('.perm-dialog-deny').trigger('click');
await expect(decision).resolves.toBe(false);
await vi.waitFor(() => expect(client.disable).toHaveBeenCalledTimes(1));
await vi.waitFor(() => expect(localStorage.getItem('puter.push.browser-off:u-1')).toBe('1'));
errors.mockRestore();
});
it.each([
['on', { permission: 'granted', registered: true, deviceUid: 'dev-1' }],
['blocked', { permission: 'denied', registered: false }],
+18
View File
@@ -20,11 +20,13 @@
import { fsCreateKindFor } from '../helpers/fsCreateKind.js';
import { get_app_by_uid } from '../helpers/getAppByUid.js';
import {
browserPushOptedOut,
browserPushState,
enableBrowserPush,
getPushClient,
PUSH_PERMISSION,
pushClientWithin,
setBrowserPushOptOut,
} from '../helpers/pushClient.js';
// Keeps a second request for the same (entity, permission) from stacking a
@@ -198,6 +200,8 @@ async function show_permission_dialog (options) {
document.body.appendChild(el_dialog);
// The registration Allow began, kept so a second Allow doesn't start another.
let push_enable = null;
const user_uid = window.user?.uuid;
const opted_out_before = browserPushOptedOut(user_uid);
// Set once a grant request has been sent without the server definitively
// refusing it. A client-side timeout or a dropped response says nothing
@@ -217,6 +221,9 @@ async function show_permission_dialog (options) {
// account is reconciled in the background.
undo_uncertain_grant(options);
}
if ( ! granted && push_enable ) {
undo_browser_push(push_enable, user_uid, opted_out_before);
}
// `close()` fires the 'close' event; the handler below resolves
// false, so resolve first.
resolve(granted);
@@ -449,6 +456,17 @@ export function guard_input ({ win, doc, ms, on_change }) {
};
}
/** Turns push back off in this browser when Allow began a registration and the dialog still ended in a denial. @param {Promise<boolean>} push_enable @param {string|undefined} user_uid @param {boolean} opted_out_before */
function undo_browser_push (push_enable, user_uid, opted_out_before) {
push_enable.then((ok) => {
if ( ! ok ) return;
getPushClient()?.disable().then(
() => setBrowserPushOptOut(user_uid, opted_out_before),
(e) => console.debug('[push] disable failed', e),
);
});
}
/**
* Withdraws a grant whose POST may have committed even though the dialog never
* saw a success response, so a "Don't Allow" can't leave a live permission the
+22 -1
View File
@@ -34,9 +34,12 @@ export const PUSH_PERMISSION = 'notifications:push';
/** How long a dialog waits for the client an extension registers after its own checks. */
export const PUSH_CLIENT_WAIT_MS = 3000;
/** How long a registration begun from a click may take before the caller moves on. */
/** How long a registration begun from a click may take, once the browser's prompt is answered, before the caller moves on. */
export const PUSH_ENABLE_TIMEOUT_MS = 15000;
/** How often the deadline below looks at whether the browser's prompt is still open. */
export const PROMPT_POLL_MS = 250;
/** @type {PushClient|null} */
let pushClient = null;
@@ -129,4 +132,22 @@ export const enableBrowserPush = (client, userUid = globalThis.user?.uuid) => {
});
};
/** `promise`, or `fallback` once `ms` have passed with the browser's notification prompt closed; time with the prompt open does not count. @param {Promise<T>} promise @param {{ ms?: number, fallback?: F, NotificationApi?: { permission: string }, poll?: number }} [opts] @returns {Promise<T|F>} @template T, F */
export const withEnableDeadline = (promise, {
ms = PUSH_ENABLE_TIMEOUT_MS,
fallback = /** @type {F} */ (false),
NotificationApi = globalThis.Notification,
poll = PROMPT_POLL_MS,
} = {}) => {
let timer;
const deadline = new Promise((resolve) => {
const tick = () => {
const open = NotificationApi?.permission === 'default';
timer = setTimeout(open ? tick : () => resolve(fallback), open ? poll : ms);
};
tick();
});
return Promise.race([promise, deadline]).finally(() => clearTimeout(timer));
};
globalThis.def?.(registerPushClient, 'push.registerPushClient');
+27
View File
@@ -150,6 +150,33 @@ describe('the opt-out record', () => {
});
});
describe('withEnableDeadline', () => {
const never = new Promise(() => {});
const granted = { permission: 'granted' };
it('answers the promise when it settles in time', async () => {
await expect(mod.withEnableDeadline(Promise.resolve(true), { ms: 50, NotificationApi: granted })).resolves.toBe(true);
});
it('gives the fallback once the time is up with the prompt closed', async () => {
await expect(mod.withEnableDeadline(never, { ms: 20, fallback: null, NotificationApi: granted })).resolves.toBeNull();
await expect(mod.withEnableDeadline(never, { ms: 20, NotificationApi: undefined })).resolves.toBe(false);
});
it('counts no time while the browser prompt is open', async () => {
const NotificationApi = { permission: 'default' };
let done = false;
const waited = mod.withEnableDeadline(never, { ms: 20, poll: 5, NotificationApi }).then(() => {
done = true;
});
await new Promise((resolve) => setTimeout(resolve, 80));
expect(done).toBe(false);
NotificationApi.permission = 'denied';
await waited;
expect(done).toBe(true);
});
});
describe('enableBrowserPush', () => {
beforeEach(() => localStorage.clear());
+3 -2
View File
@@ -93,12 +93,13 @@ describe('the permission popup and browser push', () => {
it('takes the registration Allow began and waits for it after answering', () => {
expect(requestPermission).toContain('on_push_enable: (enabling) => {');
const answered = requestPermission.indexOf("msg: 'permissionGranted'");
const waited = requestPermission.indexOf('push_enable,');
// The deadline leaves the browser's own prompt its time.
const waited = requestPermission.indexOf('await withEnableDeadline(push_enable)');
const closed = requestPermission.indexOf('window.close()');
expect(answered).toBeGreaterThan(-1);
expect(waited).toBeGreaterThan(answered);
expect(closed).toBeGreaterThan(waited);
expect(requestPermission).toContain('PUSH_ENABLE_TIMEOUT_MS');
expect(requestPermission).not.toContain('setTimeout');
});
it('answers the enable-push opener after the dialog, then closes', () => {
+3 -7
View File
@@ -59,7 +59,7 @@ import {
} from './util/authmeGrant.js';
import init_device_signals from './helpers/deviceSignals.js';
import { holdsPermissions } from './helpers/holdsPermissions.js';
import { PUSH_ENABLE_TIMEOUT_MS, PUSH_PERMISSION } from './helpers/pushClient.js';
import { PUSH_PERMISSION, withEnableDeadline } from './helpers/pushClient.js';
import { startPushWorker } from './helpers/pushWorker.js';
import item_icon from './helpers/itemIcon.js';
import { installAppIconFallback } from './helpers/appIcon.js';
@@ -864,12 +864,8 @@ const postAuthActions = async (action) => {
// The popup exists only to host this dialog; close it once answered.
if ( window.embedded_in_popup ) {
if ( push_enable ) {
await Promise.race([
push_enable,
new Promise((resolve) => setTimeout(resolve, PUSH_ENABLE_TIMEOUT_MS)),
]);
}
// Closing would take the browser's own prompt down with it, so its time doesn't count.
if ( push_enable ) await withEnableDeadline(push_enable);
window.close();
window.open('', '_self').close();
}