fix: swap the signature when you pick a From address without one

components/email/__tests__/compose-format.test.ts

@@ -3,6 +3,8 @@ import {
   buildEmbeddedSignatureHtml,
   htmlComposeBodyToPlainText,
   plainComposeBodyToHtml,
+  replaceEmbeddedSignature,
+  replacePlainTextSignature,
   stripEmbeddedSignature,
 } from '../compose-format';
 import { SIGNATURE_BLOCK_MARKER, SIGNATURE_RANGE_MARKER, containsEmbeddedSignature } from '../signature-block';
@@ -90,3 +92,52 @@ describe('compose-format (#1022): HTML <-> plain text switch', () => {
     expect(count(html, '-- ')).toBe(1);
   });
 });
+
+describe('switching identity swaps the signature', () => {
+  const other = { htmlSignature: '<i>Team</i>', textSignature: 'Team' };
+  const none = { htmlSignature: '', textSignature: '' };
+  const embed = (sig: typeof identity) => buildEmbeddedSignatureHtml(sig, { embed: true, separator: true });
+  // The DOM writes valueless attributes as =""; compare in its spelling.
+  const dom = (html: string) => new DOMParser().parseFromString(html, 'text/html').body.innerHTML;
+
+  describe('replaceEmbeddedSignature', () => {
+    it('puts the new signature where the old one was', () => {
+      const html = `<p>Hi</p>${embed(identity)}<blockquote>original</blockquote>`;
+      expect(replaceEmbeddedSignature(html, embed(other)))
+        .toBe(dom(`<p>Hi</p>${embed(other)}<blockquote>original</blockquote>`));
+    });
+
+    it('removes the signature for an identity without one', () => {
+      const html = `<p>Hi</p>${embed(identity)}<blockquote>original</blockquote>`;
+      expect(replaceEmbeddedSignature(html, embed(none))).toBe('<p>Hi</p><blockquote>original</blockquote>');
+    });
+
+    it('adds a signature above the quote when the body had none', () => {
+      expect(replaceEmbeddedSignature('<p>Hi</p><blockquote>original</blockquote>', embed(other)))
+        .toBe(dom(`<p>Hi</p>${embed(other)}<blockquote>original</blockquote>`));
+    });
+
+    it('adds a signature at the end when there is no quote', () => {
+      expect(replaceEmbeddedSignature('<p>Hi</p>', embed(other))).toBe(dom(`<p>Hi</p>${embed(other)}`));
+    });
+  });
+
+  describe('replacePlainTextSignature', () => {
+    const signed = 'Hi\n\n-- \nAlice\nAcme';
+
+    it('swaps a trailing signature', () => {
+      expect(replacePlainTextSignature(signed, identity, other, { separator: true })).toBe('Hi\n\n-- \nTeam');
+    });
+
+    it('removes it for an identity without one and adds it back', () => {
+      const bare = replacePlainTextSignature(signed, identity, none, { separator: true });
+      expect(bare).toBe('Hi');
+      expect(replacePlainTextSignature(bare, none, identity, { separator: true })).toBe(signed);
+    });
+
+    it('leaves a body alone whose signature the user edited', () => {
+      const edited = 'Hi\n\n-- \nAlice (edited)';
+      expect(replacePlainTextSignature(edited, identity, other, { separator: true })).toBe(edited);
+    });
+  });
+});

components/email/__tests__/composer-identity-signature.test.tsx

@@ -0,0 +1,259 @@
+import { render, screen, fireEvent, waitFor } from '@testing-library/react';
+import { describe, it, expect, vi, afterEach } from 'vitest';
+import React from 'react';
+import { EmailComposer } from '../email-composer';
+import { useSettingsStore } from '@/stores/settings-store';
+
+// ─── Heavy component mocks (mirrors composer-format-toggle.test.tsx) ─
+
+vi.mock('@/components/email/rich-text-editor', () => ({
+  RichTextEditor: () => React.createElement('div', { 'data-testid': 'rich-text-editor' }),
+}));
+
+vi.mock('@/components/plugins/plugin-slot', () => ({ PluginSlot: () => null }));
+vi.mock('@/components/identity/sub-address-helper', () => ({ SubAddressHelper: () => null }));
+vi.mock('@/components/templates/template-picker', () => ({ TemplatePicker: () => null }));
+vi.mock('@/components/templates/template-form', () => ({ TemplateForm: () => null }));
+vi.mock('@/components/files/file-preview-modal', () => ({ FilePreviewModal: () => null }));
+vi.mock('@/hooks/use-focus-trap', () => ({
+  useFocusTrap: () => ({ current: null }),
+}));
+vi.mock('@/hooks/use-pro-multi-account-identities', () => ({
+  useProMultiAccountIdentities: () => ({ enabled: false, groups: [], allIdentities: [] }),
+  stripCrossAccountIdentityPrefix: (id: string) => ({ localAccountId: null, rawId: id }),
+}));
+
+// ─── Store mocks ──────────────────────────────────────────────────────────────
+
+vi.mock('@/stores/auth-store', () => {
+  const state = {
+    client: null,
+    identities: [],
+    primaryIdentity: null,
+    isAuthenticated: false,
+    isDemoMode: false,
+    activeAccountId: null,
+    connectionLost: false,
+    getClientForAccount: () => undefined,
+    getAllConnectedClients: () => new Map(),
+    syncIdentities: () => {},
+    refreshIdentities: async () => {},
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useAuthStore: hook };
+});
+
+vi.mock('@/stores/identity-store', () => {
+  const state = {
+    identities: [
+      { id: 'id-me', email: 'me@example.com', name: 'Me', htmlSignature: '<b>Alice</b>', textSignature: 'Alice' },
+      { id: 'id-team', email: 'team@example.com', name: 'Team', htmlSignature: '', textSignature: '' },
+    ] as Array<Record<string, unknown>>,
+    defaultIdentityId: 'id-me',
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useIdentityStore: hook, notifyIdentitySelected: () => {} };
+});
+
+vi.mock('@/stores/account-store', () => {
+  const state = { accounts: [], getAccountById: () => undefined };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useAccountStore: hook };
+});
+
+vi.mock('@/stores/email-store', () => {
+  const state = {
+    draftSaveEnabled: false,
+    sendRawEmail: async () => ({ sent: true }),
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useEmailStore: hook };
+});
+
+vi.mock('@/stores/settings-store', () => {
+  const state = {
+    timeFormat: '24h',
+    plainTextMode: false,
+    subAddressDelimiter: '+',
+    autoSelectReplyIdentity: true,
+    attachmentReminderEnabled: false,
+    attachmentReminderKeywords: [],
+    emptySubjectWarningEnabled: true,
+    sendDelaySeconds: 0,
+    // The default position: replies/forwards get the signature appended at
+    // send time rather than embedded above the quote.
+    signaturePosition: 'below_quote',
+    signatureSeparatorEnabled: true,
+    requestReadReceiptDefault: false,
+    addTrustedSender: () => {},
+    trustedSendersAddressBook: null,
+    updateSetting: () => {},
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useSettingsStore: hook };
+});
+
+vi.mock('@/stores/contact-store', () => {
+  const state = {
+    contacts: [],
+    getAutocomplete: async () => [],
+    addToTrustedSendersBook: async () => {},
+  };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useContactStore: hook };
+});
+
+vi.mock('@/stores/template-store', () => {
+  const state = { templates: [], addTemplate: async () => {} };
+  const hook = (sel?: (s: typeof state) => unknown) =>
+    typeof sel === 'function' ? sel(state) : state;
+  hook.getState = () => state;
+  hook.setState = (p: Partial<typeof state>) => Object.assign(state, p);
+  return { useTemplateStore: hook };
+});
+
+// ─── Misc dependency mocks ────────────────────────────────────────────────────
+
+vi.mock('@/stores/toast-store', () => ({
+  toast: { info: () => {}, error: () => {}, success: () => {} },
+}));
+
+vi.mock('@/lib/plugin-hooks', () => ({
+  identityHooks: {
+    onSignatureRender: { size: 0, transform: async (value: unknown) => value },
+    onIdentitySelect: { size: 0, emit: async () => {} },
+  },
+  templateHooks: {
+    onTemplateRender: { size: 0, transform: async (value: unknown) => value },
+    onTemplateApply: { size: 0, emit: async () => {} },
+  },
+  emailHooks: {
+    onComposerOpen: { call: async () => [] },
+    onRecipientChange: { call: async () => [] },
+    getRecipientSuggestions: { call: async () => [] },
+    onRecipientChipsChange: { transform: async (chips: unknown) => chips },
+    onDraftChange: { emit: () => {} },
+    onBeforeDraftAutoSave: { transform: async (draft: unknown) => draft },
+    onBeforeEmailSend: { intercept: async () => true, interceptOutcome: async () => 'allowed' },
+    onComposeSend: { takeOver: async () => 'default' },
+    onTransformOutgoingEmail: { transform: async (email: unknown) => email },
+  },
+  contactHooks: {
+    search: { call: async () => [] },
+    onProvideRecipientSuggestions: { transform: async (initial: unknown) => initial },
+  },
+}));
+
+vi.mock('@/lib/email-sanitization', () => ({
+  sanitizeSignatureHtml: (v: string) => v,
+  sanitizeSignatureHtmlForDisplay: (v: string) => v,
+  sanitizeEmailHtml: (v: string) => v,
+  sanitizePluginBodyHtml: (v: string) => v,
+  escapeHtml: (v: string) => v,
+  parseHtmlSafely: (html: string) => new DOMParser().parseFromString(html, 'text/html'),
+}));
+
+vi.mock('@/lib/email-threading', () => ({
+  computeReplyThreadingHeaders: () => ({ inReplyTo: [], references: [] }),
+}));
+vi.mock('@/lib/sub-addressing', () => ({ generateSubAddress: () => '' }));
+vi.mock('@/lib/debug', () => ({ debug: { log: () => {}, warn: () => {}, error: () => {} } }));
+vi.mock('@/components/email/quoted-html', () => ({
+  buildQuotedHtmlBlock: () => '',
+  serializeEditorContent: () => '',
+}));
+vi.mock('@/lib/template-utils', () => ({ substitutePlaceholders: (s: string) => s }));
+
+// ─── Tests ────────────────────────────────────────────────────────────────────
+
+/**
+ * Switching From to an identity without a signature kept the previous
+ * identity's signature, so a mail from a team address went out with the
+ * personal signature. An identity the composer picks itself (a reply to an
+ * alias) still borrows the primary signature.
+ */
+
+const DRAFT = {
+  to: 'bob@example.com',
+  cc: '',
+  bcc: '',
+  subject: 'Hello',
+  body: '<p>Draft text</p>',
+  showCc: false,
+  showBcc: false,
+  selectedIdentityId: 'id-me',
+  subAddressTag: '',
+  mode: 'compose' as const,
+  draftId: null,
+};
+
+const fromSelect = () => screen.getByTestId('composer-from') as HTMLSelectElement;
+const sendButton = () => screen.getAllByTestId('composer-send')[0] as HTMLButtonElement;
+const textarea = () => document.querySelector('textarea') as HTMLTextAreaElement;
+
+async function send(onSend: ReturnType<typeof vi.fn>) {
+  fireEvent.click(sendButton());
+  await waitFor(() => expect(onSend).toHaveBeenCalledTimes(1));
+  return onSend.mock.calls[0][0] as { body: string; htmlBody?: string };
+}
+
+describe('composer signature follows the picked identity', () => {
+  afterEach(() => {
+    useSettingsStore.setState({ plainTextMode: false });
+    vi.clearAllMocks();
+  });
+
+  it('sends a reply from a hand-picked identity without the previous signature', async () => {
+    const onSend = vi.fn();
+    render(<EmailComposer mode="reply" initialData={{ ...DRAFT, mode: 'reply' }} onSend={onSend} />);
+    fireEvent.change(fromSelect(), { target: { value: 'id-team' } });
+
+    const sent = await send(onSend);
+    expect(sent.htmlBody).not.toContain('Alice');
+    expect(sent.body).not.toContain('Alice');
+  });
+
+  it('still borrows the primary signature for an identity it did not let the user pick', async () => {
+    const onSend = vi.fn();
+    render(<EmailComposer mode="reply" initialData={{ ...DRAFT, mode: 'reply', selectedIdentityId: 'id-team' }} onSend={onSend} />);
+
+    const sent = await send(onSend);
+    expect(sent.htmlBody).toContain('<b>Alice</b>');
+  });
+
+  it('swaps the signature in a plain-text message and brings it back', async () => {
+    const SIGNED = 'Plain draft\n\n-- \nAlice';
+    useSettingsStore.setState({ plainTextMode: true });
+    const onSend = vi.fn();
+    render(<EmailComposer initialData={{ ...DRAFT, body: SIGNED }} onSend={onSend} />);
+
+    fireEvent.change(fromSelect(), { target: { value: 'id-team' } });
+    await waitFor(() => expect(textarea().value).toBe('Plain draft'));
+
+    fireEvent.change(fromSelect(), { target: { value: 'id-me' } });
+    await waitFor(() => expect(textarea().value).toBe(SIGNED));
+
+    fireEvent.change(fromSelect(), { target: { value: 'id-team' } });
+    await waitFor(() => expect(textarea().value).toBe('Plain draft'));
+    const sent = await send(onSend);
+    expect(sent.body).toBe('Plain draft');
+  });
+});

components/email/compose-format.ts

@@ -107,6 +107,47 @@ export function stripEmbeddedSignature(html: string): { html: string; hadSignatu
   return { html: doc.body.innerHTML, hadSignature: true };
 }
 
+/**
+ * The HTML body with its embedded signature swapped for `signatureHtml` (from
+ * buildEmbeddedSignatureHtml; empty for an identity without a signature).
+ * The new signature goes where the old one was. A body without one - the
+ * previous identity had none - gets it before the quote, or at the end when
+ * there is no quote, which is where getInitialBody would have put it.
+ */
+export function replaceEmbeddedSignature(html: string, signatureHtml: string): string {
+  const doc = new DOMParser().parseFromString(html, 'text/html');
+  let removed = removeSignatureRange(doc);
+  if (!removed) {
+    if (!signatureHtml) return html;
+    const quote = Array.from(doc.body.childNodes).find(isQuoteBoundary) ?? null;
+    removed = { parent: doc.body, insertBefore: quote };
+  }
+  const host = doc.createElement('div');
+  host.innerHTML = signatureHtml;
+  Array.from(host.childNodes).forEach((node) =>
+    removed.parent.insertBefore(node, removed.insertBefore)
+  );
+  return doc.body.innerHTML;
+}
+
+/**
+ * The plain-text body with the previous identity's trailing signature swapped
+ * for the next one's. A body that doesn't end with the previous signature was
+ * edited there (or never had one) and is only given the new signature when
+ * the previous identity had none.
+ */
+export function replacePlainTextSignature(
+  body: string,
+  previous: SignatureIdentityLike,
+  next: SignatureIdentityLike,
+  options: { separator: boolean },
+): string {
+  const hadSignature = plainTextBodyHasSignature(body, previous);
+  if (!hadSignature && (previous?.htmlSignature || previous?.textSignature)) return body;
+  const bare = hadSignature ? plainTextBodyWithoutSignature(body, previous) : body;
+  return appendPlainTextSignature(bare, next, options);
+}
+
 /**
  * Convert the rich-text editor's body to a plain-text body. Formatting is
  * dropped (that is the point of plain text); an embedded signature comes back

components/email/email-composer.tsx

@@ -55,7 +55,8 @@ import {
   buildEmbeddedSignatureHtml,
   htmlComposeBodyToPlainText,
   plainComposeBodyToHtml,
-  removeSignatureRange,
+  replaceEmbeddedSignature,
+  replacePlainTextSignature,
 } from "@/components/email/compose-format";
 import { emailHooks, contactHooks, isExternalAttachmentResult } from "@/lib/plugin-hooks";
 import { onUploadProgress } from "@/lib/upload-progress";
@@ -794,6 +795,9 @@ export function EmailComposer({
   const [validationErrors, setValidationErrors] = useState<{ to?: boolean; body?: boolean }>({});
   const [shakeField, setShakeField] = useState<string | null>(null);
   const [selectedIdentityId, setSelectedIdentityId] = useState<string | null>(initialData?.selectedIdentityId ?? null);
+  // Set once the user picks From by hand. Their pick then brings its own
+  // signature, also when it has none (see ownSignatureIdentity).
+  const [identityPickedByUser, setIdentityPickedByUser] = useState(false);
   const [subAddressTag, setSubAddressTag] = useState<string>(initialData?.subAddressTag ?? '');
   const [fromOverrideEnabled, setFromOverrideEnabled] = useState<boolean>(initialData?.fromOverrideEnabled ?? false);
   const [fromOverrideEmail, setFromOverrideEmail] = useState<string>(initialData?.fromOverrideEmail ?? '');
@@ -944,8 +948,10 @@ export function EmailComposer({
     : null;
   // Alias identities often lack a configured signature - fall back to the primary
   // identity's signature so replies (which auto-select a matching alias) still
-  // populate the user's signature.
-  const ownSignatureIdentity = (currentIdentity?.htmlSignature || currentIdentity?.textSignature)
+  // populate the user's signature. An identity the user picked by hand keeps
+  // its own, also an empty one: switching to a team address without a
+  // signature sent the team mail with the personal signature.
+  const ownSignatureIdentity = identityPickedByUser || currentIdentity?.htmlSignature || currentIdentity?.textSignature
     ? currentIdentity
     : primaryIdentity;
   // Plugins may rewrite the signature (onSignatureRender).
@@ -961,46 +967,48 @@ export function EmailComposer({
   const prevSignatureIdentityIdRef = useRef<string | null | undefined>(signatureIdentity?.id);
   const prevSignatureSeparatorRef = useRef<boolean>(signatureSeparatorEnabled);
   const prevPluginSignatureRevisionRef = useRef(pluginSignatureRevision);
+  const prevSignatureIdentityRef = useRef(signatureIdentity);
 
   useEffect(() => {
     const editor = editorRef.current;
     const identityChanged = prevSignatureIdentityIdRef.current !== signatureIdentity?.id;
     const separatorChanged = prevSignatureSeparatorRef.current !== signatureSeparatorEnabled;
     const pluginSignatureChanged = prevPluginSignatureRevisionRef.current !== pluginSignatureRevision;
+    const previousIdentity = prevSignatureIdentityRef.current;
     prevSignatureIdentityIdRef.current = signatureIdentity?.id;
     prevSignatureSeparatorRef.current = signatureSeparatorEnabled;
     prevPluginSignatureRevisionRef.current = pluginSignatureRevision;
-    if (!editor) return;
+    prevSignatureIdentityRef.current = signatureIdentity;
     if (!identityChanged && !separatorChanged && !pluginSignatureChanged) return;
-    if (plainTextMode) return;
     // Replies/forwards only embed when configured for "above quote". Compose
     // always embeds (see getInitialBody), so swap on identity change there too.
     const isReplyLike = mode === 'reply' || mode === 'replyAll' || mode === 'forward';
     if (!isReplyLike && mode !== 'compose') return;
     if (isReplyLike && signaturePosition !== 'above_quote') return;
 
+    if (plainTextMode) {
+      // A new plain-text message carries the signature at the end of the
+      // body; in a reply the quote follows it.
+      if (!identityChanged || mode !== 'compose') return;
+      setBody((prev) => replacePlainTextSignature(prev, previousIdentity, signatureIdentity, {
+        separator: signatureSeparatorEnabled,
+      }));
+      return;
+    }
+    if (!editor) return;
+
     // serializeEditorContent (not getHTML) so a QuotedHtml island's verbatim
     // body isn't lost during the signature splice + setContent round-trip.
+    // The old range is replaced in place (bounded so it never eats into the
+    // quoted body - see removeSignatureRange), or removed when the new
+    // identity has no signature.
     const currentHtml = serializeEditorContent(editor);
-    const doc = new DOMParser().parseFromString(currentHtml, 'text/html');
-    const newSignature = buildEmbeddedSignatureHtml(signatureIdentity, {
+    // Only a new identity brings a signature back into a body without one.
+    if (!identityChanged && !containsEmbeddedSignature(currentHtml)) return;
+    const nextHtml = replaceEmbeddedSignature(currentHtml, buildEmbeddedSignatureHtml(signatureIdentity, {
       embed: true,
       separator: signatureSeparatorEnabled,
-    });
-    if (!newSignature) return;
-
-    // Remove the existing signature range (bounded so it never eats into the
-    // quoted body - see removeSignatureRange) and splice the new one in at the
-    // same position, without re-serializing/parsing twice.
-    const removed = removeSignatureRange(doc);
-    if (!removed) return;
-    const replacementHost = doc.createElement('div');
-    replacementHost.innerHTML = newSignature;
-    Array.from(replacementHost.childNodes).forEach((node) =>
-      removed.parent.insertBefore(node, removed.insertBefore)
-    );
-
-    const nextHtml = doc.body.innerHTML;
+    }));
     if (nextHtml !== currentHtml) {
       editor.commands.setContent(nextHtml, { emitUpdate: true });
     }
@@ -3543,6 +3551,7 @@ export function EmailComposer({
                   value={selectedIdentityId || primaryIdentity?.id || ''}
                   onChange={(e) => {
                     setSelectedIdentityId(e.target.value);
+                    setIdentityPickedByUser(true);
                     notifyIdentitySelected(identities, e.target.value);
                   }}
                   className="flex-1 bg-transparent text-sm text-foreground outline-none cursor-pointer hover:text-muted-foreground transition-colors min-w-0 truncate"