From 69be8a5c69ca7263b527336d06bf6f7ccf4420e0 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Thu, 2 Jul 2026 18:42:49 -0500 Subject: [PATCH] fix(notifications): reliably open the tapped message from a new-mail notification MainActivity.onCreate only parsed the incoming intent (pendingCompose / pendingOpenMessageId) when savedInstanceState == null, on the assumption that a non-null value always means a config-change recreation (e.g. rotation), where Android redelivers the same, already-handled intent and re-parsing would just navigate to a duplicate destination. But Android also passes a restored, non-null savedInstanceState when it recreates the activity after the process was killed in the background and is then relaunched by tapping a notification. There, intent is the new tap, not a replay, but the guard swallowed it exactly like a rotation, so pendingOpenMessageId was never set and the tap silently landed wherever the restored back stack was (typically the mailbox) instead of the message. That's the #157 regression from the original fix in a6ec00d (#56). pendingCompose (mailto:/share intents) went through the identical guard and had the same latent bug. Replace the savedInstanceState check with IntentHandledMarker, which marks the Intent instance itself once parsed. A config-change recreation redelivers that same marked instance, so it's correctly skipped; a genuinely new intent -- warm via onNewIntent or cold via onCreate after a process-death relaunch -- is never marked yet, so it's always parsed. This dedupes on the intent's own identity instead of an unreliable proxy for it, so it can't confuse the two recreation paths. Adds IntentHandledMarkerTest covering the marking contract: unhandled on first look, recognized as handled on a redelivered instance, and still unhandled on a freshly constructed (but content-equal) instance -- the process-death case. Closes #157 Co-Authored-By: Claude Opus 4.8 --- .../org/libremail/IntentHandledMarkerTest.kt | 46 ++++++++++++++++ .../main/kotlin/org/libremail/MainActivity.kt | 54 ++++++++++++++++--- 2 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 app/src/androidTest/kotlin/org/libremail/IntentHandledMarkerTest.kt 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 + } +} -- 2.47.3