From a6ec00d2030a2c0b3664a7b0aecba936fb956869 Mon Sep 17 00:00:00 2001 From: Jason Ross Date: Wed, 1 Jul 2026 15:51:16 -0500 Subject: [PATCH] fix(notifications): open the tapped message from a new-mail notification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tapping a new-mail notification only brought the app to the foreground: the content PendingIntent was a bare launch intent shared by every notification, and nothing on the activity side handled a message target. Per-message notifications now carry an explicit open-message intent — action + id extra + a per-message data URI, so each message keeps its own PendingIntent under filterEquals instead of all collapsing onto one FLAG_UPDATE_CURRENT entry. MainActivity parses the id on fresh launch and in onNewIntent and hands it to the NavHost as pending state (the pendingCompose handoff pattern) to navigate to the reader. The group summary keeps the plain open-the-app intent. Fixes #56 Co-Authored-By: Claude Fable 5 --- .../notifications/NotificationIntentsTest.kt | 51 +++++++++++++++++++ .../main/kotlin/org/libremail/MainActivity.kt | 15 +++++- .../libremail/notifications/MailNotifier.kt | 16 ++++-- .../notifications/NotificationIntents.kt | 33 ++++++++++++ .../kotlin/org/libremail/ui/LibreMailApp.kt | 11 ++++ 5 files changed, 120 insertions(+), 6 deletions(-) create mode 100644 app/src/androidTest/kotlin/org/libremail/notifications/NotificationIntentsTest.kt create mode 100644 app/src/main/kotlin/org/libremail/notifications/NotificationIntents.kt diff --git a/app/src/androidTest/kotlin/org/libremail/notifications/NotificationIntentsTest.kt b/app/src/androidTest/kotlin/org/libremail/notifications/NotificationIntentsTest.kt new file mode 100644 index 0000000..8a546ff --- /dev/null +++ b/app/src/androidTest/kotlin/org/libremail/notifications/NotificationIntentsTest.kt @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.notifications + +import android.content.Intent +import android.net.Uri +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith + +/** + * Locks in the notification deep-link contract: a message id round-trips build → parse, and intents + * for different messages are distinct under [Intent.filterEquals] — the identity PendingIntent keys + * on — so per-message notifications never collapse onto one shared PendingIntent. + */ +@RunWith(AndroidJUnit4::class) +class NotificationIntentsTest { + + private val context = InstrumentationRegistry.getInstrumentation().targetContext + + @Test + fun message_id_round_trips_through_the_intent() { + val id = "imap:user@example.com:INBOX:42" + assertEquals(id, NotificationIntents.messageId(NotificationIntents.openMessage(context, id))) + } + + @Test + fun uri_hostile_ids_round_trip() { + val id = "imap:user@example.com:[Gmail]/All Mail:7?&%#" + assertEquals(id, NotificationIntents.messageId(NotificationIntents.openMessage(context, id))) + } + + @Test + fun other_intents_carry_no_message_id() { + assertNull(NotificationIntents.messageId(null)) + assertNull(NotificationIntents.messageId(Intent(Intent.ACTION_MAIN))) + assertNull(NotificationIntents.messageId(Intent(Intent.ACTION_VIEW, Uri.parse("mailto:a@b.c")))) + } + + @Test + fun intents_for_different_messages_are_distinct_pending_intent_keys() { + val first = NotificationIntents.openMessage(context, "imap:a@b:INBOX:1") + val second = NotificationIntents.openMessage(context, "imap:a@b:INBOX:2") + assertFalse(first.filterEquals(second)) + assertTrue(first.filterEquals(NotificationIntents.openMessage(context, "imap:a@b:INBOX:1"))) + } +} diff --git a/app/src/main/kotlin/org/libremail/MainActivity.kt b/app/src/main/kotlin/org/libremail/MainActivity.kt index 0b1caa9..35a1514 100644 --- a/app/src/main/kotlin/org/libremail/MainActivity.kt +++ b/app/src/main/kotlin/org/libremail/MainActivity.kt @@ -20,6 +20,7 @@ import androidx.core.content.ContextCompat import androidx.lifecycle.compose.collectAsStateWithLifecycle import dagger.hilt.android.AndroidEntryPoint import org.libremail.data.settings.SettingsRepository +import org.libremail.notifications.NotificationIntents import org.libremail.ui.LibreMailApp import org.libremail.ui.compose.ComposePrefill import org.libremail.ui.compose.IntentComposeParser @@ -38,6 +39,12 @@ class MainActivity : ComponentActivity() { */ private val pendingCompose = mutableStateOf(null) + /** + * The message a tapped new-mail notification asks to open, consumed once by the NavHost. Compose + * state for the same reason as [pendingCompose]. + */ + private val pendingOpenMessageId = mutableStateOf(null) + override fun onStart() { super.onStart() // Foreground: recover IDLE push if a background start was previously blocked. @@ -47,10 +54,11 @@ class MainActivity : ComponentActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) enableEdgeToEdge() - // Only on a fresh launch — on a config-change recreation the NavHost restores the compose - // destination itself, so re-parsing the (unchanged) intent would open a duplicate. + // 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) } setContent { val dynamicColor by settingsRepository.dynamicColor.collectAsStateWithLifecycle(initialValue = true) @@ -59,6 +67,8 @@ class MainActivity : ComponentActivity() { LibreMailApp( pendingCompose = pendingCompose.value, onComposeHandled = { pendingCompose.value = null }, + pendingOpenMessageId = pendingOpenMessageId.value, + onOpenMessageHandled = { pendingOpenMessageId.value = null }, ) } } @@ -68,6 +78,7 @@ class MainActivity : ComponentActivity() { super.onNewIntent(intent) setIntent(intent) IntentComposeParser.parse(intent)?.let { pendingCompose.value = it } + NotificationIntents.messageId(intent)?.let { pendingOpenMessageId.value = it } } } diff --git a/app/src/main/kotlin/org/libremail/notifications/MailNotifier.kt b/app/src/main/kotlin/org/libremail/notifications/MailNotifier.kt index 0278886..529b6aa 100644 --- a/app/src/main/kotlin/org/libremail/notifications/MailNotifier.kt +++ b/app/src/main/kotlin/org/libremail/notifications/MailNotifier.kt @@ -35,7 +35,6 @@ class MailNotifier @Inject constructor(@ApplicationContext private val context: if (messages.isEmpty() || !hasPermission()) return ensureAccountChannel(account) val manager = NotificationManagerCompat.from(context) - val contentIntent = contentIntent() val channelId = channelId(account.id) val groupKey = groupKey(account.id) val summaryId = summaryId(account.id) @@ -52,7 +51,7 @@ class MailNotifier @Inject constructor(@ApplicationContext private val context: .setAutoCancel(true) .setOnlyAlertOnce(true) .setGroup(groupKey) - .setContentIntent(contentIntent) + .setContentIntent(openMessageIntent(message.id)) .build() manager.notify(notificationId(message.id, summaryId), notification) } @@ -72,7 +71,7 @@ class MailNotifier @Inject constructor(@ApplicationContext private val context: .setOnlyAlertOnce(true) .setGroup(groupKey) .setGroupSummary(true) - .setContentIntent(contentIntent) + .setContentIntent(openAppIntent()) .build() manager.notify(summaryId, summary) } @@ -105,7 +104,16 @@ class MailNotifier @Inject constructor(@ApplicationContext private val context: manager.deleteNotificationChannelGroup(accountId) } - private fun contentIntent(): PendingIntent { + /** Opens the tapped message's reader (each message's intent is distinct — see [NotificationIntents]). */ + private fun openMessageIntent(messageId: String): PendingIntent = PendingIntent.getActivity( + context, + 0, + NotificationIntents.openMessage(context, messageId), + PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT, + ) + + /** Just brings the app to the foreground — used by the group summary, which has no single message. */ + private fun openAppIntent(): PendingIntent { val intent = Intent(context, MainActivity::class.java).apply { flags = Intent.FLAG_ACTIVITY_SINGLE_TOP or Intent.FLAG_ACTIVITY_CLEAR_TOP } diff --git a/app/src/main/kotlin/org/libremail/notifications/NotificationIntents.kt b/app/src/main/kotlin/org/libremail/notifications/NotificationIntents.kt new file mode 100644 index 0000000..a27a40b --- /dev/null +++ b/app/src/main/kotlin/org/libremail/notifications/NotificationIntents.kt @@ -0,0 +1,33 @@ +// SPDX-License-Identifier: GPL-3.0-or-later +package org.libremail.notifications + +import android.content.Context +import android.content.Intent +import android.net.Uri +import org.libremail.MainActivity + +/** + * Builds and parses the intent behind a tapped per-message new-mail notification, keeping both sides + * of the contract ([MailNotifier] builds, MainActivity parses) in one place. + * + * The per-message `data` URI is load-bearing: PendingIntent identity ignores extras, so without a + * distinct URI every message's notification would collapse onto one FLAG_UPDATE_CURRENT PendingIntent + * and always open the most-recently-notified message. The intent is explicit (component set), so the + * private scheme needs no manifest intent-filter and adds no exported surface. + */ +object NotificationIntents { + + private const val ACTION_OPEN_MESSAGE = "org.libremail.action.OPEN_MESSAGE" + private const val EXTRA_MESSAGE_ID = "org.libremail.extra.MESSAGE_ID" + + fun openMessage(context: Context, messageId: String): Intent = Intent(context, MainActivity::class.java).apply { + action = ACTION_OPEN_MESSAGE + data = Uri.parse("libremail://message/${Uri.encode(messageId)}") + putExtra(EXTRA_MESSAGE_ID, messageId) + flags = Intent.FLAG_ACTIVITY_SINGLE_TOP or Intent.FLAG_ACTIVITY_CLEAR_TOP + } + + /** The tapped message's id, or null for any other intent (launcher, mailto:, share, …). */ + fun messageId(intent: Intent?): String? = + intent?.takeIf { it.action == ACTION_OPEN_MESSAGE }?.getStringExtra(EXTRA_MESSAGE_ID) +} diff --git a/app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt b/app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt index 6d44535..0e7882c 100644 --- a/app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt +++ b/app/src/main/kotlin/org/libremail/ui/LibreMailApp.kt @@ -55,6 +55,8 @@ fun LibreMailApp( startupViewModel: StartupReportViewModel = hiltViewModel(), pendingCompose: ComposePrefill? = null, onComposeHandled: () -> Unit = {}, + pendingOpenMessageId: String? = null, + onOpenMessageHandled: () -> Unit = {}, ) { val startDestination by appViewModel.startDestination.collectAsStateWithLifecycle() // Hold (render nothing) until the account count is known, so a cold start never flashes the @@ -79,6 +81,15 @@ fun LibreMailApp( onComposeHandled() } + // A tapped new-mail notification opens that message's reader on top of the current stack, so back + // lands where the user was (the mailbox on a cold start). If the account vanished in the meantime + // (start = onboarding) the request is consumed without navigating. + LaunchedEffect(pendingOpenMessageId) { + val messageId = pendingOpenMessageId ?: return@LaunchedEffect + if (start != Routes.ONBOARDING) navController.navigate(Routes.reader(messageId)) + onOpenMessageHandled() + } + NavHost( navController = navController, startDestination = start,