Skip to content

fix: Restoring backup from file doesn't respect 'swipe actions' setting#10893

Open
shamim-emon wants to merge 15 commits intothunderbird:mainfrom
shamim-emon:fix-issue-10621
Open

fix: Restoring backup from file doesn't respect 'swipe actions' setting#10893
shamim-emon wants to merge 15 commits intothunderbird:mainfrom
shamim-emon:fix-issue-10621

Conversation

@shamim-emon
Copy link
Copy Markdown
Collaborator

@shamim-emon shamim-emon commented Apr 18, 2026

🐛 Problem

  • While restoring backup from files , GeneralSettingsWriter uses legacy preference system to store GeneralSettings values.
  • New PreferenceManagers for GeneralSettings don't listen to this update and so GeneralSettings flow does not react to this change and UI does not get update without restarting the app.

🛠️ Solution

  • Made all preferenceManagers subscribe to PreferenceChangeBroker.
  • Notify all preferenceManagers upon GeneralSetting restore on GeneralSettingsWriter.

@github-actions

This comment was marked as resolved.

@shamim-emon shamim-emon added the report: include Include changes in user-facing reports. label Apr 18, 2026
@wmontwe wmontwe requested review from wmontwe and removed request for rafaeltonholo April 24, 2026 10:11
@wmontwe wmontwe assigned wmontwe and unassigned rafaeltonholo Apr 24, 2026
Copy link
Copy Markdown
Member

@wmontwe wmontwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the PreferenceChangeBroker solved the issue, but it causes side effects.

This is using a simple broadcaster that notifies all subscribers and therefore could lead to a broadcast storm as now every settings related manager reacts to any change. Just add a log to every receive and then change the "animation" setting you will see 11 managers updating.

While the individual reloads are relatively small, they quickly add up causing simultaneous updates across multiple managers that end up aggregated as GeneralSettings with multiple updates to consumers in the app.

Some ideas:

These are rough ideas and maybe not doable or adding to much complexity.

  1. Keep the change broker, but scope the emissions (adds complexity)
enum class PreferenceScope { ALL, DISPLAY, PRIVACY, NETWORK, DEBUGGING }

interface PreferenceChangePublisher {
    fun publish(scope: PreferenceScope = PreferenceScope.ALL)
}

fun interface PreferenceChangeSubscriber {
    fun receive(scope: PreferenceScope)
}

// In a manager:
override fun receive(scope: PreferenceScope) {
    if (scope == PreferenceScope.ALL || scope == PreferenceScope.PRIVACY) {
        configState.update { loadConfig() }
    }
}
  1. Use key-based notifications
fun interface PreferenceChangeSubscriber {
    fun receive(changedKey: String)
}

// In a manager:
override fun receive(changedKey: String) {
    if (myOwnedKeys.contains(changedKey)) {
        configState.update { loadConfig() }
    }
}
  1. Use a reactive storage
class DefaultNotificationPreferenceManager(...) {
    val configFlow = combine(
        storage.observeBoolean(KEY_QUIET_TIME),
        storage.observeString(KEY_NOTIFICATION_VIBRATE)
    ) { quietTime, vibrate -> 
        NotificationSettings(quietTime, vibrate) 
    }.stateIn(scope, Eagerly, initialValue)
}

But our storage is not reactive yet.


Other ideas are welcome

@shamim-emon shamim-emon marked this pull request as draft April 30, 2026 14:51
@shamim-emon shamim-emon marked this pull request as ready for review May 1, 2026 05:46
@shamim-emon shamim-emon added report: include Include changes in user-facing reports. and removed report: include Include changes in user-facing reports. labels May 1, 2026
@shamim-emon shamim-emon added feature-flag Changes guarded by a feature flag. Please add a comment stating the name: "feature-flag: abc" report: include Include changes in user-facing reports. and removed feature-flag Changes guarded by a feature flag. Please add a comment stating the name: "feature-flag: abc" labels May 1, 2026
@shamim-emon shamim-emon requested a review from wmontwe May 1, 2026 08:22
@shamim-emon shamim-emon removed the report: include Include changes in user-facing reports. label May 1, 2026
@shamim-emon shamim-emon added report: include Include changes in user-facing reports. and removed report: include Include changes in user-facing reports. labels May 1, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Validation Passed: All report and feature-flag labels are correctly set.

@shamim-emon shamim-emon added the report: include Include changes in user-facing reports. label May 4, 2026
shamim-emon added 15 commits May 6, 2026 11:18
…renceChangeBroker and stop implementing PreferenceChangeSubscriber.
… to ensure correct behavior of the updated PreferenceChangeSubscriber
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

report: include Include changes in user-facing reports.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[k-9 mail] restoring backup from file doesn't respect 'swipe actions' setting

3 participants