| Summary: | Toggled repos in Discover jump to the bottom of the list, and other repo names are changed to undefined | ||
|---|---|---|---|
| Product: | [Applications] Discover | Reporter: | Kamil Paral <kparal> |
| Component: | discover | Assignee: | Dan Leinir Turthra Jensen <leinir> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | adamw, aleixpol, nate |
| Priority: | NOR | ||
| Version: | 5.23.0 | ||
| Target Milestone: | --- | ||
| Platform: | Other | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
|
Description
Kamil Paral
2021-10-11 09:13:14 UTC
so I've been looking into this, https://bugs.kde.org/show_bug.cgi?id=406295 and https://bugs.kde.org/show_bug.cgi?id=443455 because they're all kiiinda in the same area: enabling and disabling software sources, under different backends. It's interesting that the PackageKit, fwupd and flatpak backends all handle this very differently, it almost feels like the abstraction model should be at a higher level so they're forced to do it in more similar ways and can't have really different behaviours. In this case, as best as I can tell, the way the PackageKit backend works is through a lot of signal callbacks between the Discover backend and PackageKit itself. The Discover PK backend connects to PackageKit's repoListChanged signal, and any time that signal is triggered, it regenerates its *entire* internal sources list from scratch (it runs its own resetSources method). When you click to enable or disable a source in Discover, the PK backend intercepts what Qt would usually do when you click a checkbox, and just calls PackageKit's repoEnable function on the relevant repo and then returns; it doesn't even explicitly redraw the checkbox. Instead it relies on PK then emitting the repoListChanged and triggering a call to resetSources, which figures everything out from scratch again. Presumably when it does this, PackageKit sends the repos in a different order - I guess it sends them in order of last change? - which is why we see the 'jump to the bottom of the list' behaviour. So, theoretically we could fix that by having resetSources not just use the repos in whatever order it gets them from PackageKit, but sort them in some kind of consistent order (alphabetically?) so that hopefully we'll get the same ordering every time it's called. Alternatively, we could have it *not* rely on the repoListChanged -> resetSources trigger, but disconnect that when we're dealing with a checkbox click and just have it call repoEnable and then change the checkbox state if it's successful, or log an error and leave the checkbox state as-is if it's not. This is more or less what the fwupd backend tries to do...although 406295 is precisely about that not actually *working*, so we'd have to figure that out too. Oh, I'm not sure yet why we get the 'other repo names changed to undefined' behaviour. I suspect possibly the PK backend clearing and re-generating its entire source list somehow causes that, I'm not sure of the exact mechanism yet. Regarding the undefined problem, it's possibly because of the issue that was fixed by this patch: https://codereview.qt-project.org/c/qt/qtdeclarative/+/372646 PackageKit sources shouldn't be un/checkable, this will render properly soon as the different fixes land onto your distro, so you won't be getting unpredictable behaviour anymore. A possibly relevant merge request was started @ https://invent.kde.org/plasma/discover/-/merge_requests/190 That won't solve the ordering of the PK repos potentially changing when you do anything to them, though. For that it would need to *not* refresh the entire list on a checkbox click, or always order the repos in some predictable way, when refreshing them, I think? OK, so backporting https://codereview.qt-project.org/c/qt/qtdeclarative/+/372646 does seem to fix the 'undefined' problem. Now every time I change the state of a PackageKit repo, the *entire page* refreshes shortly afterwards, including all the sources from other backends, and they all show correct state - rather than only the sources from PK backends refreshing, and sources from other backends going to 'undefined'. It's still kinda surprising behaviour in a way that the entire page refreshes when you click a checkbox, but it'd be quite a major change to not do that. The ordering within the PK source list can still change, but we should be able to fix that by doing some consistent ordering when we construct that list. Aleix filed https://invent.kde.org/plasma/discover/-/merge_requests/192 that should address the remaining parts of this. I'll try it out later. Git commit 476d293f5362c8a175ea72d51189e4863a556b75 by Aleix Pol Gonzalez, on behalf of Aleix Pol. Committed on 19/10/2021 at 16:38. Pushed by apol into branch 'master'. pk: Allow enabling/disabling repositories M +1 -0 libdiscover/backends/PackageKitBackend/PackageKitSourcesBackend.cpp https://invent.kde.org/plasma/discover/commit/476d293f5362c8a175ea72d51189e4863a556b75 Git commit 6128a9a80dee6436e6460a4298e69e536129f55f by Aleix Pol Gonzalez, on behalf of Aleix Pol. Committed on 21/10/2021 at 22:24. Pushed by apol into branch 'Plasma/5.23'. pk: Allow enabling/disabling repositories (cherry picked from commit 476d293f5362c8a175ea72d51189e4863a556b75) M +1 -0 libdiscover/backends/PackageKitBackend/PackageKitSourcesBackend.cpp https://invent.kde.org/plasma/discover/commit/6128a9a80dee6436e6460a4298e69e536129f55f |