Conversation
56dcd8e to
90a4fa3
Compare
| } | ||
|
|
||
| // Use the same notification ID calculation as NotificationWorker | ||
| notificationManager.notify(notificationId, notification) |
Check failure
Code scanning / CodeQL
Use of implicit PendingIntents
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
General approach: Ensure all PendingIntents used with notifications or bubbles are (a) based on explicit Intents and (b) created with FLAG_IMMUTABLE where mutation is not required. In particular, avoid FLAG_MUTABLE unless there is a concrete need.
Best fix here: The contentIntent used in the notification is already explicit and immutable, so it is fine. The only remaining mutable PendingIntent is bubbleIntent:
val bubbleIntent = android.app.PendingIntent.getActivity(
context,
bubbleRequestCode,
BubbleActivity.newIntent(context, bubbleInfo.roomToken, data.conversationName),
android.app.PendingIntent.FLAG_UPDATE_CURRENT or android.app.PendingIntent.FLAG_MUTABLE
)The BubbleActivity.newIntent(...) factory is expected to return an explicit Intent targeting BubbleActivity, and there is no indication that any caller needs to modify this PendingIntent. We can therefore safely switch this to FLAG_IMMUTABLE. This keeps existing behavior (the Intent content is still the same and FLAG_UPDATE_CURRENT is preserved) while eliminating mutability so the analyzer’s concern is addressed.
Concretely:
- In
NotificationUtils.kt, in thecreateBubbleNotificationfunction around lines 692–701, change the flags used when creatingbubbleIntentfromFLAG_UPDATE_CURRENT or FLAG_MUTABLEtoFLAG_UPDATE_CURRENT or FLAG_IMMUTABLE. - No other code or imports need to be changed; we only adjust the flags.
| @@ -697,7 +697,7 @@ | ||
| context, | ||
| bubbleRequestCode, | ||
| BubbleActivity.newIntent(context, bubbleInfo.roomToken, data.conversationName), | ||
| android.app.PendingIntent.FLAG_UPDATE_CURRENT or android.app.PendingIntent.FLAG_MUTABLE | ||
| android.app.PendingIntent.FLAG_UPDATE_CURRENT or android.app.PendingIntent.FLAG_IMMUTABLE | ||
| ) | ||
|
|
||
| val contentIntent = android.app.PendingIntent.getActivity( |
There was a problem hiding this comment.
Flag mutable is required for bubbles to work, and shouldn't be a security risk, as the content intent flags are set to IMMUTABLE
This comment was marked as resolved.
This comment was marked as resolved.
a90d336 to
315ee3b
Compare
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/5928.apk |
|
i wanted to review but #5979 happens to me again |
|
giving it another try. @rapterjet2004 does it work for you as expected? |
2826d6a to
933a46b
Compare
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/23905922700/artifacts/6243947187 |
This comment was marked as outdated.
This comment was marked as outdated.
6ed3553 to
87c5130
Compare
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/23946962562/artifacts/6259706638 |
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/23948011150/artifacts/6260084009 |
1a3fb2f to
d174829
Compare
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/24193631626/artifacts/6351236070 |
- Refactoring createConversationBubble to use best practices, keeping ChatActivity.kt simple - Refactoring NotificationWorker functions related to bubbling to now properly follow the builder pattern - better error handling of edge cases Signed-off-by: rapterjet2004 <juliuslinus1@gmail.com>
c185810 to
1fd84eb
Compare
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/24247358242/artifacts/6372834294 |
Signed-off-by: rapterjet2004 <juliuslinus1@gmail.com>
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/24249183788/artifacts/6373597825 |
There was a problem hiding this comment.
Pull request overview
This PR continues the conversation “bubble” feature implementation by adding user-facing settings (global + per-conversation) and wiring bubble creation/maintenance into notification handling and chat UI flows.
Changes:
- Adds bubble-related strings, menu items, and settings rows in Settings and Conversation Info screens.
- Introduces bubble preference persistence (global enable/force + per-conversation) and navigation helpers to focus relevant settings.
- Refactors/extends notification code to support bubble notifications, shortcuts, and bubble lifecycle management.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| app/src/main/res/values/strings.xml | Adds UI strings for bubble settings and actions. |
| app/src/main/res/menu/menu_conversation.xml | Adds menu actions to create a bubble / open in app. |
| app/src/main/res/layout/item_notification_settings.xml | Adds per-conversation bubble toggle row. |
| app/src/main/res/layout/activity_settings.xml | Adds global bubble enable + “force all” settings rows. |
| app/src/main/res/layout/activity_conversation_info.xml | Adds an ID for scrolling/highlighting bubble setting. |
| app/src/main/java/com/nextcloud/talk/utils/preferences/AppPreferencesImpl.kt | Stores/reads global bubble preferences. |
| app/src/main/java/com/nextcloud/talk/utils/preferences/AppPreferences.java | Exposes global bubble preferences in the interface. |
| app/src/main/java/com/nextcloud/talk/utils/NotificationUtils.kt | Adds bubble capability checks, bubble dismissal helpers, bubble icon loading, and bubble notification creation. |
| app/src/main/java/com/nextcloud/talk/utils/bundle/BundleKeys.kt | Adds intent extras for focusing bubble settings in UI. |
| app/src/main/java/com/nextcloud/talk/settings/SettingsActivity.kt | Implements bubble settings UI + navigation to system bubble settings. |
| app/src/main/java/com/nextcloud/talk/models/json/push/DecryptedPushMessage.kt | Changes multi-notification IDs container type. |
| app/src/main/java/com/nextcloud/talk/jobs/NotificationWorker.kt | Refactors chat notification building and adds bubble metadata handling. |
| app/src/main/java/com/nextcloud/talk/conversationinfo/ConversationInfoActivity.kt | Adds per-conversation bubble toggle behavior + highlight/scroll support. |
| app/src/main/java/com/nextcloud/talk/chat/ChatActivity.kt | Adds “Create bubble” menu action and flow to guide users to enable settings. |
| app/src/main/java/com/nextcloud/talk/chat/BubbleActivity.kt | Adds a bubble-hosted chat activity variant with “open in app”. |
| app/src/main/AndroidManifest.xml | Registers BubbleActivity with embedded/resizeable/document settings. |
| app/src/androidTest/java/com/nextcloud/talk/data/database/dao/ChatMessagesDaoTest.kt | Updates DAO test calls to match updated DAO signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private const val APP_NOTIFICATION = $$"com.android.settings.Settings$AppBubbleNotificationSettingsActivity" | ||
| private const val BUBBLE_NOTIFICATION = $$"com.android.settings.Settings$BubbleNotificationSettingsActivity" |
|
|
||
| menu.findItem(R.id.create_conversation_bubble)?.isVisible = NotificationUtils.deviceSupportsBubbles | ||
|
|
| val deviceSupportsBubbles = Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q | ||
|
|
||
| fun areSystemBubblesEnabled(context: Context): Boolean { | ||
| if (!deviceSupportsBubbles) { | ||
| return false | ||
| } | ||
|
|
||
| return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) { | ||
| Settings.Secure.getInt( | ||
| context.contentResolver, | ||
| "notification_bubbles", | ||
| 1 | ||
| ) == 1 | ||
| } else { | ||
| // Android 10 (Q) — bubbles always enabled | ||
| true | ||
| } |
| fun isSystemBubblePreferenceAll(context: Context): Boolean { | ||
| if (Build.VERSION.SDK_INT < Build.VERSION_CODES.S) { | ||
| return false | ||
| } | ||
|
|
||
| val notificationManager = context.getSystemService(NotificationManager::class.java) | ||
| return notificationManager?.bubblePreference == NotificationManager.BUBBLE_PREFERENCE_ALL | ||
| } |
| val channel = NotificationChannel(notificationChannel.id, notificationChannel.name, importance).apply { | ||
| description = notificationChannel.description | ||
| enableLights(true) | ||
| lightColor = R.color.colorPrimary |
| fun dismissBubblesWithoutExplicitSettings(context: Context?, conversationUser: User) { | ||
| dismissBubbles(context, conversationUser) { roomToken -> | ||
| !com.nextcloud.talk.utils.preferences.preferencestorage.DatabaseStorageModule( | ||
| conversationUser, | ||
| roomToken | ||
| ).getBoolean("bubble_switch", false) | ||
| } |
| } | ||
|
|
||
| fun isNotificationVisible(context: Context?, notificationId: Int): Boolean { | ||
| val notificationManager = context!!.getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager |
| .setShortLabel(fallbackConversationLabel) | ||
| .setLongLabel(fallbackConversationLabel) | ||
| .setIcon(bubbleIcon) | ||
| .setIntent(Intent(Intent.ACTION_DEFAULT)) |

Changes
NotificationUtils.createConversationBubbleto use kotlin flows and best practicesNotificationWorker.addBubblefor clarity and reducing nesting, using the return-if-error technique, and the proper usage of the builder pattern for all functions related to the bubble feature🏁 Checklist
/backport to stable-xx.x