diff --git a/src/lib/__tests__/contact-utils.test.ts b/src/lib/__tests__/contact-utils.test.ts index 0996391b..ac158b2a 100644 --- a/src/lib/__tests__/contact-utils.test.ts +++ b/src/lib/__tests__/contact-utils.test.ts @@ -1,6 +1,8 @@ import { describe, it, expect } from 'vitest'; import { + deriveFullName, getContactDisplayName, + getCustomFullName, getContactPhotoUri, normalizeContactPhotoUri, partialDateToString, @@ -80,3 +82,43 @@ describe('stringToPartialDate', () => { } }); }); + +describe('name.full for the vCard FN (#430)', () => { + const person = (full: string | undefined, components: Array<{ kind: string; value: string }>): ContactCard => ({ + id: 'c', + addressBookIds: {}, + name: { components, isOrdered: true, ...(full !== undefined ? { full } : {}) }, + }); + + it('joins the name components in display order, like the webmail form', () => { + expect(deriveFullName([ + { kind: 'surname', value: 'Doe' }, + { kind: 'generation', value: 'Jr.' }, + { kind: 'given', value: ' John ' }, + { kind: 'title', value: 'Dr.' }, + { kind: 'given2', value: 'Q' }, + ])).toBe('Dr. John Q Doe Jr.'); + // Legacy vCard-style kinds count too. + expect(deriveFullName([{ kind: 'prefix', value: 'Ms.' }, { kind: 'given', value: 'Ann' }])).toBe('Ms. Ann'); + expect(deriveFullName(undefined)).toBe(''); + expect(deriveFullName([])).toBe(''); + }); + + it('treats a full that repeats the components as derived, so an edit cannot leave it stale', () => { + const card = person('John Doe', [{ kind: 'given', value: 'John' }, { kind: 'surname', value: 'Doe' }]); + expect(getCustomFullName(card)).toBe(''); + }); + + it('keeps a display name of its own', () => { + const card = person('Johnny D', [{ kind: 'given', value: 'John' }, { kind: 'surname', value: 'Doe' }]); + expect(getCustomFullName(card)).toBe('Johnny D'); + expect(getCustomFullName(person('Solo', []))).toBe('Solo'); + expect(getCustomFullName(person(undefined, [{ kind: 'given', value: 'A' }]))).toBe(''); + }); + + it('treats an organization card whose full is the organization name as derived', () => { + const org = person('Acme Corp', []); + expect(getCustomFullName(org, 'Acme Corp')).toBe(''); + expect(getCustomFullName(org, 'Acme Inc')).toBe('Acme Corp'); + }); +}); diff --git a/src/lib/__tests__/vcard.test.ts b/src/lib/__tests__/vcard.test.ts index 96e62dc5..da74bca2 100644 --- a/src/lib/__tests__/vcard.test.ts +++ b/src/lib/__tests__/vcard.test.ts @@ -8,6 +8,25 @@ import { import type { ContactCard } from '../../api/types'; describe('parseVCard', () => { + it('keeps FN as name.full so the written card carries the mandatory FN (#430)', () => { + const fnOnly = parseVCard('BEGIN:VCARD\r\nVERSION:3.0\r\nFN:John Doe\r\nEND:VCARD'); + expect(fnOnly[0].name?.full).toBe('John Doe'); + + // FN before N: N replaces the components but must keep full. + const fnFirst = parseVCard('BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Dr. John Doe Jr.\r\nN:Doe;John;;;\r\nEND:VCARD'); + expect(fnFirst[0].name?.full).toBe('Dr. John Doe Jr.'); + + // N before FN: FN fills full on the existing name. + const nFirst = parseVCard('BEGIN:VCARD\r\nVERSION:3.0\r\nN:Doe;John;;;\r\nFN:Dr. John Doe Jr.\r\nEND:VCARD'); + expect(nFirst[0].name?.full).toBe('Dr. John Doe Jr.'); + expect(nFirst[0].name?.components?.find((c) => c.kind === 'given')?.value).toBe('John'); + }); + + it('derives name.full from N when a card has no FN (#430)', () => { + const [card] = parseVCard('BEGIN:VCARD\r\nVERSION:3.0\r\nN:Doe;John;Q;Dr.;Jr.\r\nEND:VCARD'); + expect(card.name?.full).toBe('Dr. John Q Doe Jr.'); + }); + it('parses a basic 3.0 card with name, email, and phone', () => { const vcf = [ 'BEGIN:VCARD', diff --git a/src/lib/contact-utils.ts b/src/lib/contact-utils.ts index 989e6416..09d8bf04 100644 --- a/src/lib/contact-utils.ts +++ b/src/lib/contact-utils.ts @@ -1,6 +1,7 @@ import type { ContactCard, AnniversaryDate, + NameComponent, PartialDate, Timestamp, } from '../api/types'; @@ -59,6 +60,39 @@ export function getPrimaryTitle(contact: ContactCard): string { return title?.name || ''; } +/** + * `name.full` derived from the name components in display order (prefix, + * given, middle, surname, suffix), as the webmail's form writes it. Every + * write should carry `full`: the vCard FN is built from it, FN is mandatory + * (RFC 6350 ยง6.2.1), and strict CardDAV clients such as Apple Contacts drop + * cards without it (#430). + */ +export function deriveFullName(components: readonly NameComponent[] | undefined): string { + const find = (...kinds: string[]) => + components?.find((c) => kinds.includes(c.kind))?.value?.trim() || ''; + return [ + find('title', 'prefix'), + find('given'), + find('given2', 'additional', 'middle'), + find('surname'), + find('generation', 'suffix'), + ].filter(Boolean).join(' '); +} + +/** + * The card's `name.full` when it is a display name of its own. Empty when it + * only repeats what `deriveFullName` (or, on an organization card, the + * organization name) gives, so an edit form re-derives it from the edited + * fields on save instead of writing back a stale copy (#430). + */ +export function getCustomFullName(contact: ContactCard, orgName?: string): string { + const full = contact.name?.full?.trim() || ''; + if (!full) return ''; + if (full === deriveFullName(contact.name?.components)) return ''; + if (orgName && full === orgName.trim()) return ''; + return full; +} + export function getContactInitials(contact: ContactCard): string { const name = getContactDisplayName(contact).trim(); if (!name) return '?'; diff --git a/src/lib/vcard.ts b/src/lib/vcard.ts index f3f99860..71704d9e 100644 --- a/src/lib/vcard.ts +++ b/src/lib/vcard.ts @@ -6,6 +6,7 @@ import type { PartialDate, } from '../api/types'; import { generateUUID } from './uuid'; +import { deriveFullName } from './contact-utils'; // Convert RFC 9553 AnniversaryDate (PartialDate|Timestamp|string) to vCard date string function anniversaryDateToVcardString(date: AnniversaryDate): string { @@ -573,6 +574,8 @@ function buildContact(raw: Record): ContactCard | null { switch (propName) { case 'FN': + // Keep the verbatim FN in `full` (RFC 9553): it feeds the mandatory + // vCard FN on re-export, which strict clients require (#430). if (!card.name) { const parts = val.split(' '); const components: NameComponent[] = []; @@ -582,7 +585,9 @@ function buildContact(raw: Record): ContactCard | null { } else if (parts.length === 1) { components.push({ kind: 'given', value: parts[0] }); } - card.name = { components, isOrdered: true }; + card.name = { components, isOrdered: true, full: val }; + } else if (!card.name.full) { + card.name.full = val; } break; @@ -599,7 +604,8 @@ function buildContact(raw: Record): ContactCard | null { if (nParts[0]) components.push({ kind: 'surname', value: nParts[0] }); if (nParts[4]) components.push({ kind: 'generation', value: nParts[4] }); if (components.length > 0) { - card.name = { components, isOrdered: true }; + // Preserve `full` captured from FN when FN preceded N. + card.name = { ...card.name, components, isOrdered: true }; } break; } @@ -1162,6 +1168,13 @@ function buildContact(raw: Record): ContactCard | null { } } + // A card without FN (invalid, but seen in the wild) still gets a `full`, + // derived from N like the contact form does, so the server writes an FN. + if (card.name && !card.name.full) { + const derived = deriveFullName(card.name.components); + if (derived) card.name.full = derived; + } + const hasName = (card.name && (card.name.components?.length ?? 0) > 0) || !!card.name?.full; const hasEmail = card.emails && Object.keys(card.emails).length > 0; // An organization name identifies the card just as well as a personal name. diff --git a/src/screens/ContactFormScreen.tsx b/src/screens/ContactFormScreen.tsx index 2abc38ea..5815fb8f 100644 --- a/src/screens/ContactFormScreen.tsx +++ b/src/screens/ContactFormScreen.tsx @@ -23,6 +23,7 @@ import { useContactsStore, selectGroupMembers } from '../stores/contacts-store'; import { getContactKeywords, getContactDisplayName, getContactPrimaryEmail, normalizeContactPhotoUri, partialDateToString, stringToPartialDate, + deriveFullName, getCustomFullName, } from '../lib/contact-utils'; import { splitMailbox } from '../lib/rfc5322-mailbox'; import Dialog from '../components/Dialog'; @@ -146,7 +147,6 @@ function contactToForm(contact: ContactCard, memberIds: string[]): FormState { const middle = findNameComponent(contact, 'given2', 'additional', 'middle'); const surname = findNameComponent(contact, 'surname'); const suffix = findNameComponent(contact, 'generation', 'suffix'); - const full = contact.name?.full || ''; const nicknames = contact.nicknames ? Object.values(contact.nicknames).map((n) => n.name || '').filter(Boolean) @@ -232,6 +232,9 @@ function contactToForm(contact: ContactCard, memberIds: string[]): FormState { const isOrg = contact.kind ? contact.kind === 'org' : !(given || surname) && !!rawOrgs[0]?.name; + // Only a display name of its own goes in the field; a derived one is + // derived again on save, so it can't go stale when the name changes. + const full = getCustomFullName(contact, isOrg ? rawOrgs[0]?.name : undefined); return { isOrg, prefix, given, middle, surname, suffix, full, @@ -285,9 +288,10 @@ function formToPatch( if (form.suffix.trim()) components.push({ kind: 'generation', value: form.suffix.trim() }); } - // Without personal name components, carry the organization name in - // `name.full` so servers and other clients have something to display. - const full = form.full.trim() || (form.isOrg ? orgName : ''); + // Always send `name.full`: the vCard FN is built from it and is mandatory + // (#430). Without personal name components, carry the organization name + // so servers and other clients have something to display. + const full = form.full.trim() || (form.isOrg ? orgName : deriveFullName(components)); const name: ContactCard['name'] | undefined = components.length > 0 || full ? { ...(components.length > 0 ? { components, isOrdered: true } : {}), ...(full ? { full } : {}) }