[plasmashell] [Bug 443984] Show Energy Information… does nothing if kinfocenter is not installed

2022-01-07 Thread Bug Janitor Service
https://bugs.kde.org/show_bug.cgi?id=443984

Bug Janitor Service  changed:

   What|Removed |Added

 Status|CONFIRMED   |ASSIGNED

--- Comment #5 from Bug Janitor Service  ---
A possibly relevant merge request was started @
https://invent.kde.org/frameworks/kdeclarative/-/merge_requests/100

-- 
You are receiving this mail because:
You are watching all bug changes.

[plasmashell] [Bug 443984] Show Energy Information… does nothing if kinfocenter is not installed

2022-01-07 Thread Nate Graham
https://bugs.kde.org/show_bug.cgi?id=443984

--- Comment #4 from Nate Graham  ---
I'm good with #2, that seems reasonable. I'll submit a merge request.

-- 
You are receiving this mail because:
You are watching all bug changes.

[plasmashell] [Bug 443984] Show Energy Information… does nothing if kinfocenter is not installed

2022-01-07 Thread Albert Astals Cid
https://bugs.kde.org/show_bug.cgi?id=443984

--- Comment #3 from Albert Astals Cid  ---
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

-- 
You are receiving this mail because:
You are watching all bug changes.

[plasmashell] [Bug 443984] Show Energy Information… does nothing if kinfocenter is not installed

2022-01-07 Thread Nate Graham
https://bugs.kde.org/show_bug.cgi?id=443984

--- Comment #2 from Nate Graham  ---
...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?

-- 
You are receiving this mail because:
You are watching all bug changes.

[plasmashell] [Bug 443984] Show Energy Information… does nothing if kinfocenter is not installed

2022-01-07 Thread Nate Graham
https://bugs.kde.org/show_bug.cgi?id=443984

--- Comment #1 from Nate Graham  ---
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.

-- 
You are receiving this mail because:
You are watching all bug changes.

[plasmashell] [Bug 443984] Show Energy Information… does nothing if kinfocenter is not installed

2022-01-04 Thread Nate Graham
https://bugs.kde.org/show_bug.cgi?id=443984

Nate Graham  changed:

   What|Removed |Added

 Status|REPORTED|CONFIRMED
 CC||n...@kde.org
 Ever confirmed|0   |1
   Severity|normal  |minor

-- 
You are receiving this mail because:
You are watching all bug changes.