From b1c1b58d0adc50023ab238347bcbcc32a08337ee Mon Sep 17 00:00:00 2001 From: Eugene Susla Date: Sun, 23 Aug 2026 17:23:48 -0700 Subject: [PATCH 1/3] fix(l10n): stop re-pushing a cleared per-app language override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The launch-time reconciliation added with the per-app language picker (#672) pushed the saved in-app language to the OS whenever the OS reported no override. That push was meant as a one-time migration for installs predating the wiring, but nothing recorded that it had happened, so it ran on every launch — and "no override" is also exactly what the OS reports after the user picks System default in Android's per-app picker. Their explicit clear was silently reinstated on the next cold start, and the only way off the override was the in-app picker, with nothing suggesting so. Record the migration in config (localeSyncSeeded, Hive index 37) and decide the ambiguous cases on the raw system tag instead of the folded language code: - Supported override: system wins, saved if different, seeded recorded (both sides agree; the migration must never fire again). - Override we do not ship (e.g. ja): ignored on both sides. Previously the saved code was pushed over it, overwriting an explicit OS-level choice the app merely cannot render. - No override, not yet seeded: the one-time migration push, now recorded as seeded. - No override, seeded: the user cleared it — follow the system and clear the in-app override too. The runtime path (didChangeLocales -> _adoptSystemLocale) gets the same raw-tag treatment, so clearing the override while the app is alive now switches the app back to the system language instead of doing nothing. --- .../data/data_source/config_data_source.dart | 8 ++ lib/core/data/dbo/config_dbo.dart | 9 ++ lib/core/data/dbo/config_dbo.g.dart | 9 +- .../data/repository/config_repository.dart | 8 ++ lib/core/utils/app_locale_sync.dart | 52 ++++++-- lib/dev/main_dev.dart | 2 + lib/main.dart | 23 +++- test/unit_test/app_locale_sync_test.dart | 125 ++++++++++++++---- 8 files changed, 195 insertions(+), 41 deletions(-) diff --git a/lib/core/data/data_source/config_data_source.dart b/lib/core/data/data_source/config_data_source.dart index 13fe2953c..31f0f3b32 100644 --- a/lib/core/data/data_source/config_data_source.dart +++ b/lib/core/data/data_source/config_data_source.dart @@ -218,6 +218,14 @@ class ConfigDataSource { await _update((c) => c.selectedLocale = locale); } + Future getLocaleSyncSeeded() async { + return _readMerged().localeSyncSeeded ?? false; + } + + Future setLocaleSyncSeeded() async { + await _update((c) => c.localeSyncSeeded = true); + } + Future setConfigShowMicronutrients(bool show) async { await _update((c) => c.showMicronutrients = show); } diff --git a/lib/core/data/dbo/config_dbo.dart b/lib/core/data/dbo/config_dbo.dart index 99a8c3e48..f0a8d878b 100644 --- a/lib/core/data/dbo/config_dbo.dart +++ b/lib/core/data/dbo/config_dbo.dart @@ -152,6 +152,14 @@ class ConfigDBO extends HiveObject { // nothing has been deleted. @HiveField(36) List? healthDeletedExternalIds; + // One-shot marker for [reconcileAppLocale]'s migration push: set once the + // saved language has been offered to Android's per-app language picker (or + // the picker was seen holding a value). Null means the push has not + // happened yet. Without it, an override the user cleared in the OS picker + // is indistinguishable from one that was never seeded, and gets silently + // pushed back on the next launch. + @HiveField(37) + bool? localeSyncSeeded; ConfigDBO( this.hasAcceptedDisclaimer, @@ -188,6 +196,7 @@ class ConfigDBO extends HiveObject { this.healthWorkoutKcalMultiplier, this.healthLastImportAt, this.healthDeletedExternalIds, + this.localeSyncSeeded, }); factory ConfigDBO.empty() => diff --git a/lib/core/data/dbo/config_dbo.g.dart b/lib/core/data/dbo/config_dbo.g.dart index f52b3260c..ab821512c 100644 --- a/lib/core/data/dbo/config_dbo.g.dart +++ b/lib/core/data/dbo/config_dbo.g.dart @@ -51,6 +51,7 @@ class ConfigDBOAdapter extends TypeAdapter { healthWorkoutKcalMultiplier: (fields[34] as num?)?.toDouble(), healthLastImportAt: fields[35] as DateTime?, healthDeletedExternalIds: (fields[36] as List?)?.cast(), + localeSyncSeeded: fields[37] as bool?, ) ..userCarbGoalPct = (fields[6] as num?)?.toDouble() ..userProteinGoalPct = (fields[7] as num?)?.toDouble() @@ -60,7 +61,7 @@ class ConfigDBOAdapter extends TypeAdapter { @override void write(BinaryWriter writer, ConfigDBO obj) { writer - ..writeByte(37) + ..writeByte(38) ..writeByte(0) ..write(obj.hasAcceptedDisclaimer) ..writeByte(1) @@ -134,7 +135,9 @@ class ConfigDBOAdapter extends TypeAdapter { ..writeByte(35) ..write(obj.healthLastImportAt) ..writeByte(36) - ..write(obj.healthDeletedExternalIds); + ..write(obj.healthDeletedExternalIds) + ..writeByte(37) + ..write(obj.localeSyncSeeded); } @override @@ -202,6 +205,7 @@ ConfigDBO _$ConfigDBOFromJson(Map json) => (json['healthDeletedExternalIds'] as List?) ?.map((e) => e as String) .toList(), + localeSyncSeeded: json['localeSyncSeeded'] as bool?, ) ..userCarbGoalPct = (json['userCarbGoalPct'] as num?)?.toDouble() ..userProteinGoalPct = (json['userProteinGoalPct'] as num?)?.toDouble() @@ -245,6 +249,7 @@ Map _$ConfigDBOToJson(ConfigDBO instance) => { 'healthWorkoutKcalMultiplier': instance.healthWorkoutKcalMultiplier, 'healthLastImportAt': instance.healthLastImportAt?.toIso8601String(), 'healthDeletedExternalIds': instance.healthDeletedExternalIds, + 'localeSyncSeeded': instance.localeSyncSeeded, }; const _$AppThemeDBOEnumMap = { diff --git a/lib/core/data/repository/config_repository.dart b/lib/core/data/repository/config_repository.dart index 64ccaa920..9e523cf8a 100644 --- a/lib/core/data/repository/config_repository.dart +++ b/lib/core/data/repository/config_repository.dart @@ -110,6 +110,14 @@ class ConfigRepository { await _configDataSource.setSelectedLocale(locale); } + Future getLocaleSyncSeeded() async { + return await _configDataSource.getLocaleSyncSeeded(); + } + + Future setLocaleSyncSeeded() async { + await _configDataSource.setLocaleSyncSeeded(); + } + Future setConfigShowMicronutrients(bool show) async { await _configDataSource.setConfigShowMicronutrients(show); } diff --git a/lib/core/utils/app_locale_sync.dart b/lib/core/utils/app_locale_sync.dart index 6c3004f15..0711e5b0e 100644 --- a/lib/core/utils/app_locale_sync.dart +++ b/lib/core/utils/app_locale_sync.dart @@ -7,30 +7,59 @@ import 'dart:ui'; /// and Settings -> Apps -> OpenNutriTracker -> Language. Whichever one someone /// reaches for, they should get the same answer afterwards. /// -/// The system override wins when it exists, because it is the one the user can -/// see from outside the app. When it does not exist but we have a saved choice -/// -- an install that predates this wiring, or a fresh one where only the -/// in-app picker has been used -- the saved choice is pushed out to the system -/// so both sides start from the same place. +/// Four cases, decided on the raw [systemLocaleTag] rather than on the +/// language it resolves to, because "no override at all" and "an override we +/// cannot render" mean opposite things: /// -/// Off Android and below API 33 there is no system override at all, and this -/// collapses to returning the saved choice untouched. +/// - **A supported override.** The system wins -- it is the side the user can +/// see from outside the app -- and is saved if it differs. Both sides now +/// agree, so the migration below must never fire again: seeded is recorded. +/// - **A tag we do not ship** (`ja`, say). Neither side is followed: the app +/// cannot render that language, and the saved choice is not pushed over it +/// either. The user picked that language in the OS for a reason, and +/// overwriting their choice is a louder wrong answer than ignoring it. +/// - **No override, never seeded.** The upgrade path: an install that predates +/// this wiring, or one where only the in-app picker has been used. The saved +/// choice is pushed out so both sides start from the same place. This is a +/// one-time migration, so it is recorded as seeded either way. +/// - **No override, already seeded.** The OS once held a value and no longer +/// does, which only happens when the user picked "System default" in +/// Android's picker. That is a deliberate choice, so the in-app override is +/// cleared to follow it -- without [localeSyncSeeded] this case is +/// indistinguishable from the one above and the cleared value gets silently +/// pushed back on the next cold start. +/// +/// Off Android and below API 33 there is no system override to read and none +/// can ever be set, so the first launch seeds nothing, records seeded, and +/// every launch after that returns the saved choice untouched. Future reconcileAppLocale({ required String? savedLocaleCode, required String? systemLocaleTag, + required bool localeSyncSeeded, required Iterable supportedLocales, required Future Function(String? localeCode) persistSelectedLocale, required Future Function(String? languageTag) pushToSystem, + required Future Function() markLocaleSyncSeeded, }) async { final systemCode = supportedLanguageCode(systemLocaleTag, supportedLocales); if (systemCode != null) { if (systemCode != savedLocaleCode) await persistSelectedLocale(systemCode); + if (!localeSyncSeeded) await markLocaleSyncSeeded(); return systemCode; } - if (savedLocaleCode != null) await pushToSystem(savedLocaleCode); - return savedLocaleCode; + final hasSystemTag = systemLocaleTag != null && systemLocaleTag.isNotEmpty; + if (hasSystemTag) return savedLocaleCode; + + if (!localeSyncSeeded) { + if (savedLocaleCode != null) await pushToSystem(savedLocaleCode); + await markLocaleSyncSeeded(); + return savedLocaleCode; + } + + if (savedLocaleCode != null) await persistSelectedLocale(null); + return null; } /// The supported language code behind a platform language tag, or null when @@ -47,7 +76,8 @@ String? supportedLanguageCode( if (languageTag == null || languageTag.isEmpty) return null; final code = languageTag.split(RegExp(r'[-_]')).first.toLowerCase(); if (code.isEmpty) return null; - final isSupported = - supportedLocales.any((locale) => locale.languageCode == code); + final isSupported = supportedLocales.any( + (locale) => locale.languageCode == code, + ); return isSupported ? code : null; } diff --git a/lib/dev/main_dev.dart b/lib/dev/main_dev.dart index 926f249cc..bbaa8bdbb 100644 --- a/lib/dev/main_dev.dart +++ b/lib/dev/main_dev.dart @@ -37,9 +37,11 @@ Future main() async { final localeCode = await reconcileAppLocale( savedLocaleCode: await configRepo.getSelectedLocale(), systemLocaleTag: await AppLocaleService.getApplicationLocale(), + localeSyncSeeded: await configRepo.getLocaleSyncSeeded(), supportedLocales: S.supportedLocales, persistSelectedLocale: configRepo.setSelectedLocale, pushToSystem: AppLocaleService.setApplicationLocale, + markLocaleSyncSeeded: configRepo.setLocaleSyncSeeded, ); final savedLocale = localeCode != null ? Locale(localeCode) : null; final savedAppTheme = await configRepo.getConfigAppTheme(); diff --git a/lib/main.dart b/lib/main.dart index bfaf4f875..37f5770fa 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -88,9 +88,11 @@ Future _bootstrapApp() async { final localeCode = await reconcileAppLocale( savedLocaleCode: await configRepo.getSelectedLocale(), systemLocaleTag: await AppLocaleService.getApplicationLocale(), + localeSyncSeeded: await configRepo.getLocaleSyncSeeded(), supportedLocales: S.supportedLocales, persistSelectedLocale: configRepo.setSelectedLocale, pushToSystem: AppLocaleService.setApplicationLocale, + markLocaleSyncSeeded: configRepo.setLocaleSyncSeeded, ); final savedLocale = localeCode != null ? Locale(localeCode) : null; @@ -236,13 +238,24 @@ class _OpenNutriTrackerAppState extends State } Future _adoptSystemLocale() async { - final systemCode = supportedLanguageCode( - await AppLocaleService.getApplicationLocale(), - S.supportedLocales, - ); - if (systemCode == null || !mounted) return; + final systemTag = await AppLocaleService.getApplicationLocale(); + final systemCode = supportedLanguageCode(systemTag, S.supportedLocales); + if (!mounted) return; final localeProvider = Provider.of(context, listen: false); + + if (systemCode == null) { + // A tag we do not ship is ignored, same as at launch. No tag at all + // means the user picked "System default" in Android's picker — follow + // it, or that picker appears to do nothing in the clearing direction. + if (systemTag != null && systemTag.isNotEmpty) return; + if (localeProvider.locale == null) return; + + localeProvider.updateLocale(null); + await locator().setSelectedLocale(null); + return; + } + if (localeProvider.locale?.languageCode == systemCode) return; localeProvider.updateLocale(Locale(systemCode)); diff --git a/test/unit_test/app_locale_sync_test.dart b/test/unit_test/app_locale_sync_test.dart index 0d7844aaa..95aa01241 100644 --- a/test/unit_test/app_locale_sync_test.dart +++ b/test/unit_test/app_locale_sync_test.dart @@ -23,54 +23,77 @@ const _supported = [ class _Recorder { final persisted = []; final pushed = []; + int seededMarks = 0; Future persist(String? code) async => persisted.add(code); Future push(String? tag) async => pushed.add(tag); + Future markSeeded() async => seededMarks++; } Future _reconcile( _Recorder recorder, { String? saved, String? system, -}) => - reconcileAppLocale( - savedLocaleCode: saved, - systemLocaleTag: system, - supportedLocales: _supported, - persistSelectedLocale: recorder.persist, - pushToSystem: recorder.push, - ); + bool seeded = false, +}) => reconcileAppLocale( + savedLocaleCode: saved, + systemLocaleTag: system, + localeSyncSeeded: seeded, + supportedLocales: _supported, + persistSelectedLocale: recorder.persist, + pushToSystem: recorder.push, + markLocaleSyncSeeded: recorder.markSeeded, +); void main() { group('reconcileAppLocale', () { test('the system override wins and is saved', () async { final recorder = _Recorder(); - final result = - await _reconcile(recorder, saved: 'de', system: 'pl'); + final result = await _reconcile(recorder, saved: 'de', system: 'pl'); expect(result, 'pl'); - expect(recorder.persisted, ['pl'], - reason: 'Settings should show what the OS picker holds'); - expect(recorder.pushed, isEmpty, - reason: 'the OS already has this value; writing it back is noise'); + expect( + recorder.persisted, + ['pl'], + reason: 'Settings should show what the OS picker holds', + ); + expect( + recorder.pushed, + isEmpty, + reason: 'the OS already has this value; writing it back is noise', + ); + expect( + recorder.seededMarks, + 1, + reason: 'an OS value existing means both sides are already met', + ); }); test('an override that already matches is not written again', () async { final recorder = _Recorder(); - final result = - await _reconcile(recorder, saved: 'de', system: 'de'); + final result = await _reconcile( + recorder, + saved: 'de', + system: 'de', + seeded: true, + ); expect(result, 'de'); expect(recorder.persisted, isEmpty); expect(recorder.pushed, isEmpty); + expect( + recorder.seededMarks, + 0, + reason: 'already recorded; another config write is noise', + ); }); // The upgrade path: someone chose a language in the app long before this // wiring existed, so the OS has no override yet. Their choice is what // they meant, and it seeds the system rather than being overwritten by it. - test('with no override, a saved choice seeds the system', () async { + test('with no override, a saved choice seeds the system once', () async { final recorder = _Recorder(); final result = await _reconcile(recorder, saved: 'uk', system: null); @@ -78,6 +101,41 @@ void main() { expect(result, 'uk'); expect(recorder.pushed, ['uk']); expect(recorder.persisted, isEmpty); + expect( + recorder.seededMarks, + 1, + reason: + 'unrecorded, this push would repeat on every launch and ' + 'silently reinstate an override the user cleared', + ); + }); + + // The other reading of "no override": it was seeded before, so its + // absence now is the user having picked System default in Android's + // picker. Re-pushing the saved choice would undo their action; instead + // the saved choice follows the OS and is cleared. + test('once seeded, a cleared override clears the saved choice', () async { + final recorder = _Recorder(); + + final result = await _reconcile( + recorder, + saved: 'uk', + system: null, + seeded: true, + ); + + expect(result, isNull); + expect( + recorder.pushed, + isEmpty, + reason: 'pushing the old choice back is the bug this exists for', + ); + expect( + recorder.persisted, + [null], + reason: 'System default means follow the system in both pickers', + ); + expect(recorder.seededMarks, 0); }); test('with nothing on either side, nothing happens', () async { @@ -88,6 +146,11 @@ void main() { expect(result, isNull); expect(recorder.pushed, isEmpty); expect(recorder.persisted, isEmpty); + expect( + recorder.seededMarks, + 1, + reason: 'there is nothing to migrate, so the migration is done', + ); }); test('a region-qualified tag resolves to the language we ship', () async { @@ -102,23 +165,38 @@ void main() { test('a script-and-region tag resolves the same way', () async { final recorder = _Recorder(); - final result = - await _reconcile(recorder, saved: null, system: 'zh-Hans-CN'); + final result = await _reconcile( + recorder, + saved: null, + system: 'zh-Hans-CN', + ); expect(result, 'zh'); expect(recorder.persisted, ['zh']); }); - // A tag we do not ship must not strand anyone in a half-translated app. - test('an unshipped language is treated as no override', () async { + // A tag we do not ship must not strand anyone in a half-translated app — + // but it is still the user's explicit OS-level choice, so it is ignored, + // not overwritten. The app keeps its saved language and the OS keeps its. + test('an unshipped language is ignored, not corrected', () async { final recorder = _Recorder(); final result = await _reconcile(recorder, saved: 'de', system: 'ja'); expect(result, 'de'); expect(recorder.persisted, isEmpty); - expect(recorder.pushed, ['de'], - reason: 'the OS holds a language we cannot render; correct it'); + expect( + recorder.pushed, + isEmpty, + reason: + 'overwriting an OS-level choice we merely cannot render ' + 'is a stronger action than ignoring it', + ); + expect( + recorder.seededMarks, + 0, + reason: 'nothing was reconciled; the migration question stays open', + ); }); test('an empty tag is treated as no override', () async { @@ -128,6 +206,7 @@ void main() { expect(result, 'it'); expect(recorder.pushed, ['it']); + expect(recorder.seededMarks, 1); }); }); From 422f2c72367dd5679abaf8f101d098f9d8f39c7a Mon Sep 17 00:00:00 2001 From: Eugene Susla Date: Sun, 23 Aug 2026 17:30:09 -0700 Subject: [PATCH 2/3] fix(l10n): record seeding by observation, guard failed locale reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups on the seeded flag's semantics: - localeSyncSeeded now records exactly one fact: the OS was SEEN holding an override. Recording it on the migration attempt turned every platform that can never hold one (iOS, desktop, Android < 13) into a "user who cleared it" on the second launch, silently resetting their in-app language. The migration branch now repeats harmlessly there; on Android 13+ the pushed value is observed on the next read and seeding completes then. - An unshipped override tag also records seeded — the OS demonstrably holds a value — so switching ja to System default later reads as the clear it is instead of re-triggering the migration push. - AppLocaleService.getApplicationLocale now reports read failures distinctly instead of folding PlatformException into "no override"; reconcileAppLocale decides nothing on a failed read, so a transient channel error can no longer clear a saved language. MissingPluginException stays a clean "no override": on those platforms the absence is permanent and true. - _adoptSystemLocale now runs reconcileAppLocale itself instead of hand-rolling a second copy of the decision table, so the launch and runtime paths cannot disagree about the same OS state; only the LocaleProvider update stays local. This also gives the runtime path the seeded guard it was missing — a system-language change on an override-less platform no longer wipes the in-app choice. --- lib/core/utils/app_locale_service.dart | 22 +++++-- lib/core/utils/app_locale_sync.dart | 58 +++++++++++------- lib/dev/main_dev.dart | 4 +- lib/main.dart | 42 +++++++------ test/unit_test/app_locale_service_test.dart | 23 ++++++-- test/unit_test/app_locale_sync_test.dart | 65 +++++++++++++++++---- 6 files changed, 150 insertions(+), 64 deletions(-) diff --git a/lib/core/utils/app_locale_service.dart b/lib/core/utils/app_locale_service.dart index 200ac83aa..222472464 100644 --- a/lib/core/utils/app_locale_service.dart +++ b/lib/core/utils/app_locale_service.dart @@ -19,15 +19,25 @@ import 'package:flutter/services.dart'; class AppLocaleService { static const _channel = MethodChannel('com.opennutritracker/locale'); - /// The language tag the user chose in Android's Settings, or null when they - /// have not overridden it and the app should follow its own saved choice. - static Future getApplicationLocale() async { + /// The language tag the user chose in Android's Settings, or a null tag + /// when they have not overridden it and the app should follow its own saved + /// choice. + /// + /// `readFailed` separates "the platform says there is no override" from + /// "the platform could not be asked". [reconcileAppLocale] treats a missing + /// override as the user having cleared it, so a transient channel failure + /// reported as a plain null would destroy their saved language. + /// [MissingPluginException] is not a failure: no registered handler means a + /// platform with no per-app language setting at all, where "no override" is + /// the honest answer. + static Future<({String? tag, bool readFailed})> getApplicationLocale() async { try { - return await _channel.invokeMethod('getApplicationLocale'); + final tag = await _channel.invokeMethod('getApplicationLocale'); + return (tag: tag, readFailed: false); } on PlatformException { - return null; + return (tag: null, readFailed: true); } on MissingPluginException { - return null; + return (tag: null, readFailed: false); } } diff --git a/lib/core/utils/app_locale_sync.dart b/lib/core/utils/app_locale_sync.dart index 0711e5b0e..3cc8b008a 100644 --- a/lib/core/utils/app_locale_sync.dart +++ b/lib/core/utils/app_locale_sync.dart @@ -7,40 +7,52 @@ import 'dart:ui'; /// and Settings -> Apps -> OpenNutriTracker -> Language. Whichever one someone /// reaches for, they should get the same answer afterwards. /// -/// Four cases, decided on the raw [systemLocaleTag] rather than on the +/// The cases are decided on the raw [systemLocaleTag] rather than on the /// language it resolves to, because "no override at all" and "an override we -/// cannot render" mean opposite things: +/// cannot render" mean opposite things. [localeSyncSeeded] records one fact +/// and only that fact: **the OS has been seen holding an override** -- it is +/// what makes a later absence readable as the user having cleared it. It is +/// never set on a platform that reports no override, because there the +/// absence is permanent and means nothing. /// /// - **A supported override.** The system wins -- it is the side the user can -/// see from outside the app -- and is saved if it differs. Both sides now -/// agree, so the migration below must never fire again: seeded is recorded. +/// see from outside the app -- and is saved if it differs. The OS holds a +/// value, so seeded is recorded. /// - **A tag we do not ship** (`ja`, say). Neither side is followed: the app /// cannot render that language, and the saved choice is not pushed over it -/// either. The user picked that language in the OS for a reason, and -/// overwriting their choice is a louder wrong answer than ignoring it. -/// - **No override, never seeded.** The upgrade path: an install that predates -/// this wiring, or one where only the in-app picker has been used. The saved -/// choice is pushed out so both sides start from the same place. This is a -/// one-time migration, so it is recorded as seeded either way. -/// - **No override, already seeded.** The OS once held a value and no longer -/// does, which only happens when the user picked "System default" in -/// Android's picker. That is a deliberate choice, so the in-app override is -/// cleared to follow it -- without [localeSyncSeeded] this case is -/// indistinguishable from the one above and the cleared value gets silently -/// pushed back on the next cold start. -/// -/// Off Android and below API 33 there is no system override to read and none -/// can ever be set, so the first launch seeds nothing, records seeded, and -/// every launch after that returns the saved choice untouched. +/// either -- overwriting an explicit OS-level choice is a louder wrong +/// answer than ignoring it. The OS still demonstrably holds a value, so +/// seeded is recorded; switching it to System default later reads as the +/// clear it is. +/// - **The read failed.** Nothing is decided on a failed read: the saved +/// choice is returned untouched. Treating it as "no override" would clear +/// a language the user never cleared. +/// - **No override, never seeded.** The upgrade path: an install that +/// predates this wiring, or one where only the in-app picker has been +/// used. The saved choice is pushed out so both sides start from the same +/// place. Deliberately not recorded as seeded here -- the record comes +/// from *observing* the pushed value on a later read. On Android 13+ that +/// is the next launch; on platforms with no per-app override the push +/// no-ops, the observation never comes, and this branch harmlessly repeats +/// instead of ever mistaking the platform for a user who cleared it. +/// - **No override, seeded.** The OS held a value and no longer does, which +/// only happens when the user picked "System default" in Android's picker. +/// That is a deliberate choice, so the in-app override is cleared to +/// follow it -- without the seeded record this case is indistinguishable +/// from the one above and the cleared value got silently pushed back on +/// the next cold start. Future reconcileAppLocale({ required String? savedLocaleCode, required String? systemLocaleTag, + required bool systemTagReadFailed, required bool localeSyncSeeded, required Iterable supportedLocales, required Future Function(String? localeCode) persistSelectedLocale, required Future Function(String? languageTag) pushToSystem, required Future Function() markLocaleSyncSeeded, }) async { + if (systemTagReadFailed) return savedLocaleCode; + final systemCode = supportedLanguageCode(systemLocaleTag, supportedLocales); if (systemCode != null) { @@ -50,11 +62,13 @@ Future reconcileAppLocale({ } final hasSystemTag = systemLocaleTag != null && systemLocaleTag.isNotEmpty; - if (hasSystemTag) return savedLocaleCode; + if (hasSystemTag) { + if (!localeSyncSeeded) await markLocaleSyncSeeded(); + return savedLocaleCode; + } if (!localeSyncSeeded) { if (savedLocaleCode != null) await pushToSystem(savedLocaleCode); - await markLocaleSyncSeeded(); return savedLocaleCode; } diff --git a/lib/dev/main_dev.dart b/lib/dev/main_dev.dart index bbaa8bdbb..ea2fa638d 100644 --- a/lib/dev/main_dev.dart +++ b/lib/dev/main_dev.dart @@ -34,9 +34,11 @@ Future main() async { final config = await configRepo.getConfig(); // Mirrors the reconciliation in main.dart so a dev build behaves the same // way when the OS holds a per-app language. + final systemLocale = await AppLocaleService.getApplicationLocale(); final localeCode = await reconcileAppLocale( savedLocaleCode: await configRepo.getSelectedLocale(), - systemLocaleTag: await AppLocaleService.getApplicationLocale(), + systemLocaleTag: systemLocale.tag, + systemTagReadFailed: systemLocale.readFailed, localeSyncSeeded: await configRepo.getLocaleSyncSeeded(), supportedLocales: S.supportedLocales, persistSelectedLocale: configRepo.setSelectedLocale, diff --git a/lib/main.dart b/lib/main.dart index 37f5770fa..374e88f8c 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -85,9 +85,11 @@ Future _bootstrapApp() async { // Android's own per-app language picker and ours are two doors into the // same setting, so ask the system what it holds before trusting what we // saved. See [reconcileAppLocale] for which side wins and why. + final systemLocale = await AppLocaleService.getApplicationLocale(); final localeCode = await reconcileAppLocale( savedLocaleCode: await configRepo.getSelectedLocale(), - systemLocaleTag: await AppLocaleService.getApplicationLocale(), + systemLocaleTag: systemLocale.tag, + systemTagReadFailed: systemLocale.readFailed, localeSyncSeeded: await configRepo.getLocaleSyncSeeded(), supportedLocales: S.supportedLocales, persistSelectedLocale: configRepo.setSelectedLocale, @@ -237,29 +239,31 @@ class _OpenNutriTrackerAppState extends State unawaited(_adoptSystemLocale()); } + /// The same [reconcileAppLocale] the launch path runs, deliberately: this + /// fires for any locale change — the per-app picker, but also a plain + /// system-language change on platforms with no per-app override — and a + /// second hand-rolled copy of the decision table here could disagree with + /// the launch one about the same OS state within one session. Only the + /// screen update is local; every persistence decision stays in one place. Future _adoptSystemLocale() async { - final systemTag = await AppLocaleService.getApplicationLocale(); - final systemCode = supportedLanguageCode(systemTag, S.supportedLocales); + final configRepo = locator(); + final systemLocale = await AppLocaleService.getApplicationLocale(); + final localeCode = await reconcileAppLocale( + savedLocaleCode: await configRepo.getSelectedLocale(), + systemLocaleTag: systemLocale.tag, + systemTagReadFailed: systemLocale.readFailed, + localeSyncSeeded: await configRepo.getLocaleSyncSeeded(), + supportedLocales: S.supportedLocales, + persistSelectedLocale: configRepo.setSelectedLocale, + pushToSystem: AppLocaleService.setApplicationLocale, + markLocaleSyncSeeded: configRepo.setLocaleSyncSeeded, + ); if (!mounted) return; final localeProvider = Provider.of(context, listen: false); + if (localeProvider.locale?.languageCode == localeCode) return; - if (systemCode == null) { - // A tag we do not ship is ignored, same as at launch. No tag at all - // means the user picked "System default" in Android's picker — follow - // it, or that picker appears to do nothing in the clearing direction. - if (systemTag != null && systemTag.isNotEmpty) return; - if (localeProvider.locale == null) return; - - localeProvider.updateLocale(null); - await locator().setSelectedLocale(null); - return; - } - - if (localeProvider.locale?.languageCode == systemCode) return; - - localeProvider.updateLocale(Locale(systemCode)); - await locator().setSelectedLocale(systemCode); + localeProvider.updateLocale(localeCode != null ? Locale(localeCode) : null); } @override diff --git a/test/unit_test/app_locale_service_test.dart b/test/unit_test/app_locale_service_test.dart index 1d8a384de..8ba710f1d 100644 --- a/test/unit_test/app_locale_service_test.dart +++ b/test/unit_test/app_locale_service_test.dart @@ -30,7 +30,10 @@ void main() { test('reads the language the OS holds', () async { handleWith((_) async => 'pl'); - expect(await AppLocaleService.getApplicationLocale(), 'pl'); + expect(await AppLocaleService.getApplicationLocale(), ( + tag: 'pl', + readFailed: false, + )); expect(calls.single.method, 'getApplicationLocale'); }); @@ -53,10 +56,16 @@ void main() { expect(calls.single.arguments, {'tag': null}); }); - test('a platform failure does not escape', () async { + // The read must say it failed, not answer "no override": downstream, a + // missing override from a working read means the user cleared their + // language, and a transient failure must never read as that. + test('a platform failure does not escape, and says it failed', () async { handleWith((_) => throw PlatformException(code: 'unavailable')); - await expectLater(AppLocaleService.getApplicationLocale(), completion(isNull)); + expect(await AppLocaleService.getApplicationLocale(), ( + tag: null, + readFailed: true, + )); await expectLater(AppLocaleService.setApplicationLocale('de'), completes); }); }); @@ -64,8 +73,14 @@ void main() { // Nothing on the other side of the channel is the normal case on iOS, on // desktop, and in any widget test that pumps the settings screen. Failing // there would break a screen someone opened to fix their language. + // Reported as a clean "no override", not as a failure: on these platforms + // the absence is permanent and true, and marking it failed would freeze + // locale handling there forever. test('a missing native side is not an error', () async { - expect(await AppLocaleService.getApplicationLocale(), isNull); + expect(await AppLocaleService.getApplicationLocale(), ( + tag: null, + readFailed: false, + )); await expectLater(AppLocaleService.setApplicationLocale('de'), completes); }); } diff --git a/test/unit_test/app_locale_sync_test.dart b/test/unit_test/app_locale_sync_test.dart index 95aa01241..484cb3115 100644 --- a/test/unit_test/app_locale_sync_test.dart +++ b/test/unit_test/app_locale_sync_test.dart @@ -35,9 +35,11 @@ Future _reconcile( String? saved, String? system, bool seeded = false, + bool readFailed = false, }) => reconcileAppLocale( savedLocaleCode: saved, systemLocaleTag: system, + systemTagReadFailed: readFailed, localeSyncSeeded: seeded, supportedLocales: _supported, persistSelectedLocale: recorder.persist, @@ -93,7 +95,7 @@ void main() { // The upgrade path: someone chose a language in the app long before this // wiring existed, so the OS has no override yet. Their choice is what // they meant, and it seeds the system rather than being overwritten by it. - test('with no override, a saved choice seeds the system once', () async { + test('with no override, a saved choice seeds the system', () async { final recorder = _Recorder(); final result = await _reconcile(recorder, saved: 'uk', system: null); @@ -103,13 +105,33 @@ void main() { expect(recorder.persisted, isEmpty); expect( recorder.seededMarks, - 1, + 0, reason: - 'unrecorded, this push would repeat on every launch and ' - 'silently reinstate an override the user cleared', + 'seeded means the OS was SEEN holding a value; recording it on ' + 'the attempt would turn a platform that cannot hold one — where ' + 'this branch repeats forever — into a user who cleared it', ); }); + // The observation that completes the migration: the pushed value comes + // back on the next read, which is when seeded may be recorded. On a + // platform with no per-app override the push no-ops, this never fires, + // and the saved choice survives every launch. + test('a saved choice on an override-less platform survives', () async { + final recorder = _Recorder(); + + await _reconcile(recorder, saved: 'uk', system: null); + final result = await _reconcile(recorder, saved: 'uk', system: null); + + expect(result, 'uk'); + expect( + recorder.persisted, + isEmpty, + reason: 'nothing may clear a choice the user never cleared', + ); + expect(recorder.seededMarks, 0); + }); + // The other reading of "no override": it was seeded before, so its // absence now is the user having picked System default in Android's // picker. Re-pushing the saved choice would undo their action; instead @@ -146,11 +168,7 @@ void main() { expect(result, isNull); expect(recorder.pushed, isEmpty); expect(recorder.persisted, isEmpty); - expect( - recorder.seededMarks, - 1, - reason: 'there is nothing to migrate, so the migration is done', - ); + expect(recorder.seededMarks, 0); }); test('a region-qualified tag resolves to the language we ship', () async { @@ -194,8 +212,11 @@ void main() { ); expect( recorder.seededMarks, - 0, - reason: 'nothing was reconciled; the migration question stays open', + 1, + reason: + 'the OS demonstrably holds an override, so its later absence ' + 'must read as the user clearing it — not as never-seeded, which ' + 'would re-push the saved code over their System default', ); }); @@ -206,7 +227,27 @@ void main() { expect(result, 'it'); expect(recorder.pushed, ['it']); - expect(recorder.seededMarks, 1); + expect(recorder.seededMarks, 0); + }); + + // A channel failure is not an answer. Deciding anything on it — most of + // all the seeded+absent clear below — would destroy a language the user + // never touched. + test('a failed read changes nothing, even when seeded', () async { + final recorder = _Recorder(); + + final result = await _reconcile( + recorder, + saved: 'de', + system: null, + seeded: true, + readFailed: true, + ); + + expect(result, 'de'); + expect(recorder.pushed, isEmpty); + expect(recorder.persisted, isEmpty); + expect(recorder.seededMarks, 0); }); }); From 31e492014777a583199e996ed3a1fefcf31bec1b Mon Sep 17 00:00:00 2001 From: Eugene Susla Date: Sun, 23 Aug 2026 17:37:48 -0700 Subject: [PATCH 3/3] fix(l10n): verify the migration push by reading the tag back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recording seeded only on a later, separate observation left a window — from the migration push until the next read — in which a user's clear in Android's picker still read as never-seeded and was re-pushed. The window never closed at all when the pushed language matched the device language, since no locale change meant no didChangeLocales and no read. Read the tag straight back inside the migration branch instead: on a platform that holds per-app overrides the pushed value echoes back immediately and seeded is recorded before the call returns, so any later clear is honoured; on an override-less platform the read-back stays empty and the branch keeps harmlessly repeating. This also stops the migration push re-running on every launch and locale callback on capable devices — it now runs once. _adoptSystemLocale is serialized against itself: reconciling can change the locale (the push), which re-fires didChangeLocales mid-run, and two interleaved runs would double-apply one config snapshot. A run arriving early queues a replay rather than being dropped, so a clear landing mid-run is still picked up. Tests cover the push-echo seeding, the override-less repeat (pushes asserted), and the failed-read guard. --- lib/core/utils/app_locale_sync.dart | 21 +++++++++---- lib/dev/main_dev.dart | 1 + lib/main.dart | 33 +++++++++++++++++++ test/unit_test/app_locale_sync_test.dart | 40 +++++++++++++++++------- 4 files changed, 77 insertions(+), 18 deletions(-) diff --git a/lib/core/utils/app_locale_sync.dart b/lib/core/utils/app_locale_sync.dart index 3cc8b008a..e0f6fb100 100644 --- a/lib/core/utils/app_locale_sync.dart +++ b/lib/core/utils/app_locale_sync.dart @@ -30,11 +30,13 @@ import 'dart:ui'; /// - **No override, never seeded.** The upgrade path: an install that /// predates this wiring, or one where only the in-app picker has been /// used. The saved choice is pushed out so both sides start from the same -/// place. Deliberately not recorded as seeded here -- the record comes -/// from *observing* the pushed value on a later read. On Android 13+ that -/// is the next launch; on platforms with no per-app override the push -/// no-ops, the observation never comes, and this branch harmlessly repeats -/// instead of ever mistaking the platform for a user who cleared it. +/// place. Seeded is recorded only by *observing* the pushed value, read +/// back in this same call -- never by the attempt alone. On Android 13+ +/// the read-back sees the value immediately, so the record lands before +/// this call returns and a clear arriving any time after it is honoured; +/// on platforms with no per-app override the push no-ops, the read-back +/// stays empty, and this branch harmlessly repeats instead of ever +/// mistaking the platform for a user who cleared it. /// - **No override, seeded.** The OS held a value and no longer does, which /// only happens when the user picked "System default" in Android's picker. /// That is a deliberate choice, so the in-app override is cleared to @@ -49,6 +51,7 @@ Future reconcileAppLocale({ required Iterable supportedLocales, required Future Function(String? localeCode) persistSelectedLocale, required Future Function(String? languageTag) pushToSystem, + required Future<({String? tag, bool readFailed})> Function() readSystemTag, required Future Function() markLocaleSyncSeeded, }) async { if (systemTagReadFailed) return savedLocaleCode; @@ -68,7 +71,13 @@ Future reconcileAppLocale({ } if (!localeSyncSeeded) { - if (savedLocaleCode != null) await pushToSystem(savedLocaleCode); + if (savedLocaleCode != null) { + await pushToSystem(savedLocaleCode); + final verify = await readSystemTag(); + if (!verify.readFailed && verify.tag != null && verify.tag!.isNotEmpty) { + await markLocaleSyncSeeded(); + } + } return savedLocaleCode; } diff --git a/lib/dev/main_dev.dart b/lib/dev/main_dev.dart index ea2fa638d..c1ec134de 100644 --- a/lib/dev/main_dev.dart +++ b/lib/dev/main_dev.dart @@ -43,6 +43,7 @@ Future main() async { supportedLocales: S.supportedLocales, persistSelectedLocale: configRepo.setSelectedLocale, pushToSystem: AppLocaleService.setApplicationLocale, + readSystemTag: AppLocaleService.getApplicationLocale, markLocaleSyncSeeded: configRepo.setLocaleSyncSeeded, ); final savedLocale = localeCode != null ? Locale(localeCode) : null; diff --git a/lib/main.dart b/lib/main.dart index 374e88f8c..e044ed78b 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -94,6 +94,7 @@ Future _bootstrapApp() async { supportedLocales: S.supportedLocales, persistSelectedLocale: configRepo.setSelectedLocale, pushToSystem: AppLocaleService.setApplicationLocale, + readSystemTag: AppLocaleService.getApplicationLocale, markLocaleSyncSeeded: configRepo.setLocaleSyncSeeded, ); final savedLocale = localeCode != null ? Locale(localeCode) : null; @@ -239,13 +240,41 @@ class _OpenNutriTrackerAppState extends State unawaited(_adoptSystemLocale()); } + /// True while [_adoptSystemLocale] is between its awaits; a second entry + /// then only queues a re-run instead of racing the first. + bool _adoptingSystemLocale = false; + bool _adoptSystemLocaleQueued = false; + /// The same [reconcileAppLocale] the launch path runs, deliberately: this /// fires for any locale change — the per-app picker, but also a plain /// system-language change on platforms with no per-app override — and a /// second hand-rolled copy of the decision table here could disagree with /// the launch one about the same OS state within one session. Only the /// screen update is local; every persistence decision stays in one place. + /// + /// Serialized against itself: reconciling can change the locale (the + /// migration push), which re-fires [didChangeLocales] while the first run + /// is still mid-flight. Overlapping runs would read the same pre-write + /// config snapshot and double-apply it, so a run arriving early is queued + /// and replayed once the current one finishes — dropped instead of + /// queued, it could miss a clear that arrived mid-run. Future _adoptSystemLocale() async { + if (_adoptingSystemLocale) { + _adoptSystemLocaleQueued = true; + return; + } + _adoptingSystemLocale = true; + try { + do { + _adoptSystemLocaleQueued = false; + await _reconcileAndAdoptLocale(); + } while (_adoptSystemLocaleQueued); + } finally { + _adoptingSystemLocale = false; + } + } + + Future _reconcileAndAdoptLocale() async { final configRepo = locator(); final systemLocale = await AppLocaleService.getApplicationLocale(); final localeCode = await reconcileAppLocale( @@ -256,8 +285,12 @@ class _OpenNutriTrackerAppState extends State supportedLocales: S.supportedLocales, persistSelectedLocale: configRepo.setSelectedLocale, pushToSystem: AppLocaleService.setApplicationLocale, + readSystemTag: AppLocaleService.getApplicationLocale, markLocaleSyncSeeded: configRepo.setLocaleSyncSeeded, ); + // Persistence above deliberately ran to completion regardless of widget + // lifetime — what is saved must not depend on whether this State is + // still mounted. Only the on-screen update needs the live context. if (!mounted) return; final localeProvider = Provider.of(context, listen: false); diff --git a/test/unit_test/app_locale_sync_test.dart b/test/unit_test/app_locale_sync_test.dart index 484cb3115..715e56807 100644 --- a/test/unit_test/app_locale_sync_test.dart +++ b/test/unit_test/app_locale_sync_test.dart @@ -21,12 +21,20 @@ const _supported = [ ]; class _Recorder { + /// What a read-back of the OS tag answers after a push. By default it + /// echoes the pushed value — a platform that holds per-app overrides. + /// Pass false for one that cannot hold them, where a push no-ops. + _Recorder({this.readBackFollowsPush = true}); + + final bool readBackFollowsPush; final persisted = []; final pushed = []; int seededMarks = 0; Future persist(String? code) async => persisted.add(code); Future push(String? tag) async => pushed.add(tag); + Future<({String? tag, bool readFailed})> readBack() async => + (tag: readBackFollowsPush ? pushed.lastOrNull : null, readFailed: false); Future markSeeded() async => seededMarks++; } @@ -44,6 +52,7 @@ Future _reconcile( supportedLocales: _supported, persistSelectedLocale: recorder.persist, pushToSystem: recorder.push, + readSystemTag: recorder.readBack, markLocaleSyncSeeded: recorder.markSeeded, ); @@ -94,8 +103,11 @@ void main() { // The upgrade path: someone chose a language in the app long before this // wiring existed, so the OS has no override yet. Their choice is what - // they meant, and it seeds the system rather than being overwritten by it. - test('with no override, a saved choice seeds the system', () async { + // they meant, and it seeds the system rather than being overwritten by + // it. The read-back sees the pushed value, so seeded is recorded in the + // same call — a clear arriving any time afterwards is honoured, with no + // window in which it would read as never-seeded and be re-pushed. + test('a saved choice seeds the system and the echo records it', () async { final recorder = _Recorder(); final result = await _reconcile(recorder, saved: 'uk', system: null); @@ -105,20 +117,19 @@ void main() { expect(recorder.persisted, isEmpty); expect( recorder.seededMarks, - 0, + 1, reason: - 'seeded means the OS was SEEN holding a value; recording it on ' - 'the attempt would turn a platform that cannot hold one — where ' - 'this branch repeats forever — into a user who cleared it', + 'seeded means the OS was SEEN holding a value — here via the ' + 'read-back of the push, not the attempt itself', ); }); - // The observation that completes the migration: the pushed value comes - // back on the next read, which is when seeded may be recorded. On a - // platform with no per-app override the push no-ops, this never fires, - // and the saved choice survives every launch. + // On a platform with no per-app override the push no-ops and the + // read-back stays empty, so seeded is never recorded and the migration + // branch repeats — harmlessly — instead of the platform ever being + // mistaken for a user who cleared their override. test('a saved choice on an override-less platform survives', () async { - final recorder = _Recorder(); + final recorder = _Recorder(readBackFollowsPush: false); await _reconcile(recorder, saved: 'uk', system: null); final result = await _reconcile(recorder, saved: 'uk', system: null); @@ -129,6 +140,11 @@ void main() { isEmpty, reason: 'nothing may clear a choice the user never cleared', ); + expect( + recorder.pushed, + ['uk', 'uk'], + reason: 'the repeat is the design: pushes that never take are no-ops', + ); expect(recorder.seededMarks, 0); }); @@ -227,7 +243,7 @@ void main() { expect(result, 'it'); expect(recorder.pushed, ['it']); - expect(recorder.seededMarks, 0); + expect(recorder.seededMarks, 1); }); // A channel failure is not an answer. Deciding anything on it — most of