diff --git a/app/src/androidTest/kotlin/org/libremail/ui/onboarding/BatteryOptimizationStepTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/onboarding/BatteryOptimizationStepTest.kt index a0b0852..99d8c95 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/onboarding/BatteryOptimizationStepTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/onboarding/BatteryOptimizationStepTest.kt @@ -13,6 +13,7 @@ import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.onAllNodesWithText import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick +import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.NavType import androidx.navigation.compose.NavHost @@ -75,6 +76,7 @@ class BatteryOptimizationStepTest { BatteryOptimizationManager(context), ContactsPermissionManager(context), settingsRepository, + SavedStateHandle(), ) onboarding.onAccountAdded(FIRST_ACCOUNT_ID) diff --git a/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt b/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt index 52eb3a8..6e7ef6f 100644 --- a/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt +++ b/app/src/androidTest/kotlin/org/libremail/ui/onboarding/OnboardingFlowTest.kt @@ -110,6 +110,7 @@ class OnboardingFlowTest { BatteryOptimizationManager(appContext), ContactsPermissionManager(appContext), SettingsRepository(appContext), + SavedStateHandle(), ) composeTestRule.setContent { LibreMailTheme(darkTheme = false, dynamicColor = false) { diff --git a/app/src/main/kotlin/org/libremail/ui/accountsetup/AccountSetupViewModel.kt b/app/src/main/kotlin/org/libremail/ui/accountsetup/AccountSetupViewModel.kt index 48e63f8..1ffdf01 100644 --- a/app/src/main/kotlin/org/libremail/ui/accountsetup/AccountSetupViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/accountsetup/AccountSetupViewModel.kt @@ -63,7 +63,10 @@ class AccountSetupViewModel @Inject constructor( fun onOutlookResult(data: Intent?) { if (data == null) { - _state.update { it.copy(error = "Microsoft sign-in was cancelled") } + // A null result Intent is a normal user cancel: backing out of the Microsoft sign-in tab + // returns RESULT_CANCELED with no data. That is expected, not a failure, so it is a no-op — + // surfacing an error snackbar for a deliberate cancel is just noise (#308). + AppLog.d(TAG, "Outlook sign-in cancelled by the user; no-op") return } viewModelScope.launch { diff --git a/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt b/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt index cc48459..c7064bf 100644 --- a/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt +++ b/app/src/main/kotlin/org/libremail/ui/compose/RichTextEditor.kt @@ -262,9 +262,10 @@ private fun FormattingToolbar( onFont: (String?) -> Unit, onInsertImage: (() -> Unit)?, ) { - val content = value.annotatedString.toRichContent() - val start = value.selection.min - val end = value.selection.max + // Parsing the field into RichTextContent and the dozen selection scans that light up the buttons is + // a per-keystroke hot path; memoize the whole derivation on the field value so a recomposition that + // doesn't change it reuses the result instead of re-parsing + re-scanning (#308). + val toolbar = remember(value) { toolbarStateOf(value) } Row( modifier = Modifier .fillMaxWidth() @@ -276,63 +277,61 @@ private fun FormattingToolbar( FormatButton( label = "B", description = stringResource(R.string.format_bold), - active = RichTextEditing.isStyled(content, start, end, RichStyle.Bold), + active = toolbar.bold, fontWeight = FontWeight.Bold, onClick = { onToggleStyle(RichStyle.Bold) }, ) FormatButton( label = "I", description = stringResource(R.string.format_italic), - active = RichTextEditing.isStyled(content, start, end, RichStyle.Italic), + active = toolbar.italic, fontStyle = FontStyle.Italic, onClick = { onToggleStyle(RichStyle.Italic) }, ) FormatButton( label = "U", description = stringResource(R.string.format_underline), - active = RichTextEditing.isStyled(content, start, end, RichStyle.Underline), + active = toolbar.underline, underline = true, onClick = { onToggleStyle(RichStyle.Underline) }, ) FormatButton( label = "S", description = stringResource(R.string.format_strikethrough), - active = RichTextEditing.isStyled(content, start, end, RichStyle.Strikethrough), + active = toolbar.strikethrough, strikethrough = true, onClick = { onToggleStyle(RichStyle.Strikethrough) }, ) - val fontColorArgb = RichTextEditing.styleAt(content, start, end, RichStyle.FontColor::class.java)?.argb FormatButton( label = "A", description = stringResource(R.string.format_color), - active = fontColorArgb != null, - tint = fontColorArgb?.let { Color(it) }, + active = toolbar.fontColorArgb != null, + tint = toolbar.fontColorArgb?.let { Color(it) }, onClick = onFontColor, ) - val highlightArgb = RichTextEditing.styleAt(content, start, end, RichStyle.Highlight::class.java)?.argb FormatButton( label = "H", description = stringResource(R.string.format_highlight), active = false, - swatchColor = highlightArgb?.let { Color(it) }, + swatchColor = toolbar.highlightArgb?.let { Color(it) }, onClick = onHighlight, ) FormatButton( label = "•", description = stringResource(R.string.format_bullet_list), - active = RichTextEditing.hasBlock(content, start, end, BlockMarker.BULLET), + active = toolbar.bullet, onClick = { onToggleBlock(BlockMarker.BULLET) }, ) FormatButton( label = "1.", description = stringResource(R.string.format_numbered_list), - active = RichTextEditing.hasBlock(content, start, end, BlockMarker.ORDERED), + active = toolbar.ordered, onClick = { onToggleBlock(BlockMarker.ORDERED) }, ) FormatButton( label = "❝", description = stringResource(R.string.format_quote), - active = RichTextEditing.hasBlock(content, start, end, BlockMarker.QUOTE), + active = toolbar.quote, onClick = { onToggleBlock(BlockMarker.QUOTE) }, ) FormatButton( @@ -347,12 +346,10 @@ private fun FormattingToolbar( // so its click lands on the button's on-screen center. Any control inserted *before* the block // buttons shifts them right and can push the bullet past the viewport, making that tap miss — // so these wider controls are appended last, leaving every pre-existing button in its tested spot. - val fontCss = RichTextEditing.styleAt(content, start, end, RichStyle.FontFamily::class.java)?.css - FontPicker(selectedCss = fontCss, onSelect = onFont) - val fontSizePt = RichTextEditing.styleAt(content, start, end, RichStyle.FontSize::class.java)?.pt - FontSizePicker(selectedPt = fontSizePt, onSelect = onFontSize) + FontPicker(selectedCss = toolbar.fontCss, onSelect = onFont) + FontSizePicker(selectedPt = toolbar.fontSizePt, onSelect = onFontSize) ParagraphAlignmentControl( - selected = RichTextEditing.alignmentAt(content, start, end), + selected = toolbar.alignment, onSelect = onAlignment, ) // The image button is appended last (like the other trailing controls) so it never shifts the @@ -368,6 +365,53 @@ private fun FormattingToolbar( } } +/** + * The [FormattingToolbar]'s button state derived from the field's current [TextFieldValue]: the + * active/inactive toggles plus the currently-applied color/font/size/alignment the pickers read back. + * Bundled so the parse + selection scans that produce them can be computed once per field value and + * memoized, keeping the per-keystroke toolbar off the re-parse-everything path (#308). + */ +internal data class ToolbarState( + val bold: Boolean, + val italic: Boolean, + val underline: Boolean, + val strikethrough: Boolean, + val fontColorArgb: Int?, + val highlightArgb: Int?, + val bullet: Boolean, + val ordered: Boolean, + val quote: Boolean, + val fontCss: String?, + val fontSizePt: Int?, + val alignment: RichAlign?, +) + +/** + * Parses [value] into [RichTextContent] once and runs every toolbar selection scan over that single + * parse, so [FormattingToolbar] derives all its button state in one pass instead of re-parsing the + * whole body and re-scanning per button on each recomposition (#308). Pure over plain + * [TextFieldValue] so it is JVM-unit-testable. + */ +internal fun toolbarStateOf(value: TextFieldValue): ToolbarState { + val content = value.annotatedString.toRichContent() + val start = value.selection.min + val end = value.selection.max + return ToolbarState( + bold = RichTextEditing.isStyled(content, start, end, RichStyle.Bold), + italic = RichTextEditing.isStyled(content, start, end, RichStyle.Italic), + underline = RichTextEditing.isStyled(content, start, end, RichStyle.Underline), + strikethrough = RichTextEditing.isStyled(content, start, end, RichStyle.Strikethrough), + fontColorArgb = RichTextEditing.styleAt(content, start, end, RichStyle.FontColor::class.java)?.argb, + highlightArgb = RichTextEditing.styleAt(content, start, end, RichStyle.Highlight::class.java)?.argb, + bullet = RichTextEditing.hasBlock(content, start, end, BlockMarker.BULLET), + ordered = RichTextEditing.hasBlock(content, start, end, BlockMarker.ORDERED), + quote = RichTextEditing.hasBlock(content, start, end, BlockMarker.QUOTE), + fontCss = RichTextEditing.styleAt(content, start, end, RichStyle.FontFamily::class.java)?.css, + fontSizePt = RichTextEditing.styleAt(content, start, end, RichStyle.FontSize::class.java)?.pt, + alignment = RichTextEditing.alignmentAt(content, start, end), + ) +} + @Composable private fun FormatButton( label: String, diff --git a/app/src/main/kotlin/org/libremail/ui/lock/AppLockGateHost.kt b/app/src/main/kotlin/org/libremail/ui/lock/AppLockGateHost.kt index a45cc01..b2c7ebe 100644 --- a/app/src/main/kotlin/org/libremail/ui/lock/AppLockGateHost.kt +++ b/app/src/main/kotlin/org/libremail/ui/lock/AppLockGateHost.kt @@ -16,6 +16,7 @@ import androidx.compose.runtime.setValue import androidx.compose.ui.Modifier import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.res.stringResource +import androidx.compose.ui.semantics.clearAndSetSemantics import androidx.core.content.ContextCompat import androidx.fragment.app.FragmentActivity import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel @@ -70,7 +71,21 @@ fun AppLockGateHost(viewModel: AppLockViewModel = hiltViewModel(), content: @Com LaunchedEffect(uiState) { if (uiState is AppLockUiState.Unlocked) hasEverUnlocked = true } Box(modifier = Modifier.fillMaxSize()) { - if (hasEverUnlocked) content() + // While the app is covered (Checking/Locked) the opaque LockCover hides the content visually and + // blocks its input, but the content stays in the composition (so its state survives a re-lock). + // Clear the covered subtree out of the semantics/accessibility tree so an accessibility service + // (e.g. TalkBack) can't traverse the occluded mailbox/compose nodes behind the cover (#308). When + // Unlocked the modifier is dropped, restoring the content's full semantics. + val contentCovered = uiState !is AppLockUiState.Unlocked + Box( + modifier = if (contentCovered) { + Modifier.fillMaxSize().clearAndSetSemantics { } + } else { + Modifier.fillMaxSize() + }, + ) { + if (hasEverUnlocked) content() + } when (val state = uiState) { AppLockUiState.Unlocked -> Unit diff --git a/app/src/main/kotlin/org/libremail/ui/onboarding/OnboardingViewModel.kt b/app/src/main/kotlin/org/libremail/ui/onboarding/OnboardingViewModel.kt index 079e31b..0a7408b 100644 --- a/app/src/main/kotlin/org/libremail/ui/onboarding/OnboardingViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/onboarding/OnboardingViewModel.kt @@ -2,6 +2,7 @@ package org.libremail.ui.onboarding import android.content.Intent +import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel @@ -13,6 +14,8 @@ import org.libremail.contacts.ContactsPermissionManager import org.libremail.data.settings.SettingsRepository import org.libremail.push.BatteryOptimizationManager import org.libremail.push.BatteryPromptDecision +import org.libremail.reporting.AppLog +import org.libremail.reporting.accountLogRef import javax.inject.Inject /** @@ -28,11 +31,18 @@ class OnboardingViewModel @Inject constructor( private val batteryOptimizationManager: BatteryOptimizationManager, private val contactsPermissionManager: ContactsPermissionManager, private val settingsRepository: SettingsRepository, + private val savedStateHandle: SavedStateHandle, ) : ViewModel() { - /** The id of the first account added this session, or null if none has been added yet. */ - var firstAddedAccountId: String? = null - private set + /** + * The id of the first account added this session, or null if none has been added yet. Held in + * [SavedStateHandle] rather than a plain field so it survives process death: this ViewModel is + * scoped to the onboarding nav-graph back-stack entry, whose saved state is restored after a + * process kill. Without that, a kill mid-onboarding — e.g. the excursion to system battery + * settings on the opt-in step — would drop the id and finish onto the unfiltered mailbox instead + * of the first account's inbox, defeating #30. + */ + val firstAddedAccountId: String? get() = savedStateHandle[KEY_FIRST_ACCOUNT_ID] private val _batteryPromptNeeded = MutableStateFlow(null) @@ -82,7 +92,9 @@ class OnboardingViewModel @Inject constructor( /** Records a freshly added account. Only the first one sticks — later adds don't overwrite it. */ fun onAccountAdded(accountId: String) { if (firstAddedAccountId == null) { - firstAddedAccountId = accountId + savedStateHandle[KEY_FIRST_ACCOUNT_ID] = accountId + // PII-free: the id embeds the email, so reference it only via its non-reversible log ref. + AppLog.i(TAG, "onboarding recorded first account ${accountLogRef(accountId)}") } } @@ -128,4 +140,11 @@ class OnboardingViewModel @Inject constructor( fun markContactsPromptHandled() { viewModelScope.launch { settingsRepository.setContactsPromptHandled(true) } } + + private companion object { + const val TAG = "OnboardingVM" + + /** SavedStateHandle key for [firstAddedAccountId], persisted across process death (#308). */ + const val KEY_FIRST_ACCOUNT_ID = "onboarding_first_added_account_id" + } } diff --git a/app/src/main/kotlin/org/libremail/ui/reader/ReaderScreen.kt b/app/src/main/kotlin/org/libremail/ui/reader/ReaderScreen.kt index a53d204..d962968 100644 --- a/app/src/main/kotlin/org/libremail/ui/reader/ReaderScreen.kt +++ b/app/src/main/kotlin/org/libremail/ui/reader/ReaderScreen.kt @@ -85,6 +85,7 @@ fun ReaderScreen( val noAppMessage = stringResource(R.string.attachment_no_app) val downloadFailedTemplate = stringResource(R.string.attachment_download_failed) val replyFailedMessage = stringResource(R.string.reader_reply_failed) + val starFailedMessage = stringResource(R.string.reader_star_failed) LaunchedEffect(state.deleted) { if (state.deleted) onBack() @@ -105,6 +106,8 @@ fun ReaderScreen( is ReaderEvent.ComposeFailed -> snackbarHostState.showSnackbar(event.message ?: replyFailedMessage) + + ReaderEvent.StarFailed -> snackbarHostState.showSnackbar(starFailedMessage) } } } diff --git a/app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt b/app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt index 01b1e2e..0ac70d7 100644 --- a/app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/reader/ReaderViewModel.kt @@ -50,6 +50,9 @@ sealed interface ReaderEvent { /** Building the reply/forward draft failed; the screen surfaces [message] (or a generic fallback). */ data class ComposeFailed(val message: String?) : ReaderEvent + + /** Persisting a star toggle failed; the optimistic flip was rolled back and the screen notifies. */ + data object StarFailed : ReaderEvent } @HiltViewModel @@ -140,7 +143,19 @@ class ReaderViewModel @Inject constructor( val message = _state.value.message ?: return val starred = !message.isStarred _state.update { it.copy(message = message.copy(isStarred = starred)) } - viewModelScope.launch { repository.setStarred(messageId, starred) } + viewModelScope.launch { + repository.setStarred(messageId, starred).onFailure { e -> + // The optimistic flip already updated the UI; the persist failed, so reconcile by rolling + // it back (the star would otherwise stay stuck in a state the store never accepted) and + // notify the screen. Revert to the pre-toggle value (!starred) of the current message. + AppLog.w(READER_TAG, "toggleStar persist failed; rolling back optimistic star", e) + _state.update { current -> + val shown = current.message ?: return@update current + current.copy(message = shown.copy(isStarred = !starred)) + } + _events.send(ReaderEvent.StarFailed) + } + } } fun loadRemoteImages() = _state.update { it.copy(loadRemoteImages = true) } diff --git a/app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt b/app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt index 7ac058b..33a47ea 100644 --- a/app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt +++ b/app/src/main/kotlin/org/libremail/ui/settings/SettingsViewModel.kt @@ -2,9 +2,12 @@ package org.libremail.ui.settings import android.content.Intent +import androidx.annotation.VisibleForTesting import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow @@ -12,6 +15,7 @@ import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import org.libremail.R import org.libremail.contacts.ContactsPermissionManager import org.libremail.data.security.AppLockManager @@ -62,6 +66,12 @@ class SettingsViewModel @Inject constructor( /** Whether this app is exempt from battery optimization ("Unrestricted"). */ val batteryUnrestricted: StateFlow = _batteryUnrestricted.asStateFlow() + // Blocking Keystore/DataStore work on the app-lock disable path runs here — off the main + // dispatcher — mirroring AppLockViewModel's threading policy so the reseal never runs Keystore + // crypto on the main thread (#308). Injectable so unit tests can pin it to their scheduler. + @VisibleForTesting + internal var defaultDispatcher: CoroutineDispatcher = Dispatchers.Default + fun toggleAdvanced() = _advancedExpanded.update { !it } /** Persist the account order the user produced by dragging in Settings (issue #164). */ @@ -110,13 +120,19 @@ class SettingsViewModel @Inject constructor( // Reseal under the master key whenever an auth seal actually exists — gate on the seal, not // the encryptCache setting (a separate store that can already be off while the on-disk DB is // still auth-sealed). Do it BEFORE dropping the gate, or the next launch can't open the - // cache. If it fails, keep app-lock on rather than strand the passphrase. - if (databaseKeyStore.hasAuthSealedPassphrase()) { - if (runCatching { databaseKeyStore.sealWithMaster() }.isFailure) { - _appLockMessage.value = R.string.app_lock_disable_failed - return@update + // cache. If it fails, keep app-lock on rather than strand the passphrase. The Keystore/ + // DataStore reseal is blocking crypto, so it runs off the main thread (#308). + val resealed = withContext(defaultDispatcher) { + if (databaseKeyStore.hasAuthSealedPassphrase()) { + runCatching { databaseKeyStore.sealWithMaster() }.isSuccess + } else { + true } } + if (!resealed) { + _appLockMessage.value = R.string.app_lock_disable_failed + return@update + } settingsRepository.setAppLock(false) } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 8c6b86a..4fd05e4 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -151,6 +151,7 @@ Reply Couldn\'t start the reply + Couldn\'t update star Attachments Downloaded Available offline diff --git a/app/src/test/kotlin/org/libremail/ui/accountsetup/AccountSetupViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/accountsetup/AccountSetupViewModelTest.kt index 332bf07..8ff4571 100644 --- a/app/src/test/kotlin/org/libremail/ui/accountsetup/AccountSetupViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/accountsetup/AccountSetupViewModelTest.kt @@ -124,13 +124,18 @@ class AccountSetupViewModelTest { } @Test - fun `a cancelled sign-in (null result) surfaces a cancellation message`() { + fun `a cancelled sign-in (null result) is a no-op, not an error`() { val vm = viewModel() vm.onOutlookResult(null) - assertEquals("Microsoft sign-in was cancelled", vm.state.value.error) + // #308: a normal cancel (backing out of the sign-in tab → RESULT_CANCELED, null data) must not + // raise an error snackbar; it leaves the screen untouched (idle, no error) and only breadcrumbs. + assertNull(vm.state.value.error) assertEquals(SetupStatus.IDLE, vm.state.value.status) + val entry = logBuffer.snapshot().single() + assertEquals('D', entry.level) + assertTrue(entry.message.contains("cancelled"), entry.message) } @Test @@ -263,7 +268,9 @@ class AccountSetupViewModelTest { @Test fun `consumeError clears a surfaced error`() { val vm = viewModel() - vm.onOutlookResult(null) + // Surface a real error first (a cancel is now a no-op), then confirm consumeError clears it. + vm.onOutlookLaunchFailed(IllegalStateException("appauth exploded")) + assertEquals("appauth exploded", vm.state.value.error) vm.consumeError() diff --git a/app/src/test/kotlin/org/libremail/ui/compose/RichTextEditorTest.kt b/app/src/test/kotlin/org/libremail/ui/compose/RichTextEditorTest.kt index 3746d0c..8b033bb 100644 --- a/app/src/test/kotlin/org/libremail/ui/compose/RichTextEditorTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/compose/RichTextEditorTest.kt @@ -348,6 +348,37 @@ class RichTextEditorTest { assertTrue(isBulletActive(bothLines.copy(selection = fullText))) } + @Test + fun `toolbarStateOf derives every button's state from the field value in a single pass`() { + // A run carrying bold + color + size, centered, on a bullet line. applyBlock inserts "• " and + // lands the selection back on the styled text, so every scan sees the same run the user is on. + var value = field("hello", TextRange(0, 5)) + value = applyStyle(value, RichStyle.Bold, linkColor) + value = applyStyle(value, RichStyle.FontColor(0xFF112233.toInt()), linkColor) + value = applyStyle(value, RichStyle.FontSize(18), linkColor) + value = applyAlignment(value, RichAlign.CENTER, linkColor, noFont) + value = applyBlock(value, BlockMarker.BULLET, linkColor, noFont) + + val state = toolbarStateOf(value) + + assertTrue(state.bold) + assertFalse(state.italic) + assertFalse(state.underline) + assertFalse(state.strikethrough) + assertEquals(0xFF112233.toInt(), state.fontColorArgb) + assertEquals(null, state.highlightArgb) + assertEquals(18, state.fontSizePt) + assertEquals(null, state.fontCss) + assertTrue(state.bullet) + assertFalse(state.ordered) + assertFalse(state.quote) + assertEquals(RichAlign.CENTER, state.alignment) + // Each field equals reading the same predicate directly off the value: the memoization the + // toolbar wraps this in (remember(value)) is behaviour-preserving, just computed once (#308). + assertEquals(isBoldActive(value), state.bold) + assertEquals(isBulletActive(value), state.bullet) + } + /** Mirrors exactly what FormattingToolbar reads to decide a style button's active/inactive tint. */ private fun isBoldActive(value: TextFieldValue): Boolean = RichTextEditing.isStyled( value.annotatedString.toRichContent(), diff --git a/app/src/test/kotlin/org/libremail/ui/lock/AppLockGateHostJvmTest.kt b/app/src/test/kotlin/org/libremail/ui/lock/AppLockGateHostJvmTest.kt index 3072a57..c27d0ff 100644 --- a/app/src/test/kotlin/org/libremail/ui/lock/AppLockGateHostJvmTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/lock/AppLockGateHostJvmTest.kt @@ -4,6 +4,7 @@ package org.libremail.ui.lock import android.content.Context import androidx.compose.material3.Text import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.DisposableEffect import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.junit4.v2.createComposeRule import androidx.compose.ui.test.onNodeWithText @@ -24,6 +25,7 @@ import org.robolectric.RobolectricTestRunner import org.robolectric.RuntimeEnvironment import org.robolectric.annotation.Config import org.robolectric.annotation.GraphicsMode +import kotlin.test.assertEquals /** * Robolectric JVM Compose test (#384, umbrella #373) for [AppLockGateHost], the gate that wraps the @@ -109,26 +111,50 @@ class AppLockGateHostJvmTest { } @Test - fun reLockAfterUnlock_drawsLockScreenButKeepsContentComposed() { + fun reLockAfterUnlock_keepsContentComposedButGatesItOutOfTheAccessibilityTree() { val state = MutableStateFlow(AppLockUiState.Unlocked) val vm = mockk(relaxed = true) every { vm.uiState } returns state + // Track the content's composition lifecycle directly: entered once and never disposed proves the + // content composable is RETAINED across a re-lock (its nav/scroll/draft state survives) rather + // than torn down — independently of whether its semantics are currently in the tree. + var entered = 0 + var disposed = 0 composeTestRule.setContent { CompositionLocalProvider(LocalLifecycleOwner provides resumedOwner) { LibreMailTheme(darkTheme = false, dynamicColor = false) { - AppLockGateHost(viewModel = vm) { Text(CONTENT) } + AppLockGateHost(viewModel = vm) { + DisposableEffect(Unit) { + entered++ + onDispose { disposed++ } + } + Text(CONTENT) + } } } } composeTestRule.onNodeWithText(CONTENT).assertIsDisplayed() + assertEquals(1, entered) - // Re-lock: once ever unlocked, the content stays composed (its nav/scroll/draft state survives) - // and the lock screen is drawn OVER it rather than replacing it. + // Re-lock: the lock screen is drawn OVER the content. The content stays composed (entered==1, + // never disposed), but #308 clears it out of the semantics/accessibility tree so TalkBack can't + // traverse the occluded mailbox nodes behind the opaque cover — assertDoesNotExist confirms the + // content's text is no longer reachable in the (a11y-facing) semantics tree. state.value = AppLockUiState.Locked() composeTestRule.waitForIdle() composeTestRule.onNodeWithText(string(R.string.app_lock_title)).assertIsDisplayed() - composeTestRule.onNodeWithText(CONTENT).assertExists() + composeTestRule.onNodeWithText(CONTENT).assertDoesNotExist() + assertEquals(1, entered) + assertEquals(0, disposed) + + // Unlocking again restores the content's semantics (same retained composition — still one entry). + state.value = AppLockUiState.Unlocked + composeTestRule.waitForIdle() + + composeTestRule.onNodeWithText(CONTENT).assertIsDisplayed() + assertEquals(1, entered) + assertEquals(0, disposed) } private companion object { diff --git a/app/src/test/kotlin/org/libremail/ui/onboarding/OnboardingViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/onboarding/OnboardingViewModelTest.kt index 2228d51..ac9cf95 100644 --- a/app/src/test/kotlin/org/libremail/ui/onboarding/OnboardingViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/onboarding/OnboardingViewModelTest.kt @@ -1,12 +1,16 @@ // SPDX-License-Identifier: GPL-3.0-or-later package org.libremail.ui.onboarding +import android.util.Log +import androidx.lifecycle.SavedStateHandle import io.mockk.Runs import io.mockk.coEvery import io.mockk.coVerify import io.mockk.every import io.mockk.just import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkAll import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.UnconfinedTestDispatcher @@ -30,10 +34,19 @@ class OnboardingViewModelTest { private val testDispatcher = UnconfinedTestDispatcher() @Before - fun setUp() = Dispatchers.setMain(testDispatcher) + fun setUp() { + Dispatchers.setMain(testDispatcher) + // onAccountAdded now breadcrumbs the first account via AppLog (#308); android.util.Log is a + // no-op stub that throws "not mocked" under plain JVM tests, so stub it so the call can't crash. + mockkStatic(Log::class) + every { Log.i(any(), any()) } returns 0 + } @After - fun tearDown() = Dispatchers.resetMain() + fun tearDown() { + Dispatchers.resetMain() + unmockkAll() + } private fun batteryManager(supported: Boolean = true, unrestricted: Boolean = false) = mockk { @@ -55,7 +68,8 @@ class OnboardingViewModelTest { battery: BatteryOptimizationManager = batteryManager(), contacts: ContactsPermissionManager = contactsManager(), settings: SettingsRepository = settingsRepository(), - ) = OnboardingViewModel(battery, contacts, settings) + savedStateHandle: SavedStateHandle = SavedStateHandle(), + ) = OnboardingViewModel(battery, contacts, settings, savedStateHandle) @Test fun `battery prompt is needed when not unrestricted and not handled`() = runTest(testDispatcher) { @@ -123,6 +137,21 @@ class OnboardingViewModelTest { assertEquals("imap:first@example.com", vm.firstAddedAccountId) } + @Test + fun `the first added account id survives a process-death recreation via SavedStateHandle`() = + runTest(testDispatcher) { + // #308: onboarding can be process-killed mid-flow (e.g. the excursion to system battery + // settings). The first-account id lives in SavedStateHandle, so a recreated ViewModel over + // the restored state must still finish onto that account's inbox rather than the unified one. + val handle = SavedStateHandle() + viewModel(savedStateHandle = handle).onAccountAdded("imap:first@example.com") + + // Simulate restoration after a kill: a brand-new handle seeded from the saved state, then a + // fresh ViewModel over it (a plain field would have been lost — this is exactly #30's guard). + val restored = SavedStateHandle(handle.keys().associateWith { handle.get(it) }) + assertEquals("imap:first@example.com", viewModel(savedStateHandle = restored).firstAddedAccountId) + } + @Test fun `marking the battery prompt handled persists the flag`() = runTest(testDispatcher) { val repo = settingsRepository() diff --git a/app/src/test/kotlin/org/libremail/ui/reader/ReaderViewModelActionsTest.kt b/app/src/test/kotlin/org/libremail/ui/reader/ReaderViewModelActionsTest.kt index c64bc30..503a825 100644 --- a/app/src/test/kotlin/org/libremail/ui/reader/ReaderViewModelActionsTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/reader/ReaderViewModelActionsTest.kt @@ -55,6 +55,9 @@ class ReaderViewModelActionsTest { every { Log.d(any(), any()) } returns 0 every { Log.i(any(), any()) } returns 0 every { Log.w(any(), any()) } returns 0 + // toggleStar's rollback path logs via AppLog.w(tag, msg, throwable) -> Log.w(String, String, + // Throwable); stub that overload too so the failing-write test's coroutine doesn't crash on it. + every { Log.w(any(), any(), any()) } returns 0 every { Log.e(any(), any(), any()) } returns 0 } @@ -78,6 +81,7 @@ class ReaderViewModelActionsTest { coEvery { repo.openMessage(messageId) } returns Result.success(message) every { repo.observeAttachments(messageId) } returns flowOf(listOf(attachment(0))) coEvery { repo.downloadedAttachmentParts(messageId) } returns emptySet() + coEvery { repo.setStarred(any(), any()) } returns Result.success(Unit) return repo } @@ -181,6 +185,28 @@ class ReaderViewModelActionsTest { coVerify { repo.setStarred(messageId, true) } } + @Test + fun `toggleStar rolls the optimistic star back and emits StarFailed when the write fails`() = runTest(dispatcher) { + val repo = loadedRepo() + coEvery { repo.setStarred(messageId, true) } returns Result.failure(RuntimeException("db locked")) + val vm = viewModel(repo) + advanceUntilIdle() + assertFalse(vm.state.value.message!!.isStarred) + + vm.events.test { + vm.toggleStar() + // The optimistic flip lands immediately, before the (failing) persist runs. + assertTrue(vm.state.value.message!!.isStarred) + advanceUntilIdle() + + assertEquals(ReaderEvent.StarFailed, awaitItem()) + // The failed write is reconciled: the star is rolled back to its pre-toggle value so the + // UI never stays stuck in a state the store rejected. + assertFalse(vm.state.value.message!!.isStarred) + cancelAndIgnoreRemainingEvents() + } + } + @Test fun `toggleStar does nothing before the message has loaded`() = runTest(dispatcher) { val repo = mockk(relaxed = true) diff --git a/app/src/test/kotlin/org/libremail/ui/settings/SettingsViewModelTest.kt b/app/src/test/kotlin/org/libremail/ui/settings/SettingsViewModelTest.kt index 00e50c6..cec7e02 100644 --- a/app/src/test/kotlin/org/libremail/ui/settings/SettingsViewModelTest.kt +++ b/app/src/test/kotlin/org/libremail/ui/settings/SettingsViewModelTest.kt @@ -9,6 +9,7 @@ import io.mockk.mockk import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.UnconfinedTestDispatcher import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.resetMain @@ -107,6 +108,26 @@ class SettingsViewModelTest { coVerify(exactly = 0) { settingsRepository.setAppLock(false) } } + @Test + fun `the disable-path reseal is dispatched off the main thread, not run inline on main`() = runTest(dispatcher) { + val databaseKeyStore = mockk(relaxed = true) + coEvery { databaseKeyStore.hasAuthSealedPassphrase() } returns true + val settingsRepository = mockk(relaxed = true) + val vm = viewModel(databaseKeyStore = databaseKeyStore, settingsRepository = settingsRepository) + // Swap the pinned dispatcher for a queue-and-drain one (sharing the scheduler) so we can see + // that the blocking Keystore reseal is DISPATCHED off-main rather than run inline on Main's + // unconfined pass — the point of #308: no Keystore crypto on the main dispatcher. + vm.defaultDispatcher = StandardTestDispatcher(testScheduler) + + vm.setAppLock(false) + + // Dispatched to defaultDispatcher, so it has not run on the current (unconfined Main) pass... + coVerify(exactly = 0) { databaseKeyStore.sealWithMaster() } + advanceUntilIdle() + // ...only once that off-main dispatcher is drained. + coVerify(exactly = 1) { databaseKeyStore.sealWithMaster() } + } + @Test fun `disabling app-lock with no auth seal just drops the gate`() = runTest(dispatcher) { val databaseKeyStore = mockk(relaxed = true) @@ -139,6 +160,10 @@ class SettingsViewModelTest { batteryOptimizationManager = mockk(relaxed = true), contactsPermissionManager = mockk(relaxed = true), syncScheduler = mockk(relaxed = true), - ) + ).apply { + // Pin the off-main reseal dispatcher (#308) to the test scheduler so the disable path's + // withContext(defaultDispatcher) work is advanced deterministically by advanceUntilIdle(). + defaultDispatcher = dispatcher + } } } diff --git a/config/detekt/detekt.yml b/config/detekt/detekt.yml index f190f3c..2cbbe1f 100644 --- a/config/detekt/detekt.yml +++ b/config/detekt/detekt.yml @@ -79,6 +79,9 @@ style: - '**/data/repository/AccountRepositoryImplTest.kt' - '**/ui/reader/ReaderViewModelTest.kt' - '**/ui/reader/ReaderViewModelActionsTest.kt' + # First-account onboarding breadcrumb (issue #308): onAccountAdded logs via AppLog, so this + # suite mockkStatic(Log) so the call doesn't crash on the throwing JVM stub. + - '**/ui/onboarding/OnboardingViewModelTest.kt' MagicNumber: # dp / sp / duration literals are idiomatic inline in Compose. ignoreAnnotated: ['Composable']