diff --git a/docs/parity/10-webmail-1.10-to-1.18.md b/docs/parity/10-webmail-1.10-to-1.18.md index 73d0191c..f374015f 100644 --- a/docs/parity/10-webmail-1.10-to-1.18.md +++ b/docs/parity/10-webmail-1.10-to-1.18.md @@ -70,7 +70,7 @@ Stalwart 1.0.0 items found in the same pass (1.0 SSO scopes, Files on 1.0, #1385 - What RN does: clearAllFeatureStores (`src/stores/auth-store.ts:126-134`) never clears `src/stores/search-history-store.ts:44`. - Fix hint: clear it, and check the other persisted stores in the same pass. -- [x] **Internationalized domains** — `P3` — `missing` — fixed in f5875f87 (sign-in, discovery, account rows) and 4932764a (the composer takes recipients on Unicode domains and sends them in ASCII form; the From identity and a sender on one of your own domains show as written in the composer and the reader; unit tests only, the test servers have no IDN domain); left: the sender in the mail list rows (EmailListScreen, UnifiedInboxScreen) still shows the xn-- form on your own domains +- [x] **Internationalized domains** — `P3` — `missing` — fixed in f5875f87 (sign-in, discovery, account rows) and 4932764a (the composer takes recipients on Unicode domains and sends them in ASCII form; the From identity and a sender on one of your own domains show as written in the composer and the reader; unit tests only, the test servers have no IDN domain); left: the sender in the mail list rows (EmailListScreen, UnifiedInboxScreen) still shows the xn-- form on your own domains — list rows in b35d15ca - What WEB does: `lib/idn.ts:25` (#1100). What RN does: ASCII-only checks (`src/lib/recipients.ts:31`, `src/lib/server-discovery.ts:78`). - Fix hint: toASCII on the wire, Unicode on screen. @@ -411,7 +411,7 @@ Stalwart 1.0.0 items found in the same pass (1.0 SSO scopes, Files on 1.0, #1385 - What WEB does: retries read-only batches only (`lib/jmap/client.ts:245`). - What RN does: blocks the retry only for EmailSubmission/set, Email/import, Blob/upload (`src/api/jmap-client.ts:102-109`, `:853-857`). -- [x] **Recover deleted items (#1319)** — `P3` — `missing` — fixed in 113e96bd (Settings page only; the admin day limit and hiding wait for the org policy and plug in at `src/lib/recovery/policy.ts`; the Trash folder's "Recover deleted items" link is not ported, the store's `checkRecent` is ready for it) +- [x] **Recover deleted items (#1319)** — `P3` — `missing` — fixed in 113e96bd (Settings page only; the admin day limit and hiding wait for the org policy and plug in at `src/lib/recovery/policy.ts`; the Trash folder's "Recover deleted items" link is not ported, the store's `checkRecent` is ready for it) — Trash link in 7467764e - What WEB does: `x:ArchivedItem/query` (`lib/recovery/archived-items.ts:139`, `lib/jmap/client.ts:12119`, `components/settings/recover-deleted-settings.tsx`). - What RN does: nothing (`src/screens/SettingsScreen.tsx:112`). The day limit and hiding need the policy. diff --git a/src/components/email/TrashRecoveryLink.tsx b/src/components/email/TrashRecoveryLink.tsx new file mode 100644 index 00000000..210689e7 --- /dev/null +++ b/src/components/email/TrashRecoveryLink.tsx @@ -0,0 +1,72 @@ +import React from 'react'; +import { Pressable, StyleSheet, Text, View } from 'react-native'; +import { useNavigation } from '@react-navigation/native'; +import type { NativeStackNavigationProp } from '@react-navigation/native-stack'; +import { RotateCcw } from '../icons'; +import { spacing, typography, type ThemePalette } from '../../theme/tokens'; +import { useColors } from '../../theme/colors'; +import { useLocaleStore } from '../../stores/locale-store'; +import { setPendingSettingsTab } from '../../navigation/pending-settings-tab'; +import { recoveryPolicy } from '../../lib/recovery/policy'; +import { useRecoveryStore } from '../../lib/recovery/recovery-store'; +import { recoveryClientFor, useDeletedItemsRecovery } from '../../lib/recovery/use-deleted-items-recovery'; +import { isOwnTrash, trashRecoveryLinkVisible } from '../../lib/recovery/trash-link'; +import type { Mailbox } from '../../api/types'; +import type { RootStackParamList } from '../../navigation/types'; + +/** + * A line in the Trash folder's header that leads to Settings > Recover + * deleted items (webmail components/email/trash-recovery-link.tsx): only + * when the server offers it for this account and its archive holds at least + * one message inside the days window, so it never leads to an empty page. + * Only in the account's own Trash: a shared or group Trash has an archive of + * its own that the page doesn't show. + */ +export function TrashRecoveryLink({ mailbox }: { mailbox: Mailbox | undefined }) { + const c = useColors(); + const styles = React.useMemo(() => makeStyles(c), [c]); + const t = useLocaleStore((s) => s.t); + const navigation = useNavigation>(); + const { available, accountId, key } = useDeletedItemsRecovery(); + const { days } = recoveryPolicy(); + const recent = useRecoveryStore((s) => (key ? s.recent[key] : undefined)); + const checkRecent = useRecoveryStore((s) => s.checkRecent); + const ownTrash = isOwnTrash(mailbox, accountId); + + React.useEffect(() => { + if (!available || !ownTrash || !accountId || !key) return; + void checkRecent(recoveryClientFor(accountId), key, days); + }, [available, ownTrash, accountId, key, days, checkRecent]); + + if (!trashRecoveryLinkVisible({ available, ownTrash, recent, days })) return null; + const open = () => { + setPendingSettingsTab('recovery'); + navigation.navigate('MainTabs', { screen: 'Settings' } as never); + }; + return ( + + + {t('recover_deleted.trash_hint', 'Deleted for good by mistake?')} + + + + {t('recover_deleted.trash_link', 'Recover deleted items')} + + + ); +} + +const makeStyles = (c: ThemePalette) => StyleSheet.create({ + row: { + flexDirection: 'row', + alignItems: 'center', + gap: spacing.sm, + marginHorizontal: spacing.lg, + marginBottom: spacing.sm, + paddingHorizontal: spacing.md, + paddingVertical: spacing.xs, + }, + hint: { ...typography.caption, color: c.textSecondary, flex: 1, minWidth: 0 }, + link: { flexDirection: 'row', alignItems: 'center', gap: 4, paddingVertical: 4 }, + linkText: { ...typography.captionMedium, color: c.primary }, +}); diff --git a/src/lib/__tests__/list-sender.test.ts b/src/lib/__tests__/list-sender.test.ts new file mode 100644 index 00000000..98b785b8 --- /dev/null +++ b/src/lib/__tests__/list-sender.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import { listAddressLabel } from '../list-sender'; +import { toUnicodeEmailOnOwnDomains } from '../idn'; + +// A sender with no display name on one of the user's own IDN domains showed +// as punycode in the list rows (`info@xn--bcher-kva.de`), while the message +// header already read `info@bücher.de` (webmail thread-list-item.tsx, #1100). + +const own = ['me@xn--bcher-kva.de']; +const format = (address: string) => toUnicodeEmailOnOwnDomains(address, own); + +describe('listAddressLabel', () => { + it('reads a bare address on an own IDN domain as written, and leaves others alone', () => { + expect(listAddressLabel({ email: 'info@xn--bcher-kva.de' }, format, 'Unknown')).toBe('info@bücher.de'); + expect(listAddressLabel({ email: 'info@xn--mnchen-3ya.de' }, format, 'Unknown')).toBe('info@xn--mnchen-3ya.de'); + }); + + it('prefers the display name and falls back to the unknown label', () => { + expect(listAddressLabel({ name: 'Bücher', email: 'info@xn--bcher-kva.de' }, format, 'Unknown')).toBe('Bücher'); + expect(listAddressLabel(undefined, format, 'Unknown')).toBe('Unknown'); + expect(listAddressLabel({ name: '', email: '' }, format, 'Unknown')).toBe('Unknown'); + }); +}); + +describe('list rows', () => { + const read = (file: string) => fs.readFileSync(path.resolve(__dirname, '../../screens', file), 'utf8'); + + it('name senders and recipients through the own-domain formatter', () => { + const list = read('EmailListScreen.tsx'); + // Read once by the screen and handed to the memoised rows. + expect(list).toMatch(/const formatAddress = useOwnDomainAddress\(\);/); + expect(list).toMatch(/formatAddress=\{formatAddress\}/); + expect(list.match(/listAddressLabel\([^)]*formatAddress/g)).toHaveLength(2); + expect(list).not.toMatch(/\.name \|\| [\w.?[\]]*\.email/); + + const unified = read('UnifiedInboxScreen.tsx'); + expect(unified).toMatch(/const formatAddress = useOwnDomainAddress\(\);/); + expect(unified.match(/listAddressLabel\([^)]*formatAddress/g)).toHaveLength(2); + expect(unified.match(/senderName\(item, [^)]*\), formatAddress\)/g)).toHaveLength(2); + expect(unified).not.toMatch(/\.name \|\| [\w.?[\]]*\.email/); + }); +}); diff --git a/src/lib/__tests__/settings-search.test.ts b/src/lib/__tests__/settings-search.test.ts index 1e93829f..e741f187 100644 --- a/src/lib/__tests__/settings-search.test.ts +++ b/src/lib/__tests__/settings-search.test.ts @@ -101,6 +101,8 @@ describe('settings search index (English catalog)', () => { about: 'AboutSettings.tsx', debug: 'TroubleshootingSettings.tsx', updates: 'UpdatesSettings.tsx', + sharing: 'DelegateSettings.tsx', + recovery: 'RecoverDeletedSettings.tsx', }; for (const [tab, file] of Object.entries(PANE_FILES) as [SettingsTabId, string][]) { diff --git a/src/lib/list-sender.ts b/src/lib/list-sender.ts new file mode 100644 index 00000000..96582684 --- /dev/null +++ b/src/lib/list-sender.ts @@ -0,0 +1,13 @@ +/** + * How a mail list row names an address: its display name, else the address + * itself, formatted by the caller (useOwnDomainAddress: the user's own IDN + * domains read as written, `bücher.de`), else the "unknown" label (webmail + * components/email/thread-list-item.tsx, #1100). + */ +export function listAddressLabel( + address: { name?: string | null; email?: string | null } | undefined, + formatAddress: (address: string) => string, + unknown: string, +): string { + return address?.name || (address?.email ? formatAddress(address.email) : '') || unknown; +} diff --git a/src/lib/recovery/__tests__/trash-link.test.ts b/src/lib/recovery/__tests__/trash-link.test.ts new file mode 100644 index 00000000..ac4bd802 --- /dev/null +++ b/src/lib/recovery/__tests__/trash-link.test.ts @@ -0,0 +1,93 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import { isOwnTrash, trashRecoveryLinkVisible } from '../trash-link'; +import { useRecoveryStore, type RecentCheck, type RecoveryClient } from '../recovery-store'; +import type { Mailbox } from '../../../api/types'; + +// The Trash header leads to Settings > Recover deleted items (webmail +// components/email/trash-recovery-link.tsx and its test): only in the +// account's own Trash, when the server offers recovery, and when the archive +// holds a message inside the days window it was looked up for. + +const TRASH = { id: 't', name: 'Trash', role: 'trash', accountId: 'ci' } as Mailbox; +const HAS: RecentCheck = { has: true, days: 14, checkedAt: 0 }; + +function visible(over: Partial[0]> = {}): boolean { + return trashRecoveryLinkVisible({ available: true, ownTrash: true, recent: HAS, days: 14, ...over }); +} + +afterEach(() => useRecoveryStore.getState().reset()); + +describe('isOwnTrash', () => { + it("is the account's own Trash, with or without an account id on the folder", () => { + expect(isOwnTrash(TRASH, 'ci')).toBe(true); + expect(isOwnTrash({ ...TRASH, accountId: undefined }, 'ci')).toBe(true); + }); + + it("is not another folder, a shared or group account's Trash, or anything before the session", () => { + expect(isOwnTrash({ ...TRASH, role: 'inbox' }, 'ci')).toBe(false); + expect(isOwnTrash({ ...TRASH, accountId: 'ch', isShared: true }, 'ci')).toBe(false); + expect(isOwnTrash({ ...TRASH, accountId: undefined, isShared: true }, 'ci')).toBe(false); + expect(isOwnTrash({ ...TRASH, accountId: 'ch' }, 'ci')).toBe(false); + expect(isOwnTrash(undefined, 'ci')).toBe(false); + expect(isOwnTrash(TRASH, null)).toBe(false); + }); +}); + +describe('trashRecoveryLinkVisible', () => { + it('shows when recovery is offered and the archive has a recent message', () => { + expect(visible()).toBe(true); + }); + + it('hides when the archive holds nothing inside the days, or has not been looked at', () => { + expect(visible({ recent: { ...HAS, has: false } })).toBe(false); + expect(visible({ recent: undefined })).toBe(false); + }); + + it('hides when the server refuses, outside the own Trash, or when the look was for other days', () => { + expect(visible({ available: false })).toBe(false); + expect(visible({ ownTrash: false })).toBe(false); + expect(visible({ days: 30 })).toBe(false); + }); + + it("follows the store's look at the archive, and hides again after a restore", async () => { + const client: RecoveryClient = { + advertisesArchivedItems: () => true, + queryArchivedItemIds: vi.fn(async () => ({ ids: [], total: null }) as never), + hasArchivedEmail: vi.fn(async () => true), + }; + const key = 'acc-alice|ci'; + await useRecoveryStore.getState().checkRecent(client, key, 14); + expect(client.hasArchivedEmail).toHaveBeenCalledWith({ days: 14 }); + expect(visible({ recent: useRecoveryStore.getState().recent[key] })).toBe(true); + useRecoveryStore.getState().forgetRecent(key); + expect(visible({ recent: useRecoveryStore.getState().recent[key] })).toBe(false); + }); +}); + +describe('Trash link wiring', () => { + const src = (rel: string) => fs.readFileSync(path.resolve(__dirname, '../../..', rel), 'utf8'); + + it('is mounted in the list header only when the open folder is the Trash', () => { + const list = src('screens/EmailListScreen.tsx'); + expect(list).toMatch(/\{currentMailbox\?\.role === 'trash' && \}/); + }); + + it("looks at the open folder's account and opens Settings > Recover deleted items", () => { + const link = src('components/email/TrashRecoveryLink.tsx'); + expect(link).toMatch(/const \{ available, accountId, key \} = useDeletedItemsRecovery\(\);/); + expect(link).toMatch(/const ownTrash = isOwnTrash\(mailbox, accountId\);/); + expect(link).toMatch(/if \(!available \|\| !ownTrash \|\| !accountId \|\| !key\) return;/); + expect(link).toMatch(/checkRecent\(recoveryClientFor\(accountId\), key, days\)/); + expect(link).toMatch(/if \(!trashRecoveryLinkVisible\(\{ available, ownTrash, recent, days \}\)\) return null;/); + expect(link).toMatch(/setPendingSettingsTab\('recovery'\);\s*navigation\.navigate\('MainTabs', \{ screen: 'Settings' \}/); + expect(link).toMatch(/t\('recover_deleted\.trash_hint', /); + expect(link).toMatch(/t\('recover_deleted\.trash_link', /); + // A selector that builds a new object or array each time loops the render. + expect(link).not.toMatch(/Store\(\(s\) => \(\{/); + expect(link).not.toMatch(/Store\(\(s\) => [^;]*\.(filter|map)\(/); + // The tab the link parks is one Settings opens. + expect(src('screens/SettingsScreen.tsx')).toMatch(/\{ id: 'recovery', [^}]*implemented: true\s*\}/); + }); +}); diff --git a/src/lib/recovery/trash-link.ts b/src/lib/recovery/trash-link.ts new file mode 100644 index 00000000..f695a2e7 --- /dev/null +++ b/src/lib/recovery/trash-link.ts @@ -0,0 +1,29 @@ +import type { Mailbox } from '../../api/types'; +import type { RecentCheck } from './recovery-store'; + +/** + * The account's own Trash, the one whose archive Settings > Recover deleted + * items shows (webmail components/email/trash-recovery-link.tsx). A shared + * or group Trash has an archive of its own that the page doesn't show. + * `accountId` is the JMAP account the page is for (useDeletedItemsRecovery). + */ +export function isOwnTrash(mailbox: Mailbox | undefined, accountId: string | null): boolean { + return !!accountId && mailbox?.role === 'trash' && !mailbox.isShared + && (!mailbox.accountId || mailbox.accountId === accountId); +} + +/** + * Whether the Trash header leads to Recover deleted items: the server offers + * it for this account and its archive holds a message inside the days + * window, looked up for that same window, so the link never opens an empty + * page. + */ +export function trashRecoveryLinkVisible(options: { + available: boolean; + ownTrash: boolean; + recent: RecentCheck | undefined; + days: number; +}): boolean { + const { available, ownTrash, recent, days } = options; + return available && ownTrash && !!recent?.has && recent.days === days; +} diff --git a/src/screens/EmailListScreen.tsx b/src/screens/EmailListScreen.tsx index aa1c812c..58605026 100644 --- a/src/screens/EmailListScreen.tsx +++ b/src/screens/EmailListScreen.tsx @@ -23,6 +23,7 @@ import { SwipeableRow } from '../components/SwipeableRow'; import { MoveSheet } from '../components/MoveSheet'; import { TagSheet } from '../components/TagSheet'; import { OfflineBanner } from '../components/OfflineBanner'; +import { TrashRecoveryLink } from '../components/email/TrashRecoveryLink'; import { ListAttachmentChips, ListAttachmentOpener, useListAttachments, useListRowAttachments, } from '../components/email/ListAttachmentChips'; @@ -50,6 +51,8 @@ import { useContactsStore } from '../stores/contacts-store'; import { useOutboxStore } from '../stores/outbox-store'; import { toast } from '../stores/toast-store'; import { getContactDisplayName } from '../lib/contact-utils'; +import { listAddressLabel } from '../lib/list-sender'; +import { useOwnDomainAddress } from '../lib/use-own-domain-address'; import { formatListDate } from '../lib/date-format'; import { singleLine } from '../lib/single-line'; import { cleanPreview } from '../lib/preview-text'; @@ -85,21 +88,20 @@ function reportKeywordFailure(title: string, err: unknown): void { toast.error(title, err instanceof Error ? err.message : undefined); } -function getSenderName(email: Email, unknownLabel: string): string { - return email.from?.[0]?.name || email.from?.[0]?.email || unknownLabel; -} - -function getSenderEmail(email: Email): string | undefined { - return email.from?.[0]?.email; -} - -// Sent/Drafts rows name the recipient, not "me" (webmail 1.4.12). -function getCounterpart(email: Email, showRecipient: boolean, unknownLabel: string): { name: string; email?: string } { +// Sent/Drafts rows name the recipient, not "me" (webmail 1.4.12). A bare +// address on one of the user's own IDN domains reads as written (#1100). +function getCounterpart( + email: Email, + showRecipient: boolean, + unknownLabel: string, + formatAddress: (address: string) => string, +): { name: string; email?: string } { if (showRecipient) { const to = email.to?.[0] ?? email.cc?.[0] ?? email.bcc?.[0]; - if (to) return { name: to.name || to.email, email: to.email }; + if (to) return { name: listAddressLabel(to, formatAddress, unknownLabel), email: to.email }; } - return { name: getSenderName(email, unknownLabel), email: getSenderEmail(email) }; + const from = email.from?.[0]; + return { name: listAddressLabel(from, formatAddress, unknownLabel), email: from?.email }; } function isUnread(email: Email): boolean { @@ -136,6 +138,7 @@ const EmailRow = React.memo(function EmailRow({ threadCount, showPreview, showRecipient, + formatAddress, tagIds, keywordDefs, disableAvatarImages, @@ -153,6 +156,8 @@ const EmailRow = React.memo(function EmailRow({ threadCount: number; showPreview: boolean; showRecipient: boolean; + /** useOwnDomainAddress, read once by the list rather than per row. */ + formatAddress: (address: string) => string; /** Comma-joined tag ids of the row (thread union) — a string so memo holds. */ tagIds: string; keywordDefs: KeywordDef[]; @@ -181,6 +186,7 @@ const EmailRow = React.memo(function EmailRow({ item, showRecipient, tr('email_viewer.unknown_sender', 'Unknown'), + formatAddress, ); const unread = isUnread(item); const starred = isStarred(item); @@ -570,6 +576,7 @@ export default function EmailListScreen({ const folderKeywordDefs = (currentOwnerAccountId && keywordDefsByAccount.get(currentOwnerAccountId)) || keywordDefs; const inJunk = currentRole === 'junk' || currentRole === 'spam'; const showRecipient = currentRole === 'sent' || currentRole === 'drafts'; + const formatAddress = useOwnDomainAddress(); // When threading is on, collapse same-thread emails so the list shows the // newest message per thread with a count badge (pinned threads first). @@ -885,6 +892,7 @@ export default function EmailListScreen({ threadCount={threadCountFor(item)} showPreview={showPreview} showRecipient={showRecipient} + formatAddress={formatAddress} tagIds={rowTagIds.get(key) ?? ''} keywordDefs={(item.jmapAccountId && keywordDefsByAccount.get(item.jmapAccountId)) || folderKeywordDefs} disableAvatarImages={inJunk && !showAvatarsInJunk} @@ -903,7 +911,7 @@ export default function EmailListScreen({ [ selectedIds, activeRowKey, selectionMode, handleRowPress, toggleSelect, swipeLeftAction, swipeRightAction, swipeMode, handleRowSwipe, disableThreading, rowFlags, rowTagIds, threadCountFor, - showPreview, showRecipient, keywordDefsByAccount, folderKeywordDefs, inJunk, showAvatarsInJunk, + showPreview, showRecipient, formatAddress, keywordDefsByAccount, folderKeywordDefs, inJunk, showAvatarsInJunk, loadAttachments, openAttachment, ], ); @@ -1659,6 +1667,9 @@ export default function EmailListScreen({ )} + {/* Recover deleted items, from the account's own Trash (mounted only there) */} + {currentMailbox?.role === 'trash' && } + {canEmptyFolder && !selectionMode && ( diff --git a/src/screens/UnifiedInboxScreen.tsx b/src/screens/UnifiedInboxScreen.tsx index 1855d9fd..553e4c9b 100644 --- a/src/screens/UnifiedInboxScreen.tsx +++ b/src/screens/UnifiedInboxScreen.tsx @@ -25,6 +25,8 @@ import { useSettingsStore, type SwipeAction } from '../stores/settings-store'; import { useLocaleStore } from '../stores/locale-store'; import { formatListDate } from '../lib/date-format'; import { singleLine } from '../lib/single-line'; +import { listAddressLabel } from '../lib/list-sender'; +import { useOwnDomainAddress } from '../lib/use-own-domain-address'; import { cleanPreview } from '../lib/preview-text'; import { prefetchMessage, rememberRows } from '../lib/email-detail-cache'; import { isPermanentDelete, confirmPermanentDelete } from '../lib/delete-confirm'; @@ -39,12 +41,18 @@ type Props = NativeStackScreenProps; const PAGE_SIZE = 25; -function senderName(email: UnifiedEmail, showRecipient: boolean, unknown: string): string { +// A bare address on one of the user's own IDN domains reads as written (#1100). +function senderName( + email: UnifiedEmail, + showRecipient: boolean, + unknown: string, + formatAddress: (address: string) => string, +): string { if (showRecipient) { const to = email.to?.[0]; - if (to) return to.name || to.email; + if (to) return listAddressLabel(to, formatAddress, unknown); } - return email.from?.[0]?.name || email.from?.[0]?.email || unknown; + return listAddressLabel(email.from?.[0], formatAddress, unknown); } function rowKey(e: UnifiedEmail): string { @@ -326,6 +334,7 @@ export default function UnifiedInboxScreen({ navigation, route }: Props) { const errorCount = Object.keys(errors).length; const showRecipient = role === 'sent' || role === 'drafts'; + const formatAddress = useOwnDomainAddress(); const title = view ? t(`sidebar.unified_all_${view === 'all' ? 'mail' : view}`, { all: 'All mail', unread: 'All unread', starred: 'All starred' }[view]) : role === 'inbox' @@ -366,7 +375,7 @@ export default function UnifiedInboxScreen({ navigation, route }: Props) { )} {density.showAvatar && - {senderName(item, showRecipient, t('email_viewer.unknown_sender', 'Unknown'))} + {senderName(item, showRecipient, t('email_viewer.unknown_sender', 'Unknown'), formatAddress)} {starred && } {item.hasAttachment && }