diff --git a/package.json b/package.json index 01d4f2b0..711bbd24 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "impro", - "version": "0.19.2", + "version": "0.19.3", "type": "module", "scripts": { "start": "rm -rf \"${BUILD_DIR:-build}\" && NODE_ENV=development eleventy --serve", diff --git a/src/js/activeTabMonitor.js b/src/js/activeTabMonitor.js new file mode 100644 index 00000000..a02eecc6 --- /dev/null +++ b/src/js/activeTabMonitor.js @@ -0,0 +1,105 @@ +// JSON-RPC-style wrapper for BroadcastChannel +class RpcChannel { + #name; + #channel; + #handlers = new Map(); + #requestTimeoutMs; + + constructor(channel, { requestTimeoutMs }) { + this.#name = channel.name; + this.#requestTimeoutMs = requestTimeoutMs; + this.#channel = channel; + channel.addEventListener("message", (event) => this.#handleRequest(event)); + } + + onRequest(method, handler) { + this.#handlers.set(method, handler); + } + + // Resolves with the first result another tab returns, or null if none does. + request(method, params = null) { + const channel = this.#channel; + if (!channel) return Promise.resolve(null); // closed + const id = crypto.randomUUID(); + return new Promise((resolve) => { + const onMessage = (event) => { + if (event.data?.id !== id) return; + cleanup(); + resolve(event.data.result); + }; + const cleanup = () => { + clearTimeout(timer); + channel.removeEventListener("message", onMessage); + }; + const timer = setTimeout(() => { + cleanup(); + resolve(null); + }, this.#requestTimeoutMs); + channel.addEventListener("message", onMessage); + channel.postMessage({ method, params, id }); + }); + } + + close() { + this.#channel?.close(); + this.#channel = null; + this.#handlers.clear(); + } + + async #handleRequest(event) { + const { method, params = null, id = null } = event.data ?? {}; + const handler = this.#handlers.get(method); + if (!handler) return; + let result = null; + try { + result = await handler(params); + } catch (error) { + console.error(`[rpc:${this.#name}] ${method} handler failed`, error); + return; + } + // Every tab receives every request, so null means "no response" + if (id === null || result === null) return; + this.#channel?.postMessage({ id, result }); + } +} + +const CHANNEL_NAME = "active-tab-monitor"; +const REPLY_TIMEOUT_MS = 100; + +function isThisTabFocused() { + return document.visibilityState === "visible" && document.hasFocus(); +} + +// Queries for active tabs over a broadcast channel. Every tab starts one and +// answers the others' queries, so it runs whether or not this tab reads it. +export function startActiveTabMonitor({ + replyTimeoutMs = REPLY_TIMEOUT_MS, +} = {}) { + if (typeof BroadcastChannel === "undefined") { + console.warn("[activeTabMonitor] BroadcastChannel unavailable"); + return { + async isAnyTabActive() { + return isThisTabFocused(); + }, + stop() {}, + }; + } + + const rpc = new RpcChannel(new BroadcastChannel(CHANNEL_NAME), { + requestTimeoutMs: replyTimeoutMs, + }); + + rpc.onRequest("isTabActive", () => isThisTabFocused() || null); + + return { + async isAnyTabActive() { + if (isThisTabFocused()) return true; + const res = await rpc.request("isTabActive"); + return res === true; + }, + + stop() { + rpc.close(); + }, + }; +} diff --git a/src/js/app.js b/src/js/app.js index 4f5e13f7..8d3c0b51 100644 --- a/src/js/app.js +++ b/src/js/app.js @@ -44,7 +44,8 @@ import { Api } from "/js/api.js"; import { createAuth } from "/js/auth.js"; import { NotificationService } from "/js/notificationService.js"; import { ChatNotificationService } from "/js/chatNotificationService.js"; -import { SystemNotificationService } from "/js/systemNotificationService.js"; +import { DesktopNotificationService } from "/js/desktopNotificationService.js"; +import { startActiveTabMonitor } from "/js/activeTabMonitor.js"; import { PushNotificationService } from "/js/push/pushNotificationService.js"; import { PostComposerService } from "/js/postComposerService.js"; import { AccountSwitcherService } from "/js/accountSwitcherService.js"; @@ -129,12 +130,14 @@ export async function main() { const chatNotificationService = session ? new ChatNotificationService(api) : null; - const systemNotificationService = + const activeTabMonitor = startActiveTabMonitor(); + const desktopNotificationService = notificationService && chatNotificationService - ? new SystemNotificationService( + ? new DesktopNotificationService( notificationService, chatNotificationService, router, + activeTabMonitor, ) : null; const pushNotificationService = session @@ -226,8 +229,8 @@ export async function main() { chatNotificationService.startPolling(); } - if (systemNotificationService) { - systemNotificationService.start(); + if (desktopNotificationService) { + desktopNotificationService.start(); } if (pushNotificationService) { @@ -244,7 +247,7 @@ export async function main() { identityResolver, notificationService, chatNotificationService, - systemNotificationService, + desktopNotificationService, pushNotificationService, postComposerService, accountSwitcherService, diff --git a/src/js/systemNotificationService.js b/src/js/desktopNotificationService.js similarity index 87% rename from src/js/systemNotificationService.js rename to src/js/desktopNotificationService.js index 45e9c0d8..bb4cb451 100644 --- a/src/js/systemNotificationService.js +++ b/src/js/desktopNotificationService.js @@ -4,11 +4,17 @@ import { isTouchOnlyDevice } from "/js/utils.js"; const STORAGE_KEY = "system-notifications-enabled"; const ICON_URL = "/img/impro-logo-192.png"; -export class SystemNotificationService { - constructor(notificationService, chatNotificationService, router) { +export class DesktopNotificationService { + constructor( + notificationService, + chatNotificationService, + router, + activeTabMonitor, + ) { this.notificationService = notificationService; this.chatNotificationService = chatNotificationService; this.router = router; + this.activeTabMonitor = activeTabMonitor; this._lastSeenActivityCount = notificationService.$numNotifications.get() ?? 0; this._lastSeenChatCount = @@ -31,7 +37,7 @@ export class SystemNotificationService { : `You have ${activityCount} unread notifications`, tag: "impro-activity", url: "/notifications", - }); + }).catch(console.error); } if (chatCount > this._lastSeenChatCount) { @@ -43,7 +49,7 @@ export class SystemNotificationService { : `You have ${chatCount} unread conversations`, tag: "impro-chat", url: "/messages", - }); + }).catch(console.error); } this._lastSeenActivityCount = activityCount; @@ -72,10 +78,6 @@ export class SystemNotificationService { return this.isSupported ? Notification.permission : "unsupported"; } - get isTabActive() { - return document.visibilityState === "visible" && document.hasFocus(); - } - async requestPermission() { if (!this.isSupported) return "unsupported"; const result = await Notification.requestPermission(); @@ -89,19 +91,18 @@ export class SystemNotificationService { localStorage.removeItem(STORAGE_KEY); } - notify({ title, body, tag, url }) { + async notify({ title, body, tag, url }) { if ( !this.isSupported || !this.isEnabled || - Notification.permission !== "granted" || - this.isTabActive + Notification.permission !== "granted" ) { return; } + if (await this.activeTabMonitor.isAnyTabActive()) return; const notification = new Notification(title, { body, icon: ICON_URL, - badge: ICON_URL, tag, }); notification.onclick = () => { diff --git a/src/js/views/settings/notifications.view.js b/src/js/views/settings/notifications.view.js index a4ac7f85..d83e78ed 100644 --- a/src/js/views/settings/notifications.view.js +++ b/src/js/views/settings/notifications.view.js @@ -22,25 +22,25 @@ export default async function settingsNotificationsView({ root, router, layout, - context: { auth, systemNotificationService, pushNotificationService }, + context: { auth, desktopNotificationService, pushNotificationService }, }) { await auth.requireAuth(); const state = new ReactiveStore("settingsNotificationsView"); state.$enabled = new Signal.State( - systemNotificationService?.isEnabled ?? false, + desktopNotificationService?.isEnabled ?? false, ); state.$pushBusy = new Signal.State(false); async function handleToggle(checked) { - if (!systemNotificationService) return; + if (!desktopNotificationService) return; if (!checked) { - systemNotificationService.disable(); + desktopNotificationService.disable(); state.$enabled.set(false); showToast("Desktop notifications disabled."); return; } - const result = await systemNotificationService.requestPermission(); + const result = await desktopNotificationService.requestPermission(); if (result === "granted") { state.$enabled.set(true); showToast("Desktop notifications enabled.", { style: "success" }); @@ -147,9 +147,9 @@ export default async function settingsNotificationsView({ pageEffect(root, () => { const enabled = state.$enabled.get(); - const isSupported = systemNotificationService?.isSupported ?? false; + const isSupported = desktopNotificationService?.isSupported ?? false; const permissionState = - systemNotificationService?.permissionState ?? "unsupported"; + desktopNotificationService?.permissionState ?? "unsupported"; const isDenied = permissionState === "denied"; let description = @@ -183,7 +183,7 @@ export default async function settingsNotificationsView({ class=${classnames("setting-item", { "setting-item-disabled": systemRowDisabled, })} - data-testid="settings-section-system-notifications" + data-testid="settings-section-desktop-notifications" >

Desktop notifications

@@ -191,7 +191,7 @@ export default async function settingsNotificationsView({
Notifications view", () => { await login(page); await page.goto("/settings/notifications"); - const toggle = page.locator('[data-testid="system-notifications-toggle"]'); + const toggle = page.locator('[data-testid="desktop-notifications-toggle"]'); await expect(toggle).toBeVisible({ timeout: 10000 }); await expect(toggle).not.toHaveAttribute("checked", ""); @@ -82,7 +82,7 @@ test.describe("Settings > Notifications view", () => { await login(page); await page.goto("/settings/notifications"); - const toggle = page.locator('[data-testid="system-notifications-toggle"]'); + const toggle = page.locator('[data-testid="desktop-notifications-toggle"]'); await expect(toggle).toBeVisible({ timeout: 10000 }); await toggle.click(); @@ -108,7 +108,7 @@ test.describe("Settings > Notifications view", () => { await page.goto("/settings/notifications"); const toggle = page.locator( - '[data-testid="system-notifications-toggle"]', + '[data-testid="desktop-notifications-toggle"]', ); await expect(toggle).toBeVisible({ timeout: 10000 }); await expect(toggle).toHaveAttribute("disabled", ""); @@ -138,7 +138,7 @@ test.describe("Settings > Notifications view", () => { }); await page.goto("/settings/notifications"); - const toggle = page.locator('[data-testid="system-notifications-toggle"]'); + const toggle = page.locator('[data-testid="desktop-notifications-toggle"]'); await expect(toggle).toHaveAttribute("checked", "", { timeout: 10000 }); await toggle.click(); diff --git a/tests/unit/specs/activeTabMonitor.test.js b/tests/unit/specs/activeTabMonitor.test.js new file mode 100644 index 00000000..c7efe620 --- /dev/null +++ b/tests/unit/specs/activeTabMonitor.test.js @@ -0,0 +1,186 @@ +import { describe, it, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import { startActiveTabMonitor } from "/js/activeTabMonitor.js"; +import { installFakeBroadcastChannel } from "../testHelpers.js"; + +const CHANNEL_NAME = "active-tab-monitor"; + +function macrotask() { + return new Promise((resolve) => setTimeout(resolve, 0)); +} + +// One delivery hop each way, plus room for the reply to be handled. +async function flushChannel() { + await macrotask(); + await macrotask(); + await macrotask(); +} + +describe("startActiveTabMonitor", () => { + let monitors; + let peers; + let restoreBroadcastChannel; + let originalHasFocus; + + function simulateTabState({ visible, focused }) { + Object.defineProperty(document, "visibilityState", { + value: visible ? "visible" : "hidden", + configurable: true, + }); + document.hasFocus = () => focused; + } + + function startMonitor({ replyTimeoutMs = 100 } = {}) { + const monitor = startActiveTabMonitor({ replyTimeoutMs }); + monitors.push(monitor); + return monitor; + } + + // Stands in for another tab, so the monitor under test is the only thing + // reading this document's focus state. + function createPeer({ answersQueries = false, id = null } = {}) { + const channel = new BroadcastChannel(CHANNEL_NAME); + const received = []; + channel.addEventListener("message", (event) => { + received.push(event.data); + if (answersQueries && event.data?.method === "isTabActive") { + channel.postMessage({ id: id ?? event.data.id, result: true }); + } + }); + peers.push(channel); + return { channel, received }; + } + + beforeEach(() => { + monitors = []; + peers = []; + restoreBroadcastChannel = installFakeBroadcastChannel(); + originalHasFocus = document.hasFocus; + simulateTabState({ visible: true, focused: false }); + }); + + afterEach(() => { + for (const monitor of monitors) { + monitor.stop(); + } + for (const peer of peers) { + peer.close(); + } + restoreBroadcastChannel(); + document.hasFocus = originalHasFocus; + delete document.visibilityState; + }); + + describe("this tab's own focus", () => { + it("is active when visible and focused", async () => { + const monitor = startMonitor(); + simulateTabState({ visible: true, focused: true }); + + assert.deepEqual(await monitor.isAnyTabActive(), true); + }); + + it("is not active when focused but hidden", async () => { + const monitor = startMonitor(); + simulateTabState({ visible: false, focused: true }); + + assert.deepEqual(await monitor.isAnyTabActive(), false); + }); + + it("is not active when visible but unfocused", async () => { + const monitor = startMonitor(); + simulateTabState({ visible: true, focused: false }); + + assert.deepEqual(await monitor.isAnyTabActive(), false); + }); + }); + + describe("answering other tabs", () => { + it("answers a focus query while focused", async () => { + startMonitor(); + const peer = createPeer(); + simulateTabState({ visible: true, focused: true }); + + peer.channel.postMessage({ method: "isTabActive", id: "req-1" }); + await flushChannel(); + + assert.deepEqual(peer.received, [{ id: "req-1", result: true }]); + }); + + it("stays silent on a focus query while unfocused", async () => { + startMonitor(); + const peer = createPeer(); + + peer.channel.postMessage({ method: "isTabActive", id: "req-1" }); + await flushChannel(); + + assert.deepEqual(peer.received, []); + }); + + it("stops answering once stopped", async () => { + const monitor = startMonitor(); + const peer = createPeer(); + simulateTabState({ visible: true, focused: true }); + + monitor.stop(); + peer.channel.postMessage({ method: "isTabActive", id: "req-1" }); + await flushChannel(); + + assert.deepEqual(peer.received, []); + }); + }); + + describe("asking other tabs", () => { + it("is active when a peer answers", async () => { + createPeer({ answersQueries: true }); + const monitor = startMonitor(); + + assert.deepEqual(await monitor.isAnyTabActive(), true); + }); + + it("ignores a response carrying another request's id", async () => { + createPeer({ answersQueries: true, id: "someone-else" }); + const monitor = startMonitor(); + + assert.deepEqual(await monitor.isAnyTabActive(), false); + }); + + it("is inactive when a peer stays silent", async () => { + createPeer(); + const monitor = startMonitor(); + + assert.deepEqual(await monitor.isAnyTabActive(), false); + }); + + it("is inactive when no other tab is open", async () => { + const monitor = startMonitor(); + + assert.deepEqual(await monitor.isAnyTabActive(), false); + }); + + it("does not ask when this tab is already active", async () => { + const peer = createPeer({ answersQueries: true }); + const monitor = startMonitor(); + simulateTabState({ visible: true, focused: true }); + + assert.deepEqual(await monitor.isAnyTabActive(), true); + await flushChannel(); + + assert.deepEqual(peer.received, []); + }); + + it("is inactive without BroadcastChannel support", async (t) => { + const warn = t.mock.method(console, "warn", () => {}); + restoreBroadcastChannel(); + const originalChannel = globalThis.BroadcastChannel; + delete globalThis.BroadcastChannel; + try { + const monitor = startActiveTabMonitor(); + assert.deepEqual(await monitor.isAnyTabActive(), false); + assert.deepEqual(warn.mock.callCount(), 1); + } finally { + globalThis.BroadcastChannel = originalChannel; + restoreBroadcastChannel = installFakeBroadcastChannel(); + } + }); + }); +}); diff --git a/tests/unit/specs/systemNotificationService.test.js b/tests/unit/specs/desktopNotificationService.test.js similarity index 89% rename from tests/unit/specs/systemNotificationService.test.js rename to tests/unit/specs/desktopNotificationService.test.js index 3a74ba6d..7fbc2d6c 100644 --- a/tests/unit/specs/systemNotificationService.test.js +++ b/tests/unit/specs/desktopNotificationService.test.js @@ -1,7 +1,8 @@ import { describe, it, beforeEach, afterEach } from "node:test"; import assert from "node:assert/strict"; import { Signal } from "/js/signals.js"; -import { SystemNotificationService } from "/js/systemNotificationService.js"; +import { DesktopNotificationService } from "/js/desktopNotificationService.js"; +import { startActiveTabMonitor } from "/js/activeTabMonitor.js"; function createMockNotificationService({ numNotifications = 0 } = {}) { return { @@ -15,17 +16,25 @@ function createMockChatNotificationService({ numNotifications = 0 } = {}) { }; } -function flushEffects() { - return new Promise((resolve) => +function macrotask() { + return new Promise((resolve) => setTimeout(resolve, 0)); +} + +// Effects render on rAF, then notify() awaits the cross-tab focus query before +// constructing the Notification. +async function flushEffects() { + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve)), ); + await macrotask(); + await macrotask(); } function enable() { localStorage.setItem("system-notifications-enabled", "true"); } -describe("SystemNotificationService", () => { +describe("DesktopNotificationService", () => { let instances; let originalNotification; let originalMatchMedia; @@ -33,6 +42,7 @@ describe("SystemNotificationService", () => { let navigations; let router; let originalHasFocus; + let tabMonitors; function simulateTabState({ visible, focused }) { Object.defineProperty(document, "visibilityState", { @@ -50,11 +60,21 @@ describe("SystemNotificationService", () => { }); } + // A monitor with no peer tabs: its focus queries time out immediately, so + // these tests exercise only this tab's own focus state. + function createTabMonitor() { + const monitor = startActiveTabMonitor({ replyTimeoutMs: 0 }); + tabMonitors.push(monitor); + return monitor; + } + function startService(notificationService, chatNotificationService) { - const service = new SystemNotificationService( + const activeTabMonitor = createTabMonitor(); + const service = new DesktopNotificationService( notificationService, chatNotificationService, router, + activeTabMonitor, ); const dispose = service.start(); disposers.push(dispose); @@ -64,6 +84,7 @@ describe("SystemNotificationService", () => { beforeEach(() => { instances = []; disposers = []; + tabMonitors = []; navigations = []; router = { go: (path) => navigations.push(path) }; originalNotification = globalThis.Notification; @@ -85,10 +106,16 @@ describe("SystemNotificationService", () => { localStorage.clear(); }); - afterEach(() => { + afterEach(async () => { for (const dispose of disposers) { dispose(); } + for (const monitor of tabMonitors) { + monitor.stop(); + } + // Let any notify() still awaiting a focus query settle while the mock + // Notification is in place. + await flushEffects(); globalThis.Notification = originalNotification; window.matchMedia = originalMatchMedia; document.hasFocus = originalHasFocus; @@ -289,10 +316,11 @@ describe("SystemNotificationService", () => { describe("touch-only devices", () => { it("reports as unsupported even though the Notification API exists", () => { simulateTouchOnlyDevice(); - const service = new SystemNotificationService( + const service = new DesktopNotificationService( createMockNotificationService(), createMockChatNotificationService(), router, + createTabMonitor(), ); assert.deepEqual(typeof globalThis.Notification !== "undefined", true); @@ -300,10 +328,11 @@ describe("SystemNotificationService", () => { }); it("reports as supported when hover and a fine pointer exist", () => { - const service = new SystemNotificationService( + const service = new DesktopNotificationService( createMockNotificationService(), createMockChatNotificationService(), router, + createTabMonitor(), ); assert.deepEqual(service.isSupported, true); @@ -324,10 +353,11 @@ describe("SystemNotificationService", () => { it("does not request permission or set the storage flag", async () => { simulateTouchOnlyDevice(); - const service = new SystemNotificationService( + const service = new DesktopNotificationService( createMockNotificationService(), createMockChatNotificationService(), router, + createTabMonitor(), ); const result = await service.requestPermission(); @@ -341,10 +371,11 @@ describe("SystemNotificationService", () => { it("sets the storage flag when granted", async () => { const notificationService = createMockNotificationService(); const chatNotificationService = createMockChatNotificationService(); - const service = new SystemNotificationService( + const service = new DesktopNotificationService( notificationService, chatNotificationService, router, + createTabMonitor(), ); globalThis.Notification.permission = "granted"; @@ -357,10 +388,11 @@ describe("SystemNotificationService", () => { it("does not set the storage flag when denied", async () => { const notificationService = createMockNotificationService(); const chatNotificationService = createMockChatNotificationService(); - const service = new SystemNotificationService( + const service = new DesktopNotificationService( notificationService, chatNotificationService, router, + createTabMonitor(), ); globalThis.Notification.permission = "denied"; @@ -375,10 +407,11 @@ describe("SystemNotificationService", () => { it("clears the storage flag", () => { const notificationService = createMockNotificationService(); const chatNotificationService = createMockChatNotificationService(); - const service = new SystemNotificationService( + const service = new DesktopNotificationService( notificationService, chatNotificationService, router, + createTabMonitor(), ); enable(); assert.deepEqual(service.isEnabled, true); diff --git a/tests/unit/specs/pushNotificationService.test.js b/tests/unit/specs/pushNotificationService.test.js index cbae0845..0239a138 100644 --- a/tests/unit/specs/pushNotificationService.test.js +++ b/tests/unit/specs/pushNotificationService.test.js @@ -260,7 +260,7 @@ describe("PushNotificationService registration", () => { it("is unsupported on a device that isn't touch-only", () => { setupDom(); - // Desktop gets SystemNotificationService's in-tab notifications instead, + // Desktop gets DesktopNotificationService's in-tab notifications instead, // which is gated on the same check the other way round. globalThis.window.matchMedia = (query) => ({ matches: false, diff --git a/tests/unit/testHelpers.js b/tests/unit/testHelpers.js index 4ad72045..2a96a0f3 100644 --- a/tests/unit/testHelpers.js +++ b/tests/unit/testHelpers.js @@ -312,3 +312,44 @@ export function installFakeIndexedDB({ failWrites = false } = {}) { }; return { records, openCalls, createdStores }; } + +// JSDOM has no BroadcastChannel, and node's rejects its own MessageEvent once +// JSDOM has replaced the global Event. This double keeps the parts callers rely +// on: async delivery to every same-named channel except the sender. +export function installFakeBroadcastChannel() { + const channelsByName = new Map(); + + // window.EventTarget, not node's — node's rejects JSDOM MessageEvents. + class FakeBroadcastChannel extends window.EventTarget { + constructor(name) { + super(); + this.name = name; + this.closed = false; + const peers = channelsByName.get(name) ?? new Set(); + peers.add(this); + channelsByName.set(name, peers); + } + + postMessage(data) { + if (this.closed) return; + for (const peer of channelsByName.get(this.name) ?? []) { + if (peer === this || peer.closed) continue; + setTimeout(() => { + if (peer.closed) return; + peer.dispatchEvent(new window.MessageEvent("message", { data })); + }, 0); + } + } + + close() { + this.closed = true; + channelsByName.get(this.name)?.delete(this); + } + } + + const original = globalThis.BroadcastChannel; + globalThis.BroadcastChannel = FakeBroadcastChannel; + return () => { + globalThis.BroadcastChannel = original; + }; +}