fix(notifications): open the tapped message from a new-mail notification
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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")))
|
||||
}
|
||||
}
|
||||
@@ -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<ComposePrefill?>(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<String?>(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 }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user