diff --git a/app/src/androidTest/kotlin/org/libremail/IntentHandledMarkerTest.kt b/app/src/androidTest/kotlin/org/libremail/IntentHandledMarkerTest.kt new file mode 100644 index 0000000..d109e70 --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/IntentHandledMarkerTest.kt @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail + +import android.content.Intent +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Locks in the #157 regression fix: [IntentHandledMarker] must tell a redelivered (already-parsed) + * intent apart from a genuinely new one using only the [Intent] instance itself, never + * `savedInstanceState` — which Android also sets to non-null after a process-death relaunch, where the + * delivered intent is a brand new, not-yet-handled notification tap or mailto/share, not a replay. + */ +@RunWith(AndroidJUnit4::class) +class IntentHandledMarkerTest { + + @Test + fun first_look_marks_the_intent_and_reports_it_as_unhandled() { + val intent = Intent(Intent.ACTION_MAIN) + assertTrue(IntentHandledMarker.markIfUnhandled(intent)) + } + + @Test + fun redelivering_the_same_instance_is_recognized_as_already_handled() { + // Simulates a config-change recreation: Android redelivers the very same Intent object to the + // new Activity instance's onCreate, so a second look must be recognized as a replay. + val intent = Intent(Intent.ACTION_MAIN) + IntentHandledMarker.markIfUnhandled(intent) + assertFalse(IntentHandledMarker.markIfUnhandled(intent)) + } + + @Test + fun a_freshly_constructed_equivalent_intent_is_still_unhandled() { + // Simulates a process-death relaunch (e.g. tapping a notification after the app was killed in + // the background): a brand new Intent instance arrives — content may equal one already marked + // in a prior (now-dead) process, but it was never marked in THIS one, so it must be parsed. + val first = Intent(Intent.ACTION_VIEW) + IntentHandledMarker.markIfUnhandled(first) + + val second = Intent(Intent.ACTION_VIEW) + assertTrue(IntentHandledMarker.markIfUnhandled(second)) + } +} diff --git a/app/src/main/kotlin/org/libremail/MainActivity.kt b/app/src/main/kotlin/org/libremail/MainActivity.kt index e480786..fc65324 100644 --- a/app/src/main/kotlin/org/libremail/MainActivity.kt +++ b/app/src/main/kotlin/org/libremail/MainActivity.kt @@ -66,12 +66,7 @@ class MainActivity : FragmentActivity() { } } } - // Only on a fresh launch — on a config-change recreation the NavHost restores the compose / - // reader destination itself, so re-parsing the (unchanged) intent would open a duplicate. - if (savedInstanceState == null) { - pendingCompose.value = IntentComposeParser.parse(intent) - pendingOpenMessageId.value = NotificationIntents.messageId(intent) - } + handleIntent(intent) setContent { val dynamicColor by settingsRepository.dynamicColor.collectAsStateWithLifecycle(initialValue = true) LibreMailTheme(dynamicColor = dynamicColor) { @@ -92,7 +87,54 @@ class MainActivity : FragmentActivity() { override fun onNewIntent(intent: Intent) { super.onNewIntent(intent) setIntent(intent) + handleIntent(intent) + } + + /** + * Parses [intent] for a pending compose ([IntentComposeParser]) or open-message + * ([NotificationIntents]) request — unless [IntentHandledMarker] says this exact intent instance + * was already parsed. + * + * This used to be gated on `savedInstanceState == null` in [onCreate]: a non-null value was + * assumed to mean "config-change recreation" — where Android redelivers the very same, + * already-parsed intent and the NavHost restores its own compose/reader destination itself, so + * re-parsing would only navigate to a duplicate. But Android *also* passes a restored, non-null + * savedInstanceState when it recreates this activity after the process was killed in the + * background and is then relaunched — e.g. by tapping a notification. There, `intent` is the new + * tap, not a replay, but the old guard swallowed it exactly like a rotation: `pendingOpenMessageId` + * was never set, so the tap silently landed wherever the restored back stack was, never the + * message. That regression is #157. + * + * Marking the [Intent] instance itself — rather than branching on savedInstanceState, which can't + * tell a config change and a process-death relaunch apart — is correct for both: a config-change + * recreation redelivers the very same intent this activity already marked, so it's recognized and + * skipped; a genuinely new intent — a fresh notification tap or mailto/share, whether delivered + * warm via [onNewIntent] or cold via [onCreate] after a process-death relaunch — is never marked + * yet, so it's always (re)parsed. + */ + private fun handleIntent(intent: Intent) { + if (!IntentHandledMarker.markIfUnhandled(intent)) return IntentComposeParser.parse(intent)?.let { pendingCompose.value = it } NotificationIntents.messageId(intent)?.let { pendingOpenMessageId.value = it } } } + +/** + * Marks an [Intent] as already parsed by [MainActivity.handleIntent], so a redelivery of the very same + * instance — which is what Android does when it recreates an Activity for a configuration change — is + * recognized and skipped instead of re-triggering a duplicate navigation. A freshly constructed intent + * (a new notification tap or mailto/share, however it arrives) is never marked yet, so it is always + * treated as unhandled — including right after a process-death relaunch, where `savedInstanceState` is + * restored (non-null) but the intent itself is new. Internal (not private) so androidTest can verify + * the marking contract directly. See #157. + */ +internal object IntentHandledMarker { + private const val EXTRA_HANDLED = "org.libremail.extra.INTENT_HANDLED" + + /** Marks [intent] handled and returns `true` — but only the first time this instance is seen. */ + fun markIfUnhandled(intent: Intent): Boolean { + if (intent.getBooleanExtra(EXTRA_HANDLED, false)) return false + intent.putExtra(EXTRA_HANDLED, true) + return true + } +}