fix: open a shared or group account's message from its link instead of the own message with the same id #1351

This commit is contained in:
Linus Rath
2026-10-26 20:01:11 +01:00
parent af536ac90a
commit 9de2efb722
9 changed files with 195 additions and 43 deletions
+3 -3
View File
@@ -3,7 +3,7 @@
import { useMemo } from "react";
import { useTranslations } from "next-intl";
import { useSettingsStore } from "@/stores/settings-store";
import { useEmailStore, removalFolder, canRemoveFromFolder } from "@/stores/email-store";
import { useEmailStore, removalFolder, canRemoveFromFolder, emailLinkSource } from "@/stores/email-store";
import { Email, Mailbox } from "@/lib/jmap/types";
import {
ContextMenu,
@@ -353,7 +353,7 @@ export function EmailContextMenu({
icon={LinkIcon}
label={tDeepLink("copy_message")}
onClick={() => handleAction(() => {
void copyLink(buildMailPath({ mailboxId: null, emailId: email.id, threadId: null }));
void copyLink(buildMailPath({ mailboxId: null, emailId: email.id, threadId: null, ...emailLinkSource(email) }));
})}
/>
{email.threadId && (
@@ -361,7 +361,7 @@ export function EmailContextMenu({
icon={MessagesSquare}
label={tDeepLink("copy_conversation")}
onClick={() => handleAction(() => {
void copyLink(buildMailPath({ mailboxId: null, emailId: null, threadId: email.threadId! }));
void copyLink(buildMailPath({ mailboxId: null, emailId: null, threadId: email.threadId!, ...emailLinkSource(email) }));
})}
/>
)}
+2 -2
View File
@@ -111,7 +111,7 @@ import { useDeviceDetection } from "@/hooks/use-media-query";
import { useIsPaneScoped } from "@/hooks/use-pane-context";
import { useAuthStore } from "@/stores/auth-store";
import { useAccountStore } from "@/stores/account-store";
import { useEmailStore, resolveEmailBlobContext, emailWriteSource, type EmailBlobContext } from "@/stores/email-store";
import { useEmailStore, resolveEmailBlobContext, emailWriteSource, emailLinkSource, type EmailBlobContext } from "@/stores/email-store";
import { useThemeStore } from "@/stores/theme-store";
import { EmailIdentityBadge } from "./email-identity-badge";
import { UnsubscribeBanner } from "./unsubscribe-banner";
@@ -4461,7 +4461,7 @@ export function EmailViewer({
<button
role="menuitem"
onClick={() => {
void copyLink(buildMailPath({ mailboxId: null, emailId: email.id, threadId: null }));
void copyLink(buildMailPath({ mailboxId: null, emailId: email.id, threadId: null, ...emailLinkSource(email) }));
setMoreMenuOpen(false);
setMoreMenuSub(null);
}}
+43 -24
View File
@@ -37,7 +37,7 @@ import type { UnifiedAccountClient } from "@/lib/unified-mailbox";
import { connectedAccountsGrew } from "@/lib/unified-mailbox";
import { messagePriorityFromHeaders, type MessagePriority } from "@/lib/message-priority";
import { KeyboardShortcutsModal } from "@/components/keyboard-shortcuts-modal";
import { useEmailStore, pendingInitialData, buildUnifiedAccountClients, invalidateUnifiedMailboxes, captureViewToken, ArchiveMailboxNotFoundError, ensureArchiveMailbox, resolveSelectedEmails, resolveUnstampedEmailAccountId, resolveEmailBlobContext, resolveEmailActionContext, emailWriteSource, type EmailBlobContext, emptyFolderMovesToTrash, bulkDeleteIsPermanent, shouldConfirmBulkDelete, isAggregateListView, removalFolder, canRemoveFromFolder } from "@/stores/email-store";
import { useEmailStore, pendingInitialData, buildUnifiedAccountClients, invalidateUnifiedMailboxes, captureViewToken, ArchiveMailboxNotFoundError, ensureArchiveMailbox, resolveSelectedEmails, resolveUnstampedEmailAccountId, resolveEmailBlobContext, resolveEmailActionContext, emailWriteSource, emailLinkSource, type EmailBlobContext, emptyFolderMovesToTrash, bulkDeleteIsPermanent, shouldConfirmBulkDelete, isAggregateListView, removalFolder, canRemoveFromFolder } from "@/stores/email-store";
import { findDraftIdentityIdForLogin, listRowSource, resolveDraftSource, type DraftSource } from "@/lib/draft-source";
import { useRemoveFromFolder } from "@/hooks/use-remove-from-folder";
import { groupSearchScopeFolders, SEARCH_SCOPE_ALL_FOLDERS } from "@/lib/search-scope-folders";
@@ -830,15 +830,23 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
const buildMailUrl = useCallback((state: NavSnapshot) => {
if (isEmbedded || proInterfaceActive) return null;
const path = appPath(
const open = useEmailStore.getState().selectedEmail;
const shown = open && (open.id === state.emailId || (!!state.threadId && open.threadId === state.threadId)) ? open : null;
// The shared account of the open message, so a reload opens it again
// (#1351). Not another login's: a reload must not switch the login.
const source = shown ? emailLinkSource(shown) : null;
return appPath(
buildMailPath(
{ mailboxId: state.mailboxId, emailId: state.emailId, threadId: state.threadId },
{
mailboxId: state.mailboxId, emailId: state.emailId, threadId: state.threadId,
ownerAccountId: source && !source.loginId ? source.ownerAccountId : null,
// Keep the marker while fullscreen is active, so reloading the dragged-out
// tab reopens the message fullscreen instead of in the normal layout.
fullscreen: fullscreenReading,
},
useEmailStore.getState().mailboxes,
),
);
// Keep the marker while fullscreen is active, so reloading the dragged-out
// tab reopens the message fullscreen instead of in the normal layout.
return fullscreenReading && state.emailId ? `${path}?view=fullscreen` : path;
}, [isEmbedded, proInterfaceActive, fullscreenReading]);
useBrowserNavigation({
@@ -1700,6 +1708,22 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
const deepLinkHandledRef = useRef(false);
const mailAppMountedRef = useRef(false);
useEffect(() => { mailAppMountedRef.current = true; return () => { mailAppMountedRef.current = false; }; }, []);
// Point the list at a folder that actually holds a linked message, so the
// pane behind it shows real context and "back" lands somewhere useful.
const showInOwningFolder = async (activeClient: Parameters<typeof fetchEmails>[0], email: Email) => {
const state = useEmailStore.getState();
const owning = Object.keys(email.mailboxIds ?? {}).find((id) =>
state.mailboxes.some((mb) => mb.id === id || mb.originalId === id),
);
if (owning && owning !== state.selectedMailbox) {
const mailbox = state.mailboxes.find((mb) => mb.id === owning || mb.originalId === owning);
if (mailbox) {
await fetchEmails(activeClient, mailbox.id, { background: true });
selectMailbox(mailbox.id);
selectEmail(email);
}
}
};
const applyMailDeepLink = async (link: MailDeepLink, opts?: { onLoad?: boolean }) => {
// The folder list before an account switch, if one happened: until the
// new mailbox's folders arrive the store still holds these (or nothing).
@@ -1767,8 +1791,12 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
// In the Pro shell a message permalink opens as its own fullscreen
// email tab - the tab body fetches the message and retitles itself.
if (isEmbedded) {
// A shared account's message is fetched from that account through
// the active login (#1351).
const loginId = link.ownerAccountId ? useAuthStore.getState().activeAccountId ?? undefined : undefined;
useProTabStore.getState().openEmailTab({
accountId: '',
accountId: link.ownerAccountId ?? '',
...(loginId ? { clientAccountId: loginId } : {}),
emailId: link.id,
mailboxId: null,
title: t('common.loading'),
@@ -1777,25 +1805,12 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
}
setLoadingEmail(true);
try {
const email = await fetchEmailContent(activeClient, link.id);
const email = await fetchEmailContent(activeClient, link.id, link.ownerAccountId);
if (!email) {
toast.error(t('deep_link.message_not_found'));
return;
}
// Point the list at a folder that actually holds the message, so the
// pane behind it shows real context and "back" lands somewhere useful.
const state = useEmailStore.getState();
const owning = Object.keys(email.mailboxIds ?? {}).find((id) =>
state.mailboxes.some((mb) => mb.id === id || mb.originalId === id),
);
if (owning && owning !== state.selectedMailbox) {
const mailbox = state.mailboxes.find((mb) => mb.id === owning || mb.originalId === owning);
if (mailbox) {
await fetchEmails(activeClient, mailbox.id, { background: true });
selectMailbox(mailbox.id);
selectEmail(email);
}
}
await showInOwningFolder(activeClient, email);
if (link.fullscreen && !isMobile) {
// The owning-folder branch above may have skipped selecting (the
// folder was already current) - fullscreen needs the message open.
@@ -1816,10 +1831,11 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
const openSeq = ++conversationOpenSeqRef.current;
const source: EmailBlobContext = {
client: activeClient, clientAccountId: useAuthStore.getState().activeAccountId ?? undefined,
accountId: link.ownerAccountId,
};
setLoadingEmail(true);
try {
const threadEmails = await activeClient.getThreadEmails(link.id);
const threadEmails = await activeClient.getThreadEmails(link.id, link.ownerAccountId);
if (openSeq !== conversationOpenSeqRef.current) return;
const [group] = groupEmailsByThread(threadEmails);
if (!group) {
@@ -1832,7 +1848,10 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) {
setConversationEmails(threadEmails);
setActiveView('viewer');
} else {
await fetchEmailContent(activeClient, group.latestEmail.id);
const email = await fetchEmailContent(activeClient, group.latestEmail.id, link.ownerAccountId);
// A shared account's conversation needs its folder open: the open
// message is acted on in the account of the folder on screen.
if (email && link.ownerAccountId) await showInOwningFolder(activeClient, email);
if (isTablet) setTabletListVisible(false);
}
} finally {
+2 -2
View File
@@ -3,7 +3,7 @@
import { useCallback, useEffect, useMemo, useRef, DragEvent } from "react";
import { Email } from "@/lib/jmap/types";
import { IJMAPClient } from "@/lib/jmap/client-interface";
import { resolveEmailBlobContext, resolveSelectedEmails, useEmailStore } from "@/stores/email-store";
import { emailLinkSource, resolveEmailBlobContext, resolveSelectedEmails, useEmailStore } from "@/stores/email-store";
import { emailKeyFor } from "@/lib/thread-utils";
import { useAuthStore } from "@/stores/auth-store";
import { useDragDropContext } from "@/contexts/drag-drop-context";
@@ -280,7 +280,7 @@ export function useEmailDrag({ email, sourceMailboxId, threadEmails }: UseEmailD
e.dataTransfer.setData(
"text/uri-list",
emailsToDrag
.map(em => `${appUrl(buildMailPath({ mailboxId: null, emailId: em.id, threadId: null }))}?view=fullscreen`)
.map(em => appUrl(buildMailPath({ mailboxId: null, emailId: em.id, threadId: null, ...emailLinkSource(em), fullscreen: true })))
.join("\r\n")
);
+18
View File
@@ -242,6 +242,24 @@ describe('mail paths', () => {
expect(link).toEqual({ kind: 'message', id: 'm1', accountId: 'acc-2' });
});
// Email ids repeat across accounts: a link to a shared account's message
// names that account, or it opens the own message with the same id (#1351).
it('names the account owning a message or conversation outside the own one', () => {
const message = buildMailPath({ mailboxId: null, emailId: 'm1', threadId: null, ownerAccountId: 'cn', loginId: 'bob@x', fullscreen: true });
expect(message).toBe('/mail/message/m1?account=bob%40x&owner=cn&view=fullscreen');
const [, query] = message.split('?');
expect(parseMailPath(['message', 'm1'], new URLSearchParams(query))).toEqual({
kind: 'message', id: 'm1', accountId: 'bob@x', ownerAccountId: 'cn', fullscreen: true,
});
expect(buildMailPath({ mailboxId: null, emailId: null, threadId: 't1', ownerAccountId: 'cn' })).toBe('/mail/thread/t1?owner=cn');
expect(parseMailPath(['thread', 't1'], new URLSearchParams('owner=cn'))).toEqual({
kind: 'thread', id: 't1', accountId: undefined, ownerAccountId: 'cn',
});
// Only a message link opens fullscreen; a folder link names no owner.
expect(buildMailPath({ mailboxId: null, emailId: null, threadId: 't1', fullscreen: true })).toBe('/mail/thread/t1');
expect(buildMailPath({ mailboxId: 'c', emailId: null, threadId: null, ownerAccountId: 'cn' }, MAILBOXES)).toBe('/mail/folder/c');
});
it('still understands the legacy ?email= notification link', () => {
expect(parseMailPath([], new URLSearchParams('email=m9'))).toEqual({
kind: 'message',
+24 -6
View File
@@ -137,13 +137,22 @@ const ALIASED_ROLES = new Set(['inbox', 'sent', 'drafts', 'trash', 'archive', 'j
*/
export type MailDeepLink =
| { kind: 'folder'; ref: string; accountId?: string; slot?: number }
| { kind: 'message'; id: string; accountId?: string; slot?: number; fullscreen?: boolean }
| { kind: 'thread'; id: string; accountId?: string; slot?: number };
| { kind: 'message'; id: string; accountId?: string; slot?: number; fullscreen?: boolean; ownerAccountId?: string }
| { kind: 'thread'; id: string; accountId?: string; slot?: number; ownerAccountId?: string };
export interface MailLinkState {
mailboxId: string | null;
emailId: string | null;
threadId: string | null;
/**
* The JMAP account a linked message or conversation lives in, when it is not
* the login's own: email ids repeat across accounts (#1351).
*/
ownerAccountId?: string | null;
/** The login (AccountEntry.id) reaching it, when not the active one. */
loginId?: string | null;
/** Open a message link alone, without sidebar and list. */
fullscreen?: boolean;
}
/**
@@ -227,8 +236,16 @@ export function isFolderLinkOpen(mailboxId: string, view: OpenFolderView): boole
* thread is what the user is looking at.
*/
export function buildMailPath(state: MailLinkState, mailboxes: Mailbox[] = []): string {
if (state.threadId) return `/mail/thread/${encodeSegment(state.threadId)}`;
if (state.emailId) return `/mail/message/${encodeSegment(state.emailId)}`;
const query = (fullscreen: boolean) => {
const params = new URLSearchParams();
if (state.loginId) params.set('account', state.loginId);
if (state.ownerAccountId) params.set('owner', state.ownerAccountId);
if (fullscreen) params.set('view', 'fullscreen');
const qs = params.toString();
return qs ? `?${qs}` : '';
};
if (state.threadId) return `/mail/thread/${encodeSegment(state.threadId)}${query(false)}`;
if (state.emailId) return `/mail/message/${encodeSegment(state.emailId)}${query(!!state.fullscreen)}`;
if (state.mailboxId) return `/mail/folder/${encodeSegment(buildFolderRef(state.mailboxId, mailboxes))}`;
return '/mail';
}
@@ -259,13 +276,14 @@ export function parseMailPath(
// a mail dragged out into a new browser tab opens as. The Pro shell always
// opens message links as fullscreen email tabs and ignores the flag.
const fullscreen = search?.get('view') === 'fullscreen' || undefined;
const ownerAccountId = search?.get('owner') || undefined;
const [kind, value] = segments;
if (kind && value) {
const id = decodeSegment(value);
if (id) {
if (kind === 'message') return { kind: 'message', id, accountId, slot, fullscreen };
if (kind === 'thread') return { kind: 'thread', id, accountId, slot };
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 };
}
}
+6 -2
View File
@@ -95,9 +95,13 @@ export async function openHit(hit: GlobalSearchHit, options: OpenHitOptions): Pr
// Standard layout: switch the active login when needed, then navigate.
if (!isActiveLogin) await auth.switchAccount(hit.localAccountId);
switch (hit.kind) {
case 'mail':
navigate?.(appPath(buildMailPath({ mailboxId: null, emailId: hit.id, threadId: null })));
case 'mail': {
// A group or shared account's hit names its account: ids repeat (#1351).
const own = auth.getClientForAccount(hit.localAccountId)?.getAccountId();
const ownerAccountId = hit.jmapAccountId && hit.jmapAccountId !== own ? hit.jmapAccountId : null;
navigate?.(appPath(buildMailPath({ mailboxId: null, emailId: hit.id, threadId: null, ownerAccountId })));
return;
}
case 'contacts':
// Outside the Pro shell the store holds the active login's contacts raw.
navigate?.(appPath(buildContactsPath({ contactId: hit.id })));
@@ -0,0 +1,71 @@
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { emailLinkSource, useEmailStore } from '../email-store';
import { useAuthStore } from '../auth-store';
import type { Email, Mailbox } from '@/lib/jmap/types';
import type { IJMAPClient } from '@/lib/jmap/client-interface';
// Email ids repeat across accounts (Stalwart numbers each account's mail the
// same way). A message link only named the id, so a link to a shared or group
// account's message opened the own message with that id (#1351).
const row = (id: string, mailboxId: string): Email => ({
id, threadId: `t-${id}`, mailboxIds: { [mailboxId]: true }, keywords: {}, subject: `Subject ${id}`,
receivedAt: '2026-10-01T00:00:00Z', from: [], to: [], preview: '', size: 1, hasAttachment: false,
} as Email);
const mailbox = (id: string, extra: Partial<Mailbox> = {}): Mailbox => ({
id, originalId: id, name: id, role: null, accountId: 'own', totalEmails: 1, unreadEmails: 0,
totalThreads: 1, unreadThreads: 0, sortOrder: 0, isSubscribed: true, myRights: {}, ...extra,
} as unknown as Mailbox);
const OWN_INBOX = mailbox('inbox', { role: 'inbox' });
const GROUP_INBOX = mailbox('grp:inbox', { originalId: 'inbox', role: 'inbox', accountId: 'grp', isShared: true });
function client() {
return {
getAccountId: () => 'own',
getEmail: vi.fn(async (id: string, accountId?: string) => row(id, accountId ? `${accountId}:inbox` : 'inbox')),
} as unknown as IJMAPClient & { getEmail: ReturnType<typeof vi.fn> };
}
describe('message links and the account that owns the message', () => {
let c: ReturnType<typeof client>;
beforeEach(() => {
c = client();
useAuthStore.setState({ client: c, activeAccountId: 'alice@example.org' } as never);
useEmailStore.setState({
mailboxes: [OWN_INBOX, GROUP_INBOX],
accountMailboxes: {},
viewingAccountId: null,
selectedMailbox: 'inbox',
selectedEmail: null,
emails: [row('e1', 'inbox')],
searchQuery: '',
});
});
it('fetches a linked message from the account the link names, not the own row with its id', async () => {
const email = await useEmailStore.getState().fetchEmailContent(c, 'e1', 'grp');
expect(c.getEmail).toHaveBeenCalledWith('e1', 'grp');
expect(email?.mailboxIds).toEqual({ 'grp:inbox': true });
});
it('fetches from the listed row\'s account without an owner', async () => {
await useEmailStore.getState().fetchEmailContent(c, 'e1');
expect(c.getEmail).toHaveBeenCalledWith('e1', undefined);
});
it('names the owner of a row in a shared folder, and none for the own one', () => {
expect(emailLinkSource(row('e1', 'inbox'))).toEqual({ ownerAccountId: null, loginId: null });
useEmailStore.setState({ selectedMailbox: 'grp:inbox' });
expect(emailLinkSource(row('e1', 'grp:inbox'))).toEqual({ ownerAccountId: 'grp', loginId: null });
});
it('names the account and login a stamped row of a merged view comes from', () => {
const fromBob = { ...row('e1', 'inbox'), sourceClientAccountId: 'bob@example.org', sourceAccountId: 'b1' };
const bobClient = { getAccountId: () => 'b1' };
useAuthStore.setState({ getClientForAccount: (id: string) => (id === 'bob@example.org' ? bobClient : null) } as never);
expect(emailLinkSource(fromBob)).toEqual({ ownerAccountId: null, loginId: 'bob@example.org' });
expect(emailLinkSource({ ...fromBob, sourceAccountId: 'b-grp' })).toEqual({ ownerAccountId: 'b-grp', loginId: 'bob@example.org' });
});
});
+26 -4
View File
@@ -335,7 +335,8 @@ interface EmailStore {
// times; later calls are no-ops while a prior one is in flight.
prefetchInitialData: (client: IJMAPClient) => Promise<void>;
loadMoreEmails: (client: IJMAPClient) => Promise<void>;
fetchEmailContent: (client: IJMAPClient, emailId: string) => Promise<Email | null>;
/** `ownerAccountId`: the JMAP account a link names for the message (#1351). */
fetchEmailContent: (client: IJMAPClient, emailId: string, ownerAccountId?: string) => Promise<Email | null>;
fetchQuota: (client: IJMAPClient) => Promise<void>;
sendEmail: (client: IJMAPClient, to: string[], subject: string, body: string, cc?: string[], bcc?: string[], identityId?: string, fromEmail?: string, draftId?: string, fromName?: string, htmlBody?: string, attachments?: Array<{ blobId: string; name: string; type: string; size: number; disposition?: 'attachment' | 'inline'; cid?: string }>, inReplyTo?: string[], references?: string[], delayedUntil?: string, envelopeMailFrom?: string, options?: { requestReadReceipt?: boolean; requestDsn?: boolean; requireTls?: boolean; priority?: MessagePriority; localAccountId?: string; draftAccountId?: string }) => Promise<SendEmailResult>;
sendRawEmail: (client: IJMAPClient, rawMimeBlob: Blob, identityId: string, delayedUntil?: string, envelopeRecipients?: string[]) => Promise<SendEmailResult>;
@@ -1235,6 +1236,20 @@ export function emailWriteSource(email: Pick<Email, 'sourceClientAccountId' | 's
return { clientAccountId, accountId };
}
/**
* What a link to a row in the view on screen names besides its id, as ids
* repeat across accounts and logins (#1351): the JMAP account when it is not
* the login's own, the login when it is not the active one.
*/
export function emailLinkSource(email: Pick<Email, 'sourceClientAccountId' | 'sourceAccountId'>): { ownerAccountId: string | null; loginId: string | null } {
const auth = useAuthStore.getState();
const { client, accountId, clientAccountId } = resolveEmailBlobContext(email, auth.client);
return {
ownerAccountId: accountId && accountId !== client?.getAccountId() ? accountId : null,
loginId: clientAccountId && clientAccountId !== auth.activeAccountId ? clientAccountId : null,
};
}
// List requests can finish after navigation. Never apply an old folder/tag
// response (or error) to the view the user has since selected.
/**
@@ -3831,15 +3846,22 @@ export const useEmailStore = create<EmailStore>((set, get) => ({
}
},
fetchEmailContent: async (client, emailId) => {
fetchEmailContent: async (client, emailId, ownerAccountId) => {
try {
// Route to the owning account. In aggregate views (All Mail, unified,
// cross-account) the selected mailbox is virtual, so derive the client +
// accountId from the email itself (handles shared/group accounts); fall
// back to the selected-mailbox shared-folder logic for normal views.
const selected = get().selectedEmail;
const listEmail = selected?.id === emailId ? selected : get().emails.find(e => e.id === emailId);
const { client: actionClient, accountId } = resolveEmailBlobContext(listEmail ?? null, client);
let listEmail = selected?.id === emailId ? selected : get().emails.find(e => e.id === emailId);
let { client: actionClient, accountId } = resolveEmailBlobContext(listEmail ?? null, client);
// A link names the account of a message outside the login's own; a
// listed row with its id from another account is not that message.
if (ownerAccountId && accountId !== ownerAccountId) {
listEmail = undefined;
actionClient = client;
accountId = ownerAccountId;
}
if (!actionClient) throw new Error('No connected client for email source');
const email = await actionClient.getEmail(emailId, accountId);