From fe2c848f44d1a66e626c25e7ae628c623852b200 Mon Sep 17 00:00:00 2001 From: JingMatrix Date: Thu, 6 Aug 2026 00:23:57 +0200 Subject: [PATCH] Fix the scope targets no filter can reveal and the routes that lead nowhere The system server is offered as a synthetic row rather than an installed package, so no filter can lead a reader back to it, yet it was subject to the system-apps filter and a module whose declared scope is the framework showed an empty list. Exempt the framework alone, and only when the module asks for it. The empty-scope dialog's dismiss action closed it and returned the reader to the page they were trying to leave, where back raised it again; the only exit was to disable the module. Make that button leave, and offer the module's own recommendation as the primary action where it declares one. Legacy modules name the framework "android" and the android package "system"; modern modules and the daemon use the reverse. The store normalised the installed scope but not the catalogue's, so one module could name one target two ways in adjacent lines. Move the transform into one function both readings pass through. Module-update notifications are always enqueued for user 0 but carried the user whose PACKAGE_REPLACED raised them, so for a module installed in a private space as well as the main user the surviving notice pointed into a locked profile. Prefer user 0 when the module is installed there. --- .../daemon/system/NotificationManager.kt | 30 ++++++- .../manager/data/model/ModuleDetection.kt | 27 ++++-- .../manager/ui/screens/modules/ScopeScreen.kt | 82 ++++++++++++++++--- .../ui/screens/modules/ScopeViewModel.kt | 32 ++++++-- .../ui/screens/repo/RepoDetailsScreen.kt | 18 +++- .../ui/screens/repo/RepoDetailsViewModel.kt | 19 +++++ 6 files changed, 182 insertions(+), 26 deletions(-) diff --git a/daemon/src/main/kotlin/org/matrix/vector/daemon/system/NotificationManager.kt b/daemon/src/main/kotlin/org/matrix/vector/daemon/system/NotificationManager.kt index b9db34cee..602ab3883 100644 --- a/daemon/src/main/kotlin/org/matrix/vector/daemon/system/NotificationManager.kt +++ b/daemon/src/main/kotlin/org/matrix/vector/daemon/system/NotificationManager.kt @@ -517,13 +517,36 @@ object NotificationManager { modulePackageName, userName) + // Which user the link opens, which is not necessarily the one whose update raised this. + // + // Every notice this function posts is enqueued for user 0 — see the call at the end — so the + // reader tapping one is standing in user 0 whichever user's PACKAGE_REPLACED fired it. A link + // naming a secondary user therefore lands them somewhere they cannot see: a module installed + // in a private space as well as the main user raised two of these, the tag is the package + // alone so the second overwrote the first, and the survivor pointed into a profile that is + // locked more often than not. The scope editor then listed that profile's apps, which for a + // locked private space is nothing at all, and blamed a filter nobody had set. + // + // Preferring user 0 gives up nothing, because this id does not choose a configuration. A + // module is one package and one APK for the whole device with one scope set and one enabled + // flag; what the id selects is only which user's apps are offered as targets. So the same + // rule applies to the "not activated yet" half, where the act waiting to be done — turning + // the module on — is likewise device-wide. + // + // The triggering user is kept when the module is not in user 0 at all, which is a module + // that lives only in a secondary profile. There is no better answer for that one, and the + // notice is at least still about somewhere the module exists. + val linkUserId = + if (packageManager?.isPackageAvailable(modulePackageName, 0, true) == true) 0 + else moduleUserId + val intent = Intent(openManagerAction).apply { setPackage("android") data = Uri.Builder() .scheme("module") - .encodedAuthority("$modulePackageName:$moduleUserId") + .encodedAuthority("$modulePackageName:$linkUserId") .build() } val pi = @@ -554,6 +577,11 @@ object NotificationManager { // that a module installed for two users shows one notice rather than two — which is what it did // before this as well. The collision with the scope prompt is gone either way, now that a scope // tag carries its ":user:target" suffix. + // + // What that price used to include, and no longer does: the two raisings overwrite each other, + // so the surviving notice carried whichever user's link happened to be written last. That is + // what [linkUserId] above is for — the tag stays user-free and the destination stops depending + // on the order two broadcasts arrived in. val tag = if (enabled) modulePackageName else notActivatedTag(modulePackageName) runCatching { nm?.enqueueNotificationWithTag("android", opPkg, tag, tag.hashCode(), notif, 0) diff --git a/manager/src/main/kotlin/org/matrix/vector/manager/data/model/ModuleDetection.kt b/manager/src/main/kotlin/org/matrix/vector/manager/data/model/ModuleDetection.kt index d7af6d3c4..8974ef1c9 100644 --- a/manager/src/main/kotlin/org/matrix/vector/manager/data/model/ModuleDetection.kt +++ b/manager/src/main/kotlin/org/matrix/vector/manager/data/model/ModuleDetection.kt @@ -219,18 +219,33 @@ object ModuleDetection { .getOrNull() ?.filter { it.isNotEmpty() } ?: return emptyList() - // Legacy modules name the system server the other way round: their "android" is the - // daemon's "system", and their "system" is the ordinary "android" package. The convention - // is as old as XposedBridge and universal among legacy modules, so the swap is - // unconditional. - return raw.map { + return swapLegacyFrameworkNames(raw) + } + + /** + * A legacy module's declared scope, spelled the way everything else here spells it. + * + * Legacy modules name the system server the other way round: their "android" is the daemon's + * "system", and their "system" is the ordinary "android" package. XposedBridge reported + * `packageName` as "system" for the system dialogues so that a module testing for "android" + * found system_server alone, and the scope vocabulary grew up around that; LSPosed later made + * "system" the system server and left "android" as the real package, which is what every + * modern module and the whole of the daemon mean by the two words today. The convention is + * universal among legacy modules, so the swap is unconditional for them. + * + * Not private, and not applied at the point of reading alone: the store shows a module's + * declared scope from the catalogue rather than from the APK, and that list is written in the + * module's own vocabulary too — so it has to pass through here before it is put on screen + * beside a list that already has. + */ + fun swapLegacyFrameworkNames(scope: List): List = + scope.map { when (it) { "android" -> "system" "system" -> "android" else -> it } } - } private fun String?.toIntOrZero(): Int = this?.trim()?.takeWhile { it.isDigit() }?.toIntOrNull() ?: 0 diff --git a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ScopeScreen.kt b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ScopeScreen.kt index db0b68c29..10dfe7dfb 100644 --- a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ScopeScreen.kt +++ b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ScopeScreen.kt @@ -197,6 +197,13 @@ fun ScopeScreen( } val haptics = LocalHapticFeedback.current var confirmStranded by remember { mutableStateOf(false) } + // Whether the stranding question has already been put this visit, and answered by neither + // button. Two slots cannot hold three answers, so when the module has asked for something the + // buttons are "give it that" and "switch it off" and there is none that says "leave it exactly + // as it is" — cancelling the dialog is the only way to say that, and a warning that comes + // straight back on the next back press turns cancelling into a wall the reader cannot get + // past. Asked once, then believed. + var strandWarned by remember { mutableStateOf(false) } val frameworkRestartNeeded by viewModel.frameworkRestartNeeded.collectAsStateWithLifecycle() val staticScopeNotice = stringResource(R.string.scope_static) @@ -238,7 +245,8 @@ fun ScopeScreen( // Leaving a module enabled with nothing to hook does nothing at all but looks like it works. fun attemptBack() { - if (viewModel.wouldStrandModule()) confirmStranded = true else onNavigateBack() + if (!strandWarned && viewModel.wouldStrandModule()) confirmStranded = true + else onNavigateBack() } // The gesture leaves this screen exactly as the arrow does, so it asks the same question @@ -505,24 +513,76 @@ fun ScopeScreen( } if (confirmStranded) { + // Three things the reader might mean and two slots to say them in — `VectorAlertDialog` + // wraps Material's `AlertDialog`, which has a confirm button and a dismiss button and + // nothing else. Which two are offered depends on whether the module asked for anything, + // and the third is always reachable by cancelling the dialog. + // + // A module with a recommendation is the interesting case: the useful answer there is not + // "switch it off" but "give it what it asked for", which is what the pre-Compose manager + // offered as its positive button whenever a recommendation existed, keeping disable for + // the negative one. Offering to disable a module that has told us exactly which apps it + // wants is offering to throw away the answer while holding it. + val hasRecommended = !state.recommended.isEmpty + // Written once and dropped into whichever slot is free: the same act — turn the module off + // and leave — is the positive answer when there is nothing better to offer and the + // negative one when there is. + val disableAndLeave: @Composable () -> Unit = { + TextButton( + onClick = { + viewModel.setModuleEnabled(false) + confirmStranded = false + onNavigateBack() + } + ) { + Text(stringResource(R.string.scope_empty_disable)) + } + } VectorAlertDialog( - onDismissRequest = { confirmStranded = false }, + // Tapping outside, or the system back the dialog handles itself, is a cancel and not + // an answer — so it goes back to the list being edited rather than off the screen. + // It is also the only way to say "leave it exactly as it is" when the buttons are + // taken, which is why it records that the question has now been asked; see + // [strandWarned]. + onDismissRequest = { + strandWarned = true + confirmStranded = false + }, title = { Text(stringResource(R.string.scope_empty_title)) }, text = { Text(stringResource(R.string.scope_empty_message)) }, confirmButton = { - TextButton( - onClick = { - viewModel.setModuleEnabled(false) - confirmStranded = false - onNavigateBack() + if (hasRecommended) { + // Ticks the recommendation and returns the reader to the list, deliberately + // without leaving: this is an edit like every other on this screen and still + // has to be applied, and navigating away from it would drop the draft on the + // floor a moment after offering it. + TextButton( + onClick = { + viewModel.useRecommended() + confirmStranded = false + } + ) { + Text(stringResource(R.string.scope_use_recommended)) } - ) { - Text(stringResource(R.string.scope_empty_disable)) + } else { + disableAndLeave() } }, dismissButton = { - TextButton(onClick = { confirmStranded = false }) { - Text(stringResource(R.string.scope_empty_keep)) + if (hasRecommended) { + disableAndLeave() + } else { + // Leaves, which the label has always promised and the button never did: + // dismissing the dialog alone put the reader back on the page they were trying + // to leave, where pressing back asked them the same question again. + TextButton( + onClick = { + confirmStranded = false + onNavigateBack() + } + ) { + Text(stringResource(R.string.scope_empty_keep)) + } } }, ) diff --git a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ScopeViewModel.kt b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ScopeViewModel.kt index 7e9326cc4..23eef4c82 100644 --- a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ScopeViewModel.kt +++ b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/modules/ScopeViewModel.kt @@ -305,6 +305,7 @@ class ScopeViewModel( target in filters.draft || target in filters.saved || target in filters.touched + val requested = app.packageName in recommended if (locked || filters.recommendedOnly) { // Both answer one question — what does this module want, and what have // I given it — and the other filters have no say in either. Chrome is @@ -314,14 +315,31 @@ class ScopeViewModel( // which is most of them, showed an empty list for exactly that reason. // The screen greys the other three out in both cases, and this is the // code that makes that honest rather than decorative. - return@filter matchesQuery && (inPlay || app.packageName in recommended) + return@filter matchesQuery && (inPlay || requested) } - // The framework is a system target and is filtered like one. It needs no - // exemption of its own: once it is in the scope the line above puts it - // beyond every filter, and before it is chosen it is simply the most - // system of system apps, so someone who has asked not to see those has - // asked not to see it. - val matchesSys = inPlay || filters.showSystem || !app.isSystemApp + // The framework, when the module has asked for it, and nothing else. + // + // A request does not generally outrank the reader's filters: a module may + // name dozens of system packages, and exempting all of them would leave + // the system-apps switch turning nothing off on exactly the modules whose + // lists are longest. The reader has "What the module asks for" for that + // view, and it already overrides all three. + // + // The framework is the exception because it is not one of the several + // hundred rows the filters exist to thin out. It is not an installed + // package at all — it is a synthetic row this view model adds — so it + // cannot be found by turning any filter on and hunting for it, and a + // reader who has never seen it has no reason to think it exists. Hidden, + // a module whose whole declared scope is the framework shows an empty + // list, which is the one case where the filters do not thin a list but + // erase it. + val frameworkRequested = + requested && app.packageName == SYSTEM_FRAMEWORK_PACKAGE + val matchesSys = + inPlay || frameworkRequested || filters.showSystem || !app.isSystemApp + // No exemption needed on either of these: the framework row is built with + // `isGame = false` and is not an installed package, so it is not in the + // module set. Both already pass it. val matchesGame = inPlay || filters.showGames || !app.isGame val matchesModule = inPlay || showMods || modules == null || app.packageName !in modules diff --git a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/repo/RepoDetailsScreen.kt b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/repo/RepoDetailsScreen.kt index a27820153..07a66f5a4 100644 --- a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/repo/RepoDetailsScreen.kt +++ b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/repo/RepoDetailsScreen.kt @@ -93,6 +93,7 @@ import kotlinx.coroutines.launch import org.matrix.vector.manager.ui.theme.LocalizedOverlay import org.matrix.vector.manager.R import org.matrix.vector.manager.ui.theme.currentLocale +import org.matrix.vector.manager.data.model.ModuleDetection import org.matrix.vector.manager.data.model.OnlineModule import org.matrix.vector.manager.data.model.Release import org.matrix.vector.manager.data.model.ReleaseAsset @@ -130,6 +131,7 @@ fun RepoDetailsScreen(packageName: String, onNavigateBack: () -> Unit) { viewModel(factory = RepoDetailsViewModelFactory(packageName)) val state by viewModel.state.collectAsState() val installedScope by viewModel.installedScope.collectAsState() + val installedIsLegacy by viewModel.installedIsLegacy.collectAsState() val install by viewModel.installState.collectAsState() val context = LocalContext.current @@ -319,6 +321,7 @@ fun RepoDetailsScreen(packageName: String, onNavigateBack: () -> Unit) { module = module, listState = informationScroll, installedScope = installedScope, + installedIsLegacy = installedIsLegacy, onOpenUrl = openUrl, ) } @@ -748,6 +751,8 @@ private fun InformationTab( listState: LazyListState, /** What the copy on this device declares, when the catalogue declares nothing. */ installedScope: List, + /** Whether that copy is a legacy module, which decides how to read the catalogue's scope. */ + installedIsLegacy: Boolean, onOpenUrl: (String) -> Unit, ) { // Hoisted: the rows below are emitted from a LazyListScope, which is not a composable. @@ -767,7 +772,18 @@ private fun InformationTab( // into. The catalogue first, because it describes the published module. Failing that, // what the installed copy declares in its own APK — accurate for the build actually on // this device, and labelled as such so the two are not confused. - val published = module.scope?.takeIf { it.isNotEmpty() } + // + // The catalogue's list is written in the module's own vocabulary, and a legacy + // module's is the reverse of everything else here: its "android" is the system server + // and its "system" is the ordinary android package. The installed list beside it has + // already been through that swap on its way out of the APK, so without this the same + // module can name the same target two different ways in two adjacent lines. + val published = + module.scope + ?.takeIf { it.isNotEmpty() } + ?.let { + if (installedIsLegacy) ModuleDetection.swapLegacyFrameworkNames(it) else it + } InfoRow( icon = Icons.Rounded.TrackChanges, label = diff --git a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/repo/RepoDetailsViewModel.kt b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/repo/RepoDetailsViewModel.kt index 4d1ae526d..a7805c792 100644 --- a/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/repo/RepoDetailsViewModel.kt +++ b/manager/src/main/kotlin/org/matrix/vector/manager/ui/screens/repo/RepoDetailsViewModel.kt @@ -95,6 +95,24 @@ class RepoDetailsViewModel( private val _installedScope = MutableStateFlow>(emptyList()) val installedScope: StateFlow> = _installedScope.asStateFlow() + /** + * Whether the copy on this device is a legacy module, which decides how to read the + * *catalogue's* scope. + * + * The two generations spell the framework differently — see + * `ModuleDetection.swapLegacyFrameworkNames` — and the catalogue entry is written in whichever + * vocabulary its module belongs to. Nothing in the payload says which that is, so the installed + * copy is the only thing that can answer it, and only for a module that is installed at all. + * + * False while the module is absent, which is the honest answer rather than a safe one: with + * nothing on the device to inspect there is no way to know, and guessing would relabel a + * target on the strength of nothing. The consequence is a legacy module whose catalogue names + * `android` reading as `android` until it is installed and as `system` afterwards — visibly + * odd, and less misleading than the alternative, which is asserting one of the two at random. + */ + private val _installedIsLegacy = MutableStateFlow(false) + val installedIsLegacy: StateFlow = _installedIsLegacy.asStateFlow() + private fun readInstalledScope() { viewModelScope.launch(Dispatchers.IO) { val packageManager = ServiceLocator.context.packageManager @@ -110,6 +128,7 @@ class RepoDetailsViewModel( info.lastUpdateTime, ) _installedScope.value = manifest.scope + _installedIsLegacy.value = manifest.isLegacy } }