From b1a0eaf8054175f81892678d49d4e5c54116128f Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Sat, 31 Oct 2026 18:15:50 +0100 Subject: [PATCH] fix: turn sender favicons off on upgrade unless they were switched on by choice --- .../__tests__/settings-store-policy.test.ts | 48 ++++++++++++++++ src/stores/settings-store.ts | 55 ++++++++++++++++--- 2 files changed, 96 insertions(+), 7 deletions(-) diff --git a/src/stores/__tests__/settings-store-policy.test.ts b/src/stores/__tests__/settings-store-policy.test.ts index 750c4eef..476eac3d 100644 --- a/src/stores/__tests__/settings-store-policy.test.ts +++ b/src/stores/__tests__/settings-store-policy.test.ts @@ -44,6 +44,54 @@ describe('explicitSettings', () => { }); }); +describe('settings saved before a default changed', () => { + // Older versions saved the full snapshot, so senderFavicons: true (the old + // default) sits in it whether or not the user ever turned it on. + it('does not count an old default as chosen', () => { + expect(legacyExplicitSettings({ senderFavicons: true, showBirthdayCalendar: true, density: 'compact' })).toEqual(['density']); + }); + + it('moves an old default to the new one on upgrade, and lets a policy default apply', async () => { + await AsyncStorage.setItem('webmail:settings:v1', JSON.stringify({ + senderFavicons: true, + includeGroupInUnified: false, + autoSelectReplyIdentity: true, + showBirthdayCalendar: true, + density: 'compact', + })); + await useSettingsStore.getState().hydrate(); + expect(useSettingsStore.getState()).toMatchObject({ + senderFavicons: false, + includeGroupInUnified: true, + autoSelectReplyIdentity: false, + showBirthdayCalendar: false, + density: 'compact', + explicitSettings: ['density'], + }); + + resetPolicyRulesForTests(); + await AsyncStorage.setItem('webmail:settings:v1', JSON.stringify({ senderFavicons: true, density: 'compact' })); + useSettingsStore.setState({ hydrated: false }); + useSettingsStore.getState().applyPolicyRules(rules({ defaults: { senderFavicons: true, density: 'comfortable' } })); + await useSettingsStore.getState().hydrate(); + // The organisation's default reaches the setting nobody chose. + expect(useSettingsStore.getState()).toMatchObject({ senderFavicons: true, density: 'compact', explicitSettings: ['density'] }); + }); + + it('keeps settings saved with explicitSettings as they are', async () => { + await AsyncStorage.setItem('webmail:settings:v1', JSON.stringify({ senderFavicons: true, explicitSettings: ['senderFavicons'] })); + await useSettingsStore.getState().hydrate(); + expect(useSettingsStore.getState()).toMatchObject({ senderFavicons: true, explicitSettings: ['senderFavicons'] }); + }); + + it('treats an old default in an imported file the same way', () => { + useSettingsStore.setState({ hydrated: true }); + expect(useSettingsStore.getState().importSettings(JSON.stringify({ senderFavicons: true, dateFormat: 'full' }))).toBe(true); + expect(useSettingsStore.getState()).toMatchObject({ senderFavicons: false, dateFormat: 'full' }); + expect(useSettingsStore.getState().explicitSettings).not.toContain('senderFavicons'); + }); +}); + describe('applyPolicyRules', () => { it('does not write the defaults over stored settings it has not read yet', async () => { await AsyncStorage.setItem('webmail:settings:v1', JSON.stringify({ theme: 'dark', explicitSettings: ['theme'] })); diff --git a/src/stores/settings-store.ts b/src/stores/settings-store.ts index eaf87145..1eb90c51 100644 --- a/src/stores/settings-store.ts +++ b/src/stores/settings-store.ts @@ -858,17 +858,54 @@ function policyContext(explicit: readonly string[]): PolicyPatchContext { return { builtIn: POLICY_BUILT_IN, explicit: new Set(explicit), isValid: isValidPolicyValue }; } +/** + * Defaults an earlier version had, for the settings whose default changed + * since. Versions before explicitSettings saved the whole snapshot, defaults + * included, so a value equal to the old default was most likely never + * picked: senderFavicons was on until sender domains stopped going to the + * favicon service by default; the other three changed when the defaults were + * aligned with the webmail (0.1.59). + */ +const LEGACY_DEFAULTS: Readonly>> = { + senderFavicons: true, + includeGroupInUnified: false, + autoSelectReplyIdentity: true, + showBirthdayCalendar: true, +}; + +function isLegacyDefault(source: Record, key: string): boolean { + return key in LEGACY_DEFAULTS && source[key] === LEGACY_DEFAULTS[key as keyof PersistedSettings]; +} + /** * Settings from before explicitSettings existed: a value other than the * build default can only have come from the user, so it counts as chosen - * (webmail legacyExplicitSettings). + * (webmail legacyExplicitSettings). A value equal to that setting's old + * default doesn't (webmail migrateNotificationSoundDefault). */ export function legacyExplicitSettings(source: Record): string[] { return Object.keys(source).filter( - (key) => isPolicySettingKey(key) && source[key] !== undefined && source[key] !== POLICY_BUILT_IN[key], + (key) => + isPolicySettingKey(key) && + source[key] !== undefined && + source[key] !== POLICY_BUILT_IN[key] && + !isLegacyDefault(source, key), ); } +/** + * Moves a setting still at its old default, in settings from before + * explicitSettings existed, to the current default: the user never picked + * it (webmail migrateNotificationSoundDefault, migrateTintListRowsDefault). + */ +export function migrateLegacyDefaults>(source: T): T { + const out: Record = { ...source }; + for (const key of Object.keys(LEGACY_DEFAULTS)) { + if (isLegacyDefault(out, key)) out[key] = DEFAULT_PERSISTED[key as keyof PersistedSettings]; + } + return out as T; +} + /** Whether the organisation policy pins `key`, so the user can't change it. */ export function isSettingLockedByPolicy(key: string): boolean { return isLockedSetting(policyRules, key); @@ -942,9 +979,10 @@ export const useSettingsStore = create((set, get) => ({ const raw = await AsyncStorage.getItem(STORAGE_KEY); if (raw) { const parsed = JSON.parse(raw) as Partial; - const merged = mergeWithDefaults(parsed); + let merged = mergeWithDefaults(parsed); if (!Array.isArray(parsed.explicitSettings)) { merged.explicitSettings = legacyExplicitSettings(merged as unknown as Record); + merged = migrateLegacyDefaults(merged as unknown as Record) as unknown as PersistedSettings; } // The policy may have arrived (from its cache) before the settings. const patch = currentPolicyPatch(merged); @@ -1089,12 +1127,15 @@ export const useSettingsStore = create((set, get) => ({ return false; } if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return false; - const incoming = fromExportShape(parsed as Record); + let incoming = fromExportShape(parsed as Record); // A file without explicitSettings (older clients): what it sets to - // something other than the default was chosen. - const chosen = Array.isArray(incoming.explicitSettings) - ? incoming.explicitSettings.filter((key): key is string => typeof key === 'string') + // something other than the default was chosen, and what is still at an + // old default moves to the current one. + const legacy = !Array.isArray(incoming.explicitSettings); + const chosen = !legacy + ? (incoming.explicitSettings as unknown[]).filter((key): key is string => typeof key === 'string') : legacyExplicitSettings(incoming as Record); + if (legacy) incoming = migrateLegacyDefaults(incoming as Record) as typeof incoming; // Validate against the current state so keys absent from the file keep // their value instead of snapping back to the default. const merged = mergeWithDefaults({