Skip to content

fix(theme): decode the custom palettes through generated serializers so their role keys survive minification - #1049

Merged
A-EDev merged 2 commits into
mainfrom
fix/custom-theme-palette-roles
Sep 12, 2026
Merged

A-EDev merged 2 commits into
mainfrom
fix/custom-theme-palette-roles

Conversation

@A-EDev

@A-EDev A-EDev commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Summary

Custom themes have never applied in a release build, and since 1f88972d they crash the app at launch. Both are the same defect.

LocalDataManager read the saved palettes with gson.fromJson(raw, CustomThemePalettes::class.java). Gson recovers Map<CustomColorRole, Long> from the field's generic signature, and io.github.aedev.flow.ui.theme is named by no keep rule in app/proguard-rules.pro — so R8 strips that signature, Gson reads the map raw, and hands back String keys. Confirmed in the R8 output of :app:minifyGithubReleaseWithR8, not inferred:

CustomThemeColors.values    ->  type: 'Ljava/util/Map;'                                    (no Signature)
SponsorBlockSegment.segment ->  Signature { "Ljava/util/List<" "Ljava/lang/Float;" ">;" }  (kept class)

No dalvik.annotation.Signature anywhere in the three dex files mentions CustomColorRole.

Two consequences, one cause:

  • Every CustomThemeColors.colorOf(role) looked an enum up in a String-keyed map, missed, and fell back to a default — the palette silently never applied ([Bug]: Custom Theme not saving at all #964).
  • 1f88972d added WidgetThemeSignature.persistedForm(), which sorts those entries by it.key.name. It is the first code to actually touch a key, so it throws ClassCastException: java.lang.String cannot be cast to CustomColorRole on Dispatchers.Main.immediate while FlowApp collects the DataStore flow — an unrecoverable crash at launch for anyone who has ever opened the custom theme editor. Reported in the discussion on a Samsung SM-A165F, and bisected there to this build (#1288 fine, #1289 broken).

The fix

Palette persistence moves to a new data/local/CustomThemePalettePersistence.kt that encodes and decodes through kotlinx.serialization, calling StoredPalettes.serializer() explicitly. There is no reflection and no generic signature to strip, so this cannot regress if a keep rule moves again — which is the same failure mode as #996, now hit for the second time. Roles are stored by name, so a role the app no longer knows drops out of the palette instead of failing the whole decode.

The wire format is unchanged — Gson already wrote role names as JSON object keys — so existing saves and settings backups read straight back. No user has to re-create their theme.

Fixing the read also un-breaks the write path, so the second commit of the reformat aside, the diff is five lines of behaviour.

style(data): apply the project ktlint rules to LocalDataManager is kept as its own commit: touching the file made it ratchet-eligible, and separating it keeps the actual fix reviewable.

Related issue

Closes #964

Change type

  • Bug fix
  • Feature
  • Refactor or maintenance
  • Build, packaging, or CI
  • Documentation

Validation

  • ./gradlew :app:assembleGithubDebug
  • ./gradlew :app:testGithubDebugUnitTest — full suite green
  • I ran any additional flavor-specific build or test tasks affected by this change.
  • I manually tested the affected behavior on an Android device or emulator.

Also run:

  • :app:compileFossDebugKotlin — passes.
  • :app:minifyGithubReleaseWithR8 plus dexdump -a over the three dex files — this is the evidence for the root cause quoted above.
  • ktlint via a scoped spotlessKotlinApply over the four touched files; the build script was restored afterwards.

New tests, 8 in total:

  • CustomThemePalettePersistenceTest — round trip, decoding a palette written by the previous Gson reader, an unknown role dropping out, the legacy comma-separated migration, and unreadable JSON falling back to defaults.
  • WidgetThemeSignatureTest — the direct regression test: persistedForm() over a decoded palette signs without a cast failure, is stable across two calls, and changes when a colour changes.

Test device and Android version: none — not yet run on hardware. The crash and the theme both want a device pass before release.

Risk and compatibility

  • No preference migration needed. The stored JSON is already correct; only the reader was wrong. Settings backups written by any previous version restore unchanged.
  • A palette section missing from the JSON entirely now falls back to that variant's defaults rather than to an empty map. The editor always writes all three, so this only affects hand-edited or truncated data.
  • No Room schema change, no new dependency, no change to playback, background work, or network behaviour.
  • WidgetThemeSignature.persistedForm() is left unguarded on the main dispatcher. Wrapping it would hide the next bug rather than fix one, so the cause is fixed instead — worth a deliberate decision if we would rather it never be able to take the app down.
  • The change does not introduce secrets, private data, or unexpected telemetry.
  • New user-facing text uses Android string resources. — no new user-facing text.
  • Dependency and lockfile changes are intentional and limited to this PR. — none.
  • Room schema changes include the required version bump and migration, or this PR does not change the Room schema.
  • Breaking changes and upgrade steps are clearly documented. — none.

Copilot AI lite review requested due to automatic review settings September 11, 2026 22:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@A-EDev
A-EDev merged commit 210b753 into main Sep 12, 2026
5 checks passed
@A-EDev A-EDev added the merged Pull request verified, tested and merged label Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged Pull request verified, tested and merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Custom Theme not saving at all

2 participants