Bug 443984 - Show Energy Information… does nothing if kinfocenter is not installed
Summary: Show Energy Information… does nothing if kinfocenter is not installed
Status: RESOLVED FIXED
Alias: None
Product: frameworks-kdeclarative
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: 5.90.0
Platform: Other Linux
: NOR minor
Target Milestone: ---
Assignee: Kai Uwe Broulik
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-10-18 17:36 UTC by Albert Astals Cid
Modified: 2022-02-08 15:35 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.92


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Albert Astals Cid 2021-10-18 17:36:54 UTC
STEPS TO REPRODUCE
1. Open the battery applet
2. Click on the hamburger menu
3. Click in Show Energy Information…

OBSERVED RESULT

Nothing happens

EXPECTED RESULT

You get a message saying "Please install kinfocenter". I guess another acceptable solution would be to not show the "Show Energy Information…" if kinfocenter is not installed

KDE Plasma Version: 5.23.0
KDE Frameworks Version: 5.87.0
Qt Version: 5.15.2+kde
Comment 1 Nate Graham 2022-01-07 14:55:15 UTC
Showing this menu item/button is gated behind `KCMShell.authorize("kcm_energyinfo.desktop").length > 0`. Looks like that doesn't return false when the desktop file in question isn't found.
Comment 2 Nate Graham 2022-01-07 15:06:09 UTC
...And that calls KAuthorized::authorizeControlModules(), which only checks for whether there are authorization restrictions in kdeglobals under "KDE Control Module Restrictions". So according to this function, it is perfectly valid to report that a module is authorized even if it is not installed.

Options for fixing this
1. Change the definition of "authorized" to include "installed" in KAuthorized::authorizeControlModules()
2. Change the definition of "authorized" to include "installed" in KCMShell::authorize()
3. Create a new function KCMShell::authorizedAndInstalled() that uses KCMShell::authorize()
but also checks installation status, and port all uses of KCMShell::authorize() to it
4. Add an additional check for installation status in every QML applet that uses KCMShell::authorize()

Option 1 would be easiest and involve the least new code and porting churn, but would entail changing the what the function returns in ways that callers might theoretically not want. Though it's hard for me to even imagine a use case for calling KAuthorized::authorizeControlModules() on KCMs that aren't installed and actually wanting it to return true. I suspect if I submit a merge request for this, Frameworks people will complain about it. But maybe not.

Option 3 is probably the most correct, but once we did it, we'd port everything that uses KCMShell::authorize() to KCMShell::authorizedAndInstalled(), begging the question of why anyone would want to use KCMShell::authorize() anymore given that it doesn't also factor in installation status.

Albert, what do you think?
Comment 3 Albert Astals Cid 2022-01-07 21:51:53 UTC
Not an expert in this part of the stack but given the names of the classes i would go with 2.

Making KAuthorized be about anything else than authorized may not be the best idea, but adding the installed part to the KCMShell makes more sense, but again as said, not an expert of the stack so my opinion is not super worthy
Comment 4 Nate Graham 2022-01-07 21:57:17 UTC
I'm good with #2, that seems reasonable. I'll submit a merge request.
Comment 5 Bug Janitor Service 2022-01-07 22:20:16 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kdeclarative/-/merge_requests/100
Comment 6 Nate Graham 2022-02-07 20:14:35 UTC
Actually it looks like KCMShell::openInfoCenter() is already trying to detect KInfoCenter's installation status, but failing. I'll fix that bug rather than working around it or using a different approach.
Comment 7 Bug Janitor Service 2022-02-07 20:17:30 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kdeclarative/-/merge_requests/113
Comment 8 Nate Graham 2022-02-08 15:35:32 UTC
Git commit d146c4fda13b7bfdcb07c59bb38c19959d5c621b by Nate Graham.
Committed on 07/02/2022 at 20:14.
Pushed by ngraham into branch 'master'.

Improve Open[app]() functions

These functions had divergent code, and openInfoCenter was not working
properly because it was following the pattern set by openSystemSettings
which did not work because System Settings' desktop file does not follow
rDNS style naming, but other ones do.
FIXED-IN: 5.92

M  +12   -8    src/qmlcontrols/kquickcontrolsaddons/kcmshell.cpp

https://invent.kde.org/frameworks/kdeclarative/commit/d146c4fda13b7bfdcb07c59bb38c19959d5c621b