From 62afa7c04ac69539358e958ae556556a1349ffd0 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Thu, 12 Mar 2026 22:45:38 +0000 Subject: [PATCH 1/5] fix(notifications): return Notification-like stub so Teams can manage lifecycle Teams calls addEventListener, close(), and dispatchEvent() on the object returned by new Notification(). Our bare stubs lacked these methods, breaking Teams' internal state machine after the first notification and causing subsequent messages to only show title-based unread counts. Fixes #2248 Co-Authored-By: Claude Opus 4.6 --- app/browser/preload.js | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/app/browser/preload.js b/app/browser/preload.js index 5788a46b..07d2a1fb 100644 --- a/app/browser/preload.js +++ b/app/browser/preload.js @@ -151,6 +151,30 @@ ipcRenderer.invoke("get-config").then((config) => { console.error("Preload: Failed to load config for notifications:", err); }); +// Create a Notification-like stub so Teams can manage lifecycle without errors. +// Without addEventListener/close/dispatchEvent, Teams' internal state machine +// breaks after the first notification, causing subsequent ones to stop firing. +function createNotificationStub() { + const stub = { + onclick: null, + onclose: null, + onerror: null, + onshow: null, + close() { if (this.onclose) this.onclose(); }, + addEventListener(type, listener) { + if (type === 'click') this.onclick = listener; + else if (type === 'close') this.onclose = listener; + else if (type === 'show') this.onshow = listener; + else if (type === 'error') this.onerror = listener; + }, + removeEventListener() {}, + dispatchEvent() { return true; }, + }; + // Fire the show event asynchronously like a real Notification + setTimeout(() => { if (stub.onshow) stub.onshow(); }, 0); + return stub; +} + // Helper functions for notification handling (extracted to reduce cognitive complexity) function playNotificationSound(notifSound) { if (globalThis.electronAPI?.playNotificationSound) { @@ -180,10 +204,9 @@ function createWebNotification(classicNotification, title, options) { return new classicNotification(title, options); } catch (err) { console.debug("Could not create native notification:", err); - return null; } } - return null; + return createNotificationStub(); } function createElectronNotification(options) { @@ -195,8 +218,7 @@ function createElectronNotification(options) { console.debug("showNotification failed", e); } } - // Return stub object for Electron notifications - return { onclick: null, onclose: null, onerror: null }; + return createNotificationStub(); } function createCustomNotification(title, options) { @@ -229,8 +251,7 @@ function createCustomNotification(title, options) { console.error("Failed to send custom notification:", e); } - // Return stub object - return { onclick: null, onclose: null, onerror: null }; + return createNotificationStub(); } // Override window.Notification immediately before Teams loads @@ -245,8 +266,7 @@ function createCustomNotification(title, options) { function CustomNotification(title, options) { // Use config from closure scope (will be null initially, populated async) if (notificationConfig?.disableNotifications) { - // Return dummy object to avoid Teams errors - return { onclick: null, onclose: null, onerror: null }; + return createNotificationStub(); } options = options || {}; @@ -264,8 +284,7 @@ function createCustomNotification(title, options) { } if (method === "web") { - const notification = createWebNotification(classicNotification, title, options); - return notification || { onclick: null, onclose: null, onerror: null }; + return createWebNotification(classicNotification, title, options); } return createElectronNotification(options); From a33522e797369a1838800c426d8573fe08e3abf9 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" Date: Thu, 12 Mar 2026 22:46:05 +0000 Subject: [PATCH 2/5] chore: add changelog entry for PR #2329 --- .changelog/pr-2329.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 .changelog/pr-2329.txt diff --git a/.changelog/pr-2329.txt b/.changelog/pr-2329.txt new file mode 100644 index 00000000..863d29a2 --- /dev/null +++ b/.changelog/pr-2329.txt @@ -0,0 +1 @@ +Fix: Ensure all notification methods return a Notification-like stub for Teams. - by @IsmaelMartinez (#2329) From 42a730cd885baf7b016940907a5d225f2d50aa3b Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Thu, 12 Mar 2026 23:31:46 +0000 Subject: [PATCH 3/5] fix(notifications): implement removeEventListener on notification stub Co-Authored-By: Claude Opus 4.6 --- app/browser/preload.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/browser/preload.js b/app/browser/preload.js index 07d2a1fb..f9eeee46 100644 --- a/app/browser/preload.js +++ b/app/browser/preload.js @@ -167,7 +167,12 @@ function createNotificationStub() { else if (type === 'show') this.onshow = listener; else if (type === 'error') this.onerror = listener; }, - removeEventListener() {}, + removeEventListener(type, listener) { + if (type === 'click' && (!listener || this.onclick === listener)) this.onclick = null; + else if (type === 'close' && (!listener || this.onclose === listener)) this.onclose = null; + else if (type === 'show' && (!listener || this.onshow === listener)) this.onshow = null; + else if (type === 'error' && (!listener || this.onerror === listener)) this.onerror = null; + }, dispatchEvent() { return true; }, }; // Fire the show event asynchronously like a real Notification From 8c78cad70d386a71be40e6ae3d6503abdbd2dece Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Fri, 13 Mar 2026 11:05:22 +0000 Subject: [PATCH 4/5] fix(notifications): scope lifecycle stub to electron and custom paths only Web notification path returns the real Notification object (or null on failure), preserving Teams' ability to manage its own lifecycle. The Notification-like stub is only needed for electron and custom methods where the actual notification is handled out-of-process. Co-Authored-By: Claude Opus 4.6 --- app/browser/preload.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/app/browser/preload.js b/app/browser/preload.js index f9eeee46..f2ea142e 100644 --- a/app/browser/preload.js +++ b/app/browser/preload.js @@ -209,9 +209,10 @@ function createWebNotification(classicNotification, title, options) { return new classicNotification(title, options); } catch (err) { console.debug("Could not create native notification:", err); + return null; } } - return createNotificationStub(); + return null; } function createElectronNotification(options) { @@ -271,7 +272,8 @@ function createCustomNotification(title, options) { function CustomNotification(title, options) { // Use config from closure scope (will be null initially, populated async) if (notificationConfig?.disableNotifications) { - return createNotificationStub(); + // Return dummy object to avoid Teams errors + return { onclick: null, onclose: null, onerror: null }; } options = options || {}; @@ -289,7 +291,8 @@ function createCustomNotification(title, options) { } if (method === "web") { - return createWebNotification(classicNotification, title, options); + const notification = createWebNotification(classicNotification, title, options); + return notification || { onclick: null, onclose: null, onerror: null }; } return createElectronNotification(options); From 359ed5ebff0a8c2c9243a53d51ea003fb7d76a56 Mon Sep 17 00:00:00 2001 From: Ismael Martinez Ramos Date: Fri, 13 Mar 2026 11:09:54 +0000 Subject: [PATCH 5/5] test(notifications): add e2e tests for notification lifecycle stub Verifies that the preload.js Notification override returns objects with the lifecycle methods Teams expects (addEventListener, removeEventListener, close, dispatchEvent). Covers electron, custom, and web notification methods. Tests run without authentication, so they work both locally and in the cross-distro Docker containers. Co-Authored-By: Claude Opus 4.6 --- tests/e2e/notifications.spec.js | 340 ++++++++++++++++++++++++++++++++ 1 file changed, 340 insertions(+) create mode 100644 tests/e2e/notifications.spec.js diff --git a/tests/e2e/notifications.spec.js b/tests/e2e/notifications.spec.js new file mode 100644 index 00000000..c99eee17 --- /dev/null +++ b/tests/e2e/notifications.spec.js @@ -0,0 +1,340 @@ +import { test, expect } from '@playwright/test'; +import { _electron as electron } from 'playwright'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +/** + * Notification lifecycle tests. + * + * Verifies that the preload.js Notification override returns objects with + * the lifecycle methods Teams expects (addEventListener, removeEventListener, + * close, dispatchEvent). Without these, Teams' internal state machine breaks + * after the first notification call, causing subsequent ones to silently fail. + * + * These tests launch the app with a clean profile (no login session), so they + * exercise the preload in a real Electron renderer without needing Microsoft + * authentication. They work both locally and inside Docker cross-distro + * containers. + */ + +const LIFECYCLE_METHODS = ['addEventListener', 'removeEventListener', 'close', 'dispatchEvent']; +const CALLBACK_PROPERTIES = ['onclick', 'onclose', 'onerror', 'onshow']; + +async function launchApp(notificationMethod) { + const userDataDir = mkdtempSync(join(tmpdir(), 'teams-e2e-notif-')); + const args = [ + './app/index.js', + `--notificationMethod=${notificationMethod}`, + ...(process.env.CI ? ['--no-sandbox'] : []), + ]; + + const electronApp = await electron.launch({ + args, + env: { ...process.env, E2E_USER_DATA_DIR: userDataDir }, + timeout: 30000, + }); + + return { electronApp, userDataDir }; +} + +async function getMainWindow(electronApp) { + await electronApp.firstWindow({ timeout: 30000 }); + // Wait for windows to settle + await new Promise(resolve => setTimeout(resolve, 4000)); + + const windows = electronApp.windows(); + // Find a window with a real page (not about:blank) + return windows.find(w => { + const url = w.url(); + try { + const hostname = new URL(url).hostname; + return hostname === 'teams.cloud.microsoft' || + hostname === 'teams.microsoft.com' || + hostname === 'teams.live.com' || + hostname === 'login.microsoftonline.com'; + } catch { + return false; + } + }); +} + +async function cleanup(electronApp, userDataDir) { + if (electronApp) { + try { + await Promise.race([ + electronApp.close(), + new Promise((_, reject) => setTimeout(() => reject(new Error('timeout')), 5000)), + ]); + } catch { + try { + const pid = electronApp.process()?.pid; + if (pid) process.kill(pid, 'SIGKILL'); + } catch { /* ignore */ } + } + } + if (userDataDir) { + try { rmSync(userDataDir, { recursive: true, force: true }); } catch { /* ignore */ } + } +} + +test.describe('Notification override', () => { + test('window.Notification is overridden with correct static API', async () => { + let electronApp, userDataDir; + try { + ({ electronApp, userDataDir } = await launchApp('electron')); + const mainWindow = await getMainWindow(electronApp); + expect(mainWindow).toBeTruthy(); + + await mainWindow.waitForLoadState('load', { timeout: 30000 }); + + // Check static API + const permissionValue = await mainWindow.evaluate(() => window.Notification.permission); + expect(permissionValue).toBe('granted'); + + const requestResult = await mainWindow.evaluate(() => window.Notification.requestPermission()); + expect(requestResult).toBe('granted'); + } finally { + await cleanup(electronApp, userDataDir); + } + }); + + test('electron method returns stub with lifecycle methods', async () => { + let electronApp, userDataDir; + try { + ({ electronApp, userDataDir } = await launchApp('electron')); + const mainWindow = await getMainWindow(electronApp); + expect(mainWindow).toBeTruthy(); + + await mainWindow.waitForLoadState('load', { timeout: 30000 }); + + // Create a notification and inspect the returned object + const stubShape = await mainWindow.evaluate(() => { + const n = new window.Notification('Test', { body: 'test body' }); + return { + hasAddEventListener: typeof n.addEventListener === 'function', + hasRemoveEventListener: typeof n.removeEventListener === 'function', + hasClose: typeof n.close === 'function', + hasDispatchEvent: typeof n.dispatchEvent === 'function', + hasOnclick: 'onclick' in n, + hasOnclose: 'onclose' in n, + hasOnerror: 'onerror' in n, + hasOnshow: 'onshow' in n, + }; + }); + + expect(stubShape.hasAddEventListener).toBe(true); + expect(stubShape.hasRemoveEventListener).toBe(true); + expect(stubShape.hasClose).toBe(true); + expect(stubShape.hasDispatchEvent).toBe(true); + expect(stubShape.hasOnclick).toBe(true); + expect(stubShape.hasOnclose).toBe(true); + expect(stubShape.hasOnerror).toBe(true); + expect(stubShape.hasOnshow).toBe(true); + } finally { + await cleanup(electronApp, userDataDir); + } + }); + + test('electron stub addEventListener wires up callbacks correctly', async () => { + let electronApp, userDataDir; + try { + ({ electronApp, userDataDir } = await launchApp('electron')); + const mainWindow = await getMainWindow(electronApp); + expect(mainWindow).toBeTruthy(); + + await mainWindow.waitForLoadState('load', { timeout: 30000 }); + + const result = await mainWindow.evaluate(() => { + const n = new window.Notification('Test', { body: 'test' }); + let clickFired = false; + let closeFired = false; + + n.addEventListener('click', () => { clickFired = true; }); + n.addEventListener('close', () => { closeFired = true; }); + + // Simulate Teams calling onclick/onclose + if (n.onclick) n.onclick(); + if (n.onclose) n.onclose(); + + return { clickFired, closeFired }; + }); + + expect(result.clickFired).toBe(true); + expect(result.closeFired).toBe(true); + } finally { + await cleanup(electronApp, userDataDir); + } + }); + + test('electron stub removeEventListener clears callbacks', async () => { + let electronApp, userDataDir; + try { + ({ electronApp, userDataDir } = await launchApp('electron')); + const mainWindow = await getMainWindow(electronApp); + expect(mainWindow).toBeTruthy(); + + await mainWindow.waitForLoadState('load', { timeout: 30000 }); + + const result = await mainWindow.evaluate(() => { + const n = new window.Notification('Test', { body: 'test' }); + const handler = () => {}; + + n.addEventListener('click', handler); + const hadHandler = n.onclick === handler; + + n.removeEventListener('click', handler); + const cleared = n.onclick === null; + + return { hadHandler, cleared }; + }); + + expect(result.hadHandler).toBe(true); + expect(result.cleared).toBe(true); + } finally { + await cleanup(electronApp, userDataDir); + } + }); + + test('electron stub close() triggers onclose callback', async () => { + let electronApp, userDataDir; + try { + ({ electronApp, userDataDir } = await launchApp('electron')); + const mainWindow = await getMainWindow(electronApp); + expect(mainWindow).toBeTruthy(); + + await mainWindow.waitForLoadState('load', { timeout: 30000 }); + + const closeFired = await mainWindow.evaluate(() => { + const n = new window.Notification('Test', { body: 'test' }); + let fired = false; + n.addEventListener('close', () => { fired = true; }); + n.close(); + return fired; + }); + + expect(closeFired).toBe(true); + } finally { + await cleanup(electronApp, userDataDir); + } + }); + + test('electron stub fires show event asynchronously', async () => { + let electronApp, userDataDir; + try { + ({ electronApp, userDataDir } = await launchApp('electron')); + const mainWindow = await getMainWindow(electronApp); + expect(mainWindow).toBeTruthy(); + + await mainWindow.waitForLoadState('load', { timeout: 30000 }); + + const showFired = await mainWindow.evaluate(async () => { + const n = new window.Notification('Test', { body: 'test' }); + let fired = false; + n.addEventListener('show', () => { fired = true; }); + // Wait for the async setTimeout(0) to fire + await new Promise(resolve => setTimeout(resolve, 50)); + return fired; + }); + + expect(showFired).toBe(true); + } finally { + await cleanup(electronApp, userDataDir); + } + }); + + test('custom method returns stub with lifecycle methods', async () => { + let electronApp, userDataDir; + try { + ({ electronApp, userDataDir } = await launchApp('custom')); + const mainWindow = await getMainWindow(electronApp); + expect(mainWindow).toBeTruthy(); + + await mainWindow.waitForLoadState('load', { timeout: 30000 }); + + const stubShape = await mainWindow.evaluate(() => { + const n = new window.Notification('Test', { body: 'test body' }); + return { + hasAddEventListener: typeof n.addEventListener === 'function', + hasRemoveEventListener: typeof n.removeEventListener === 'function', + hasClose: typeof n.close === 'function', + hasDispatchEvent: typeof n.dispatchEvent === 'function', + }; + }); + + expect(stubShape.hasAddEventListener).toBe(true); + expect(stubShape.hasRemoveEventListener).toBe(true); + expect(stubShape.hasClose).toBe(true); + expect(stubShape.hasDispatchEvent).toBe(true); + } finally { + await cleanup(electronApp, userDataDir); + } + }); + + test('web method does not break native Notification lifecycle', async () => { + let electronApp, userDataDir; + try { + ({ electronApp, userDataDir } = await launchApp('web')); + const mainWindow = await getMainWindow(electronApp); + expect(mainWindow).toBeTruthy(); + + await mainWindow.waitForLoadState('load', { timeout: 30000 }); + + // For web method, verify the override exists and returns something + // (either a real Notification or a bare fallback). The key assertion is + // that calling new Notification() does not throw. + const result = await mainWindow.evaluate(() => { + try { + const n = new window.Notification('Test', { body: 'test body' }); + return { + didNotThrow: true, + type: typeof n, + hasOnclick: 'onclick' in n, + }; + } catch (e) { + return { didNotThrow: false, error: e.message }; + } + }); + + expect(result.didNotThrow).toBe(true); + expect(result.type).toBe('object'); + expect(result.hasOnclick).toBe(true); + } finally { + await cleanup(electronApp, userDataDir); + } + }); + + test('multiple notifications can be created sequentially', async () => { + let electronApp, userDataDir; + try { + ({ electronApp, userDataDir } = await launchApp('electron')); + const mainWindow = await getMainWindow(electronApp); + expect(mainWindow).toBeTruthy(); + + await mainWindow.waitForLoadState('load', { timeout: 30000 }); + + // This is the core regression test: Teams creates multiple notifications + // in sequence. If the stub is missing lifecycle methods, Teams' state + // machine breaks and stops calling new Notification() after the first. + const result = await mainWindow.evaluate(() => { + const stubs = []; + for (let i = 0; i < 5; i++) { + const n = new window.Notification(`Test ${i}`, { body: `body ${i}` }); + stubs.push({ + hasAddEventListener: typeof n.addEventListener === 'function', + hasClose: typeof n.close === 'function', + }); + } + return stubs; + }); + + expect(result).toHaveLength(5); + for (const stub of result) { + expect(stub.hasAddEventListener).toBe(true); + expect(stub.hasClose).toBe(true); + } + } finally { + await cleanup(electronApp, userDataDir); + } + }); +});