Bug 443583 - Toggled repos in Discover jump to the bottom of the list, and other repo names are changed to undefined
Summary: Toggled repos in Discover jump to the bottom of the list, and other repo name...
Status: RESOLVED FIXED
Alias: None
Product: Discover
Classification: Applications
Component: discover (show other bugs)
Version: 5.23.0
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Dan Leinir Turthra Jensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-11 09:13 UTC by Kamil Paral
Modified: 2023-01-12 18:02 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kamil Paral 2021-10-11 09:13:14 UTC
This is an upstream report of Fedora downstream bug https://bugzilla.redhat.com/show_bug.cgi?id=2011774 . I'll reprint the comment 0 here, please read the downstream report for full information, and also to see the attachments.

With plasma-discover-5.22.90-3.fc35, repo configuration suffers from redrawing issues and confusing item reordering. If you enable/disable any repo in the "Fedora Linux 35" section, that repo, together will all other repos defined in that same file, is moved to the very bottom of the list (so e.g. "F35 Updates + Updates Debug + Updates Source" are moved to the bottom). If you don't pay close attention to this, you might get confused. For example if the 'updates' triplet jumps away and on the same place appears the 'fedora' triplet with the same selection as originally (the first item enabled, the other two disabled), you might not re-read the item again carefully and just assume that your click wasn't registered, and click again (and then it disappears from the view and you'll not realize you've just disabled the 'fedora' repo).

Additionally, each time you perform the change, all items in the "Firmware Updates" and "Flatpak" sections get renamed to "undefined". For some items, scrolling up and down is enough the redraw them into proper names, for other items, you need to change the current tab to some other one and back.

See the attached video: https://bugzilla.redhat.com/attachment.cgi?id=1830328

Version-Release number of selected component (if applicable):
plasma-discover-5.23.0-1.fc35

How reproducible:
always

Steps to Reproduce:
1. go to Discover -> Settings
2. click an RPM repo to enable/disable it
3. see the repo (and related ones) disappear, find them at the bottom
4. see other items in other sections renamed to "undefined"
Comment 1 Adam Williamson 2021-10-14 17:49:24 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.
Comment 2 Adam Williamson 2021-10-14 17:51:06 UTC
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.
Comment 3 Aleix Pol 2021-10-18 13:26:04 UTC
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.
Comment 4 Bug Janitor Service 2021-10-18 13:27:04 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/discover/-/merge_requests/190
Comment 5 Adam Williamson 2021-10-18 15:10:49 UTC
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?
Comment 6 Adam Williamson 2021-10-18 17:46:56 UTC
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.
Comment 7 Adam Williamson 2021-10-18 19:54:46 UTC
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.
Comment 8 Aleix Pol 2021-10-19 16:40:06 UTC
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
Comment 9 Aleix Pol 2021-10-21 22:24:15 UTC
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