diff --git a/components/mail/__tests__/mail-app-source-routing.test.tsx b/components/mail/__tests__/mail-app-source-routing.test.tsx index b0dd78261..329c8c0a1 100644 --- a/components/mail/__tests__/mail-app-source-routing.test.tsx +++ b/components/mail/__tests__/mail-app-source-routing.test.tsx @@ -256,3 +256,29 @@ describe('archiving a group message by month', () => { expect(client.createMailbox).toHaveBeenCalledWith('2026', 'k', 'ce', undefined); }); }); + +describe('shared message deep link from the personal Inbox (#1491)', () => { + it('selects the shared folder for a freshly fetched message without source stamps', async () => { + window.history.replaceState(null, '', '/mail/message/eaaaaab?owner=ce'); + const email = { ...groupMessage, mailboxIds: { 'ce:a': true }, sourceClientAccountId: undefined, sourceAccountId: undefined } as Email; + const fetchContent = vi.fn().mockImplementation(async () => { + useEmailStore.setState({ selectedEmail: email }); + return email; + }); + const fetchEmails = vi.fn().mockResolvedValue(undefined); + useEmailStore.setState({ + emails: [], selectedEmail: null, selectedMailbox: 'a', selectedKeyword: null, + fetchEmailContent: fetchContent, fetchEmails, + }); + const mounted = render(); + try { + await waitFor(() => expect(fetchContent).toHaveBeenCalledWith(client, 'eaaaaab', 'ce')); + await waitFor(() => expect(useEmailStore.getState().selectedMailbox).toBe('ce:a')); + expect(fetchEmails).toHaveBeenCalledWith(client, 'ce:a', { background: true }); + expect(useEmailStore.getState().selectedEmail?.subject).toBe('GRP one'); + } finally { + mounted.unmount(); + window.history.replaceState(null, '', '/'); + } + }); +}); diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 33ae06070..e40a46794 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -1796,7 +1796,11 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { ? await freshMailboxes(switchedFrom) : useEmailStore.getState().mailboxes; const state = useEmailStore.getState(); - const mailboxId = resolveFolderRef(link.ref, mailboxes); + const mailboxId = resolveFolderRef( + link.ref, + mailboxes, + link.ownerAccountId === activeClient.getAccountId() ? undefined : link.ownerAccountId, + ); if (!mailboxId) { toast.error(t('deep_link.folder_not_found')); return; diff --git a/integration/tests/20-push-owner.spec.ts b/integration/tests/20-push-owner.spec.ts new file mode 100644 index 000000000..b51725917 --- /dev/null +++ b/integration/tests/20-push-owner.spec.ts @@ -0,0 +1,75 @@ +import { test, expect } from '@playwright/test'; +import { ACCOUNTS, GROUP, WEBMAIL_URL } from './helpers/config'; +import { JmapClient } from './helpers/jmap'; +import { login, seedSettings, folderRow, expandSharedFolders, emailItem, addAccount, switchAccount } from './helpers/app'; +import { pushClick } from '../../lib/__tests__/helpers/push-click'; + +test.describe('notification target across personal and shared accounts (#1491)', () => { + const cases = [false, true].flatMap(shared => [1, 3].flatMap(unread => + [false, true].map(warm => ({ shared, unread, warm, secondary: false })), + )); + cases.push(...[1, 3].map(unread => ({ shared: true, unread, warm: true, secondary: true }))); + for (const { shared, unread, warm, secondary } of cases) { + test(`${shared ? 'shared' : 'personal'}, ${unread} unread, ${warm ? 'existing' : 'new'} window${secondary ? ', second login' : ''}`, async ({ page }) => { + const member = ACCOUNTS.carol; + const client = await JmapClient.connect(member.email, member.password); + const sharedId = client.accountIdByName(GROUP.team.email); + await client.reset(); + await client.reset(sharedId); + const targetId = shared ? sharedId : client.accountId; + let deliveredId = ''; + const subject = `Push target ${shared ? 'shared' : 'personal'} ${unread}`; + for (const owner of [client.accountId, sharedId]) { + const inbox = (await client.mailboxByRole('inbox', owner))!; + for (let i = 0; i < unread; i++) { + const result = await client.request([['Email/set', { + accountId: owner, + create: { mail: { + mailboxIds: { [inbox.id]: true }, keywords: {}, + from: [{ email: ACCOUNTS.alice.email }], + to: [{ email: owner === sharedId ? GROUP.team.email : member.email }], + subject: owner === targetId ? `${subject} ${i}` : `Other mailbox ${i}`, + receivedAt: new Date(Date.now() + i * 1000).toISOString(), + textBody: [{ partId: 'b', type: 'text/plain' }], + bodyValues: { b: { value: `Body in ${owner}` } }, + } }, + }, '0']]); + expect(result.methodResponses[0][1].created?.mail).toBeTruthy(); + if (owner === targetId) deliveredId = result.methodResponses[0][1].created.mail.id; + } + } + await seedSettings(page, { enableUnifiedMailbox: false, mailLayout: 'split' }); + if (secondary) { + await login(page, ACCOUNTS.bob); + await addAccount(page, member); + await switchAccount(page, ACCOUNTS.bob.email); + } else { + await login(page, member); + } + await expect(folderRow(page, { role: 'inbox', shared: false }).first()).toHaveAttribute('data-selected', 'true'); + const target = await pushClick(targetId, async (url) => { + const response = await page.request.get(new URL(url, WEBMAIL_URL).href); + expect(response.status()).toBe(200); + const preview = await response.json(); + expect(preview.unreadTotal).toBe(unread); + expect(preview.slot).toBe(secondary ? 1 : 0); + expect(preview.email.subject).toBe(`${subject} ${unread - 1}`); + return preview; + }, { warm, emailId: deliveredId }); + if (!warm) { + const nextPage = await page.context().newPage(); + await page.close(); + page = nextPage; + } + await page.goto(target); + if (unread === 1) { + await expect(page.locator('[data-tour="email-viewer"]')).toContainText(`${subject} 0`); + } + if (shared) await expandSharedFolders(page, GROUP.team.email); + await expect(folderRow(page, { role: 'inbox', shared }).first()).toHaveAttribute('data-selected', 'true'); + if (unread > 1) { + await expect(emailItem(page, `${subject} ${unread - 1}`).first()).toBeVisible(); + } + }); + } +}); diff --git a/lib/__tests__/helpers/push-click.ts b/lib/__tests__/helpers/push-click.ts new file mode 100644 index 000000000..7509a9c73 --- /dev/null +++ b/lib/__tests__/helpers/push-click.ts @@ -0,0 +1,57 @@ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; + +export async function pushClick( + accountId: string, + preview: (url: string) => Promise, + options: { warm?: boolean; basePath?: string; legacy?: boolean; emailId?: string } = {}, +): Promise { + const source = readFileSync(resolve('public/sw.js'), 'utf8'); + let notification: { data: unknown } | undefined; + let target: string | undefined; + const listeners = new Map void>(); + const cache = new Map(); + const client = { + id: 'window-1', url: 'https://mail.example.test/', + postMessage() {}, + async focus() { return this; }, + async navigate(url: string) { target = url; return this; }, + }; + const context = vm.createContext({ + URL, URLSearchParams, Response, AbortController, setTimeout, clearTimeout, console, + self: { + location: { href: `https://mail.example.test${options.basePath ?? ''}/sw.js`, origin: 'https://mail.example.test' }, + addEventListener: (type: string, fn: (event: unknown) => void) => listeners.set(type, fn), + clients: { + matchAll: async () => options.warm ? [client] : [], + openWindow: async (url: string) => { target = url; }, + }, + registration: { + showNotification: async (_title: string, data: { data: unknown }) => { notification = data; }, + getNotifications: async () => [], + }, + }, + caches: { open: async () => ({ + match: async (key: string) => cache.has(key) ? new Response(cache.get(key)) : undefined, + put: async (key: string, response: Response) => cache.set(key, await response.text()), + }) }, + fetch: async (url: string) => ({ ok: true, json: async () => preview(url) }), + }); + vm.runInContext(source, context); + async function dispatch(type: string, event: Record) { + const waits: Promise[] = []; + const listener = listeners.get(type); + if (!listener) throw new Error(`Missing worker handler: ${type}`); + listener({ ...event, waitUntil: (p: Promise) => waits.push(p) }); + await Promise.all(waits); + } + const payload = options.legacy + ? { changed: { [accountId]: { EmailDelivery: 'new' } } } + : { kind: 'jmap-email-push', accountId, emailIds: [options.emailId ?? 'new-email'] }; + await dispatch('push', { data: { json: () => payload } }); + if (!notification) throw new Error('No notification shown'); + await dispatch('notificationclick', { notification: { ...notification, close() {} } }); + if (!target) throw new Error('Notification did not navigate'); + return target; +} diff --git a/lib/__tests__/sw-push-owner.test.ts b/lib/__tests__/sw-push-owner.test.ts new file mode 100644 index 000000000..ace7cf58d --- /dev/null +++ b/lib/__tests__/sw-push-owner.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from 'vitest'; +import { parseMailPath, resolveFolderRef } from '@/lib/deep-links'; +import type { Mailbox } from '@/lib/jmap/types'; +import { pushClick } from './helpers/push-click'; + +const folders = [ + { id: 'a', name: 'Inbox', role: 'inbox', accountId: 'personal', totalEmails: 0, unreadEmails: 0 }, + { id: 'team:a', originalId: 'a', name: 'Inbox', role: 'inbox', isShared: true, accountId: 'team', totalEmails: 0, unreadEmails: 0 }, +] as Mailbox[]; + +function parseTarget(target: string) { + const url = new URL(target, 'https://mail.example.test'); + return parseMailPath(url.pathname.split('/mail/')[1].split('/'), url.searchParams); +} + +describe('push click keeps the owning mailbox (#1491)', () => { + for (const owner of ['personal', 'team']) { + for (const unreadTotal of [1, 3]) { + for (const warm of [false, true]) { + it(`${owner}, ${unreadTotal} unread, ${warm ? 'existing' : 'new'} window`, async () => { + const target = await pushClick(owner, async (url) => { + expect(new URL(url, 'https://mail.example.test').searchParams.get('accountId')).toBe(owner); + return { slot: 0, unreadTotal, email: { id: 'same-id', threadId: 't', subject: 'Mail' } }; + }, { warm }); + const link = parseTarget(target); + expect(link?.slot).toBe(0); + if (unreadTotal === 1) { + expect(link).toMatchObject({ kind: 'message', id: 'same-id', ownerAccountId: owner }); + } else { + expect(link).toMatchObject({ kind: 'folder', ref: 'inbox', ownerAccountId: owner }); + if (link?.kind !== 'folder') throw new Error('Expected folder link'); + expect(resolveFolderRef(link.ref, folders, link.ownerAccountId)).toBe(owner === 'team' ? 'team:a' : 'a'); + } + }); + } + } + } + + it('keeps the owner when organisation policy hides message details', async () => { + const target = await pushClick('team', async () => ({ slot: 2, unreadTotal: 1, redacted: true, email: { id: 'same-id' } })); + expect(parseTarget(target)).toMatchObject({ kind: 'message', ownerAccountId: 'team', slot: 2 }); + }); + + it('keeps the login slot and owner with a mount prefix and legacy payload', async () => { + const target = await pushClick('team', async () => ({ slot: 2, unreadTotal: 3, email: { id: 'same-id' } }), { basePath: '/webmail', legacy: true }); + expect(target).toMatch(/^\/webmail\/mail\/folder\/inbox\?/); + expect(parseTarget(target)).toMatchObject({ kind: 'folder', ownerAccountId: 'team', slot: 2 }); + }); + + it('does not fall back to a personal folder for an unavailable owner', () => { + expect(resolveFolderRef('inbox', folders, 'unavailable')).toBeNull(); + }); + + it('keeps old unscoped folder links working', () => { + expect(resolveFolderRef('inbox', folders)).toBe('a'); + }); +}); diff --git a/lib/deep-links.ts b/lib/deep-links.ts index e3c1459a4..483ad33a9 100644 --- a/lib/deep-links.ts +++ b/lib/deep-links.ts @@ -136,7 +136,7 @@ const ALIASED_ROLES = new Set(['inbox', 'sent', 'drafts', 'trash', 'archive', 'j * login by its cookie slot instead - what a push notification knows. */ export type MailDeepLink = - | { kind: 'folder'; ref: string; accountId?: string; slot?: number } + | { kind: 'folder'; ref: string; accountId?: string; slot?: number; ownerAccountId?: string } | { kind: 'message'; id: string; accountId?: string; slot?: number; fullscreen?: boolean; ownerAccountId?: string } | { kind: 'thread'; id: string; accountId?: string; slot?: number; ownerAccountId?: string }; @@ -179,9 +179,18 @@ export function buildFolderRef(mailboxId: string, mailboxes: Mailbox[] = []): st * Exact ids are checked before aliases so a custom folder whose id happens to * read like an alias still resolves to itself. */ -export function resolveFolderRef(ref: string, mailboxes: Mailbox[] = []): string | null { +export function resolveFolderRef(ref: string, mailboxes: Mailbox[] = [], ownerAccountId?: string): string | null { const decoded = decodeSegment(ref); + if (ownerAccountId) { + const owned = mailboxes.filter((mailbox) => mailbox.accountId === ownerAccountId); + const exact = owned.find((mailbox) => mailbox.id === decoded); + if (exact) return exact.id; + return ALIASED_ROLES.has(decoded) + ? owned.find((mailbox) => mailbox.role === decoded)?.id ?? null + : null; + } + if (mailboxes.some((m) => m.id === decoded)) return decoded; const virtual = VIRTUAL_FOLDER_ALIASES[decoded]; @@ -284,7 +293,7 @@ export function parseMailPath( if (id) { if (kind === 'message') return { kind: 'message', id, accountId, slot, fullscreen, ownerAccountId }; if (kind === 'thread') return { kind: 'thread', id, accountId, slot, ownerAccountId }; - if (kind === 'folder') return { kind: 'folder', ref: id, accountId, slot }; + if (kind === 'folder') return { kind: 'folder', ref: id, accountId, slot, ownerAccountId }; } } diff --git a/public/sw.js b/public/sw.js index a33068c02..975db0c27 100644 --- a/public/sw.js +++ b/public/sw.js @@ -521,7 +521,7 @@ async function handlePush(event) { // (Organisation > Devices): the route sent neither. title = accountLabel ? `New mail (${accountLabel})` : "New mail"; body = unreadTotal > 1 ? `${unreadTotal} unread messages` : "You have new mail"; - if (unreadTotal <= 1) data = { kind: "email", emailId: email.id, threadId: email.threadId, slot }; + if (unreadTotal <= 1) data = { kind: "email", emailId: email.id, threadId: email.threadId, accountId, slot }; } else if (email) { const sender = email.from && email.from[0]; const senderName = (sender && sender.name) || (sender && sender.email) || "New mail"; @@ -534,7 +534,7 @@ async function handlePush(event) { body += "\n" + (more === 1 ? "+1 more message" : `+${more} more messages`); } else { // Exactly one unread: deep-link straight to that message on click. - data = { kind: "email", emailId: email.id, threadId: email.threadId, slot }; + data = { kind: "email", emailId: email.id, threadId: email.threadId, accountId, slot }; } } else { title = accountLabel ? `New mail (${accountLabel})` : "New mail"; @@ -922,16 +922,17 @@ function getReusableClientScore(state) { function buildClickUrl(data) { if (!data) return `${BASE_PATH}/`; - // `?slot=` names the login the message belongs to (see the preview API). - const slotQuery = Number.isInteger(data.slot) ? `?slot=${data.slot}` : ""; + const params = new URLSearchParams(); + if (Number.isInteger(data.slot)) params.set("slot", String(data.slot)); + if (typeof data.accountId === "string" && data.accountId) params.set("owner", data.accountId); + const targetQuery = params.size ? `?${params}` : ""; if (data.kind === "email" && data.emailId) { // Permalink (#733). Under NEXT_PUBLIC_LOCALE_PREFIX=always the proxy // redirects this to the localised path; the worker has no locale to add. - return `${BASE_PATH}/mail/message/${encodeURIComponent(data.emailId)}${slotQuery}`; + return `${BASE_PATH}/mail/message/${encodeURIComponent(data.emailId)}${targetQuery}`; } - // A group of new mail in a known login: that login's Inbox. - if (data.kind === "mail-list" && slotQuery) { - return `${BASE_PATH}/mail/folder/inbox${slotQuery}`; + if (data.kind === "mail-list" && Number.isInteger(data.slot)) { + return `${BASE_PATH}/mail/folder/inbox${targetQuery}`; } // Generic "New mail" toast (preview API failed or returned no email): land // the user on the latest unread message in their Inbox rather than just the