From 01841e9faf68d9bae28194abb7a97e0730da4463 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Thu, 2 Jul 2026 17:19:42 -0500 Subject: [PATCH 1/2] feat(onboarding): deep-link battery step nearer the per-app background-activity screen (best-effort) Spiked #150 against the AOSP Settings source (not just the reference docs): no public, non-hidden Settings action opens the "Unrestricted/Optimized/Restricted" screen directly for a specific package. ACTION_VIEW_ADVANCED_POWER_USAGE_DETAIL would, but it's @hide/non-SDK; the other public battery action, ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS, isn't package-scoped and is a worse landing for one known app. So ACTION_APPLICATION_DETAILS_SETTINGS (one tap from the target via "Battery" on stock/Pixel/AOSP) stays the primary target. BatteryOptimizationManager.settingsIntent() is restructured into a verified, never-dead-end fallback chain: try app-details, and if it doesn't resolve on some device, fall back to the battery-optimization list rather than nothing. The ordering/selection logic is extracted into a small Android-free helper so it's directly unit-testable; a new instrumented test checks the real candidate intents/order against a real PackageManager. Co-Authored-By: Claude Opus 4.8 --- .../BatteryOptimizationManagerIntentTest.kt | 48 ++++++++++++++ .../push/BatteryOptimizationManager.kt | 64 ++++++++++++++++--- .../onboarding/BatteryOptimizationScreen.kt | 8 ++- .../push/BatteryOptimizationManagerTest.kt | 49 ++++++++++++++ 4 files changed, 157 insertions(+), 12 deletions(-) create mode 100644 app/src/androidTest/kotlin/org/libremail/push/BatteryOptimizationManagerIntentTest.kt create mode 100644 app/src/test/kotlin/org/libremail/push/BatteryOptimizationManagerTest.kt diff --git a/app/src/androidTest/kotlin/org/libremail/push/BatteryOptimizationManagerIntentTest.kt b/app/src/androidTest/kotlin/org/libremail/push/BatteryOptimizationManagerIntentTest.kt new file mode 100644 index 0000000..8989de7 --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/push/BatteryOptimizationManagerIntentTest.kt @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.push + +import android.content.Context +import android.net.Uri +import android.provider.Settings +import androidx.test.core.app.ApplicationProvider +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Exercises the real [BatteryOptimizationManager.candidateIntents]/[BatteryOptimizationManager.settingsIntent] + * against a real `Context`/`PackageManager` (#150): `Intent`/`Uri`/`PackageManager.resolveActivity` are + * unmocked SDK stubs off-device, so this on-device coverage is the only place the actual candidate + * actions, order, and package scoping can be checked directly. The fallback-selection logic itself + * (which candidate wins, and that there's always a last resort) is covered Android-free by + * `BatteryOptimizationManagerTest` in the `test` source set; end-to-end launch-from-the-onboarding- + * button coverage lives in `BatteryOptimizationStepTest`. + */ +@RunWith(AndroidJUnit4::class) +class BatteryOptimizationManagerIntentTest { + + private val context = ApplicationProvider.getApplicationContext() + private val manager = BatteryOptimizationManager(context) + + @Test + fun candidateIntents_tryAppDetailsFirst_thenTheBatteryOptimizationList() { + val candidates = manager.candidateIntents() + + assertEquals(2, candidates.size) + assertEquals(Settings.ACTION_APPLICATION_DETAILS_SETTINGS, candidates[0].action) + assertEquals(Uri.fromParts("package", context.packageName, null), candidates[0].data) + assertEquals(Settings.ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS, candidates[1].action) + } + + @Test + fun settingsIntent_landsOnAppDetails_becauseItAlwaysResolvesOnARealDevice() { + // Every real/emulated Android device ships a Settings app that resolves app-details for any + // installed package, so the primary (most direct) candidate always wins here. The fallback + // exists for devices this test environment can't represent (see the class KDoc). + val intent = manager.settingsIntent() + + assertEquals(Settings.ACTION_APPLICATION_DETAILS_SETTINGS, intent.action) + assertEquals(Uri.fromParts("package", context.packageName, null), intent.data) + } +} diff --git a/app/src/main/kotlin/org/libremail/push/BatteryOptimizationManager.kt b/app/src/main/kotlin/org/libremail/push/BatteryOptimizationManager.kt index a694b7c..ff67541 100644 --- a/app/src/main/kotlin/org/libremail/push/BatteryOptimizationManager.kt +++ b/app/src/main/kotlin/org/libremail/push/BatteryOptimizationManager.kt @@ -18,8 +18,32 @@ import javax.inject.Singleton * * We deliberately do NOT use the restricted `REQUEST_IGNORE_BATTERY_OPTIMIZATIONS` permission or its * one-tap dialog: Google Play limits that permission to an approved set of use cases (rejection risk, - * see #17). Sending the user to the system screen instead needs no extra permission and is safe on - * both Play and F-Droid (#16). + * see #17). Sending the user to a system screen instead needs no extra permission and is safe on both + * Play and F-Droid (#16). + * + * ### Which screen, and why (#150) + * The onboarding goal is to land as close as possible to the per-app "Unrestricted / Optimized / + * Restricted" screen. Of the `Settings` actions that are both public (part of the SDK, not `@hide`) + * and don't require the restricted permission above, none opens that exact screen for a specific + * package — confirmed against the AOSP `Settings` source, not just the reference docs: + * - [Settings.ACTION_APPLICATION_DETAILS_SETTINGS], scoped to our package, is the closest safe option: + * on stock Android/Pixel/AOSP it lands one tap ("Battery") away from the target screen. This is + * [settingsIntent]'s primary candidate, unchanged from before this investigation. + * - [Settings.ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS] is public and needs no permission, but is + * *not* package-scoped: it opens the system-wide "Battery Optimization" app list (defaulting to a + * filter that hides already-optimized apps, so the user must switch it to "All apps" to even find + * LibreMail), and tapping an app there shows a legacy two-state Optimize/"Don't optimize" dialog that + * predates the "Restricted" option. That is a worse landing than app-details for one known app, so + * it is used only as a fallback for the rare case app-details itself doesn't resolve. + * - The one action that *would* jump straight to the target, `ACTION_VIEW_ADVANCED_POWER_USAGE_DETAIL`, + * is `@hide` in the platform source — not part of the public SDK. Using its literal string would + * mean depending on a private, unversioned implementation detail, exactly the fragility this class + * already avoids for the restricted-permission route, so it is out for the same reason. + * + * OEM skins (Samsung, Xiaomi, etc.) may rename, relocate, or move this control into a manufacturer + * settings app entirely. There is no public intent for those short of hardcoding fragile, + * version-specific OEM component names, so devices without a "Battery" entry on the app-details page + * simply keep the app-details landing rather than this class reaching for one. * * Note [isIgnoringBatteryOptimizations] reflects the Doze allowlist: it is `true` only for the * "Unrestricted" setting and `false` for *both* "Optimized" and "Restricted", so it cannot single out @@ -41,13 +65,35 @@ class BatteryOptimizationManager @Inject constructor(@ApplicationContext private } /** - * Intent to this app's system details screen, where the user can open **Battery** and choose - * **Unrestricted**. App-details is targeted (rather than the flat battery-optimization list) - * because it is the only route to the Unrestricted/Optimized/Restricted setting and lands - * directly on LibreMail. Always resolvable since API 9. + * Best-effort deep link toward the per-app battery-optimization screen (see the class KDoc for the + * rationale and OEM caveats). Returns the first of [candidateIntents] that resolves an activity on + * this device, falling back to the last candidate — today, app-details, resolvable since API 9 — + * so the caller always gets an intent that lands somewhere useful instead of a dead end. */ - fun settingsIntent(): Intent = Intent( - Settings.ACTION_APPLICATION_DETAILS_SETTINGS, - Uri.fromParts("package", context.packageName, null), + fun settingsIntent(): Intent = + firstResolvableOrElseLast(candidateIntents()) { it.resolveActivity(context.packageManager) != null } + + /** + * Candidate settings screens, most specific/direct first. `internal` (rather than private) so the + * candidate list itself — actions, order, and the package scoping — is directly checkable from a + * test. See the class KDoc for why each candidate is here, in this order. + */ + internal fun candidateIntents(): List = listOf( + Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS, Uri.fromParts("package", context.packageName, null)), + Intent(Settings.ACTION_IGNORE_BATTERY_OPTIMIZATION_SETTINGS), ) } + +/** + * Returns the first element of [candidates] for which [resolves] holds, or the last element if none do + * — so a caller building a best-effort fallback chain always gets something usable rather than `null`. + * Top-level and `internal` (rather than folded into [BatteryOptimizationManager.settingsIntent]) so + * this selection/fallback logic is exhaustively unit-testable without touching real Android intent + * resolution, the same reasoning as [BatteryPromptDecision]. + * + * @param candidates must be non-empty, ordered most-preferred first. + */ +internal fun firstResolvableOrElseLast(candidates: List, resolves: (T) -> Boolean): T { + require(candidates.isNotEmpty()) { "candidates must not be empty" } + return candidates.firstOrNull(resolves) ?: candidates.last() +} diff --git a/app/src/main/kotlin/org/libremail/ui/onboarding/BatteryOptimizationScreen.kt b/app/src/main/kotlin/org/libremail/ui/onboarding/BatteryOptimizationScreen.kt index 0314c5a..9ad77bc 100644 --- a/app/src/main/kotlin/org/libremail/ui/onboarding/BatteryOptimizationScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/onboarding/BatteryOptimizationScreen.kt @@ -35,9 +35,11 @@ import org.libremail.R /** * Final onboarding step (shown only when needed, see [OnboardingViewModel.batteryPromptNeeded]): * invites the user to allow unrestricted background/battery usage so push and periodic sync aren't - * throttled by Doze. **Take me there** deep-links to the system screen (no restricted permission); - * **Not now** skips. Either way [onFinish] proceeds to the inbox. On returning from Settings the - * status is re-read and, if the app is now unrestricted, the screen reflects that with a "done" state. + * throttled by Doze. **Take me there** deep-links as directly as possible toward the per-app battery + * screen (see [org.libremail.push.BatteryOptimizationManager] for the best-effort fallback chain; no + * restricted permission is ever used); **Not now** skips. Either way [onFinish] proceeds to the inbox. + * On returning from Settings the status is re-read and, if the app is now unrestricted, the screen + * reflects that with a "done" state. * * @param viewModel the graph-scoped onboarding view model (holds live battery status + the flag). * @param onFinish leaves onboarding for the inbox; the caller also marks the prompt handled. diff --git a/app/src/test/kotlin/org/libremail/push/BatteryOptimizationManagerTest.kt b/app/src/test/kotlin/org/libremail/push/BatteryOptimizationManagerTest.kt new file mode 100644 index 0000000..54b2f2b --- /dev/null +++ b/app/src/test/kotlin/org/libremail/push/BatteryOptimizationManagerTest.kt @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.push + +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertFailsWith + +/** + * Unit tests for the `firstResolvableOrElseLast` fallback-selection helper that backs + * [BatteryOptimizationManager.settingsIntent] (#150). Deliberately exercised with plain `String` + * candidates rather than real `Intent`s: `Intent`/`Uri`/`PackageManager` are unmocked Android SDK stubs + * in a JVM unit test and throw on use, so the selection/ordering logic is kept generic and Android-free + * specifically so it's testable here (see its KDoc). The real candidate intents and end-to-end + * resolution are covered by the instrumented `BatteryOptimizationStepTest`. + */ +class BatteryOptimizationManagerTest { + + @Test + fun `returns the first candidate that resolves`() { + assertEquals("b", firstResolvableOrElseLast(listOf("a", "b", "c")) { it == "b" }) + } + + @Test + fun `returns the only candidate when it is the first checked, even if it would not resolve`() { + assertEquals("only", firstResolvableOrElseLast(listOf("only")) { it == "only" }) + } + + @Test + fun `falls back to the last candidate when none resolve, rather than dead-ending`() { + assertEquals("c", firstResolvableOrElseLast(listOf("a", "b", "c")) { false }) + } + + @Test + fun `falls back to the last candidate even when it is the only one and it does not resolve`() { + assertEquals("only", firstResolvableOrElseLast(listOf("only")) { false }) + } + + @Test + fun `prefers an earlier resolving candidate over a later one, even if both resolve`() { + assertEquals("first", firstResolvableOrElseLast(listOf("first", "second")) { true }) + } + + @Test + fun `rejects an empty candidate list rather than silently returning nothing`() { + assertFailsWith { + firstResolvableOrElseLast(emptyList()) { true } + } + } +} From 30c5251a68522fe791e037d5e80dd4527894c022 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Thu, 2 Jul 2026 17:46:59 -0500 Subject: [PATCH 2/2] feat(settings): reorder the Advanced settings section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reorder the Advanced block's SwitchRows to Push Mail, Load Remote Images, Encrypt Local Cache, Require Screen Lock, then Allow insecure STARTTLS fallback (moved last). Relocate the Background battery usage row out of Advanced entirely and into the main settings list, directly above local retention ("Storage on this device"), since it's common enough (OEM battery optimization delaying push mail) that it shouldn't be hidden behind "Advanced". Pure composable placement — no string changes, no behavior change to any toggle. Closes #162 Co-Authored-By: Claude Opus 4.8 --- .../libremail/ui/settings/SettingsScreen.kt | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/app/src/main/kotlin/org/libremail/ui/settings/SettingsScreen.kt b/app/src/main/kotlin/org/libremail/ui/settings/SettingsScreen.kt index 9a7cdd8..94da453 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/SettingsScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/SettingsScreen.kt @@ -203,6 +203,22 @@ fun SettingsScreen( ) HorizontalDivider() + // Background battery usage (#162): relocated out of the collapsible Advanced section and + // into the main list, directly above local retention — OEM battery optimization delaying + // push mail is common enough that it shouldn't be hidden behind "Advanced". + ClickRow( + title = stringResource(R.string.settings_adv_battery), + subtitle = stringResource( + if (batteryUnrestricted) { + R.string.settings_adv_battery_unrestricted + } else { + R.string.settings_adv_battery_optimized + }, + ), + onClick = { runCatching { context.startActivity(viewModel.batterySettingsIntent()) } }, + ) + HorizontalDivider() + // Global device-only retention default (issue #13); accounts may override it. RetentionSection( count = settings.retentionCount, @@ -230,23 +246,6 @@ fun SettingsScreen( checked = settings.pushIdle, onCheckedChange = viewModel::setPushIdle, ) - ClickRow( - title = stringResource(R.string.settings_adv_battery), - subtitle = stringResource( - if (batteryUnrestricted) { - R.string.settings_adv_battery_unrestricted - } else { - R.string.settings_adv_battery_optimized - }, - ), - onClick = { runCatching { context.startActivity(viewModel.batterySettingsIntent()) } }, - ) - SwitchRow( - title = stringResource(R.string.settings_adv_starttls), - checked = settings.allowStartTls, - onCheckedChange = viewModel::setAllowStartTls, - subtitle = stringResource(R.string.settings_adv_starttls_summary), - ) SwitchRow( title = stringResource(R.string.settings_adv_remote_images), checked = settings.loadRemoteImages, @@ -264,6 +263,12 @@ fun SettingsScreen( onCheckedChange = viewModel::setAppLock, subtitle = stringResource(R.string.settings_adv_app_lock_summary), ) + SwitchRow( + title = stringResource(R.string.settings_adv_starttls), + checked = settings.allowStartTls, + onCheckedChange = viewModel::setAllowStartTls, + subtitle = stringResource(R.string.settings_adv_starttls_summary), + ) } } }