Bug 454651

Summary: Pager gets keyboard focus twice
Product: [Plasma] plasmashell Reporter: ratijas <me>
Component: PagerAssignee: Plasma Bugs List <plasma-bugs>
Severity: normal CC: hein, nate, niccolo.venerandi
Priority: NOR    
Version: master   
Target Milestone: 1.0   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed In: 5.26
Attachments: Panel focus

Description ratijas 2022-05-31 11:02:52 UTC
Created attachment 149353 [details]
Panel focus


Focusing elements with arrows behaves a bit strange: some elements like a Pager get focused twice, and some — like Minimize all Windows — don't get a focus ring at all. See the video where I focus the panel, and press Right key repeatedly.

0. Make sure you have at least two rows of Virtual Desktops. Add a Pager and Minimize all Windows applets to the panel.
1. Focus a panel with Meta+Alt+P shortcut
2. Press Right key repeatedly

First, Pager gets focus 4 times in a row (sorry for the pun), two of which look like a glitch, because they iterate over the applet twice without any visual clue why is that happening. Turned out, it's just a second/bottom row desktops getting focus after the first/top row ones. Some selection ring would help to distinguish. Unfortunately, many many applets suffer from poor keyboard navigation, so this is part of a bigger problem here.

Second, Minimize all Windows is not focusable in this way at all. Focus just wraps to the first applet on the left instead.

Pager should show some visual clues which desktop is currently focused, especially when there are two or more rows.

Minimize all Windows applet should be focusable with a keyboard the same way other applets are.

Operating System: Arch Linux
KDE Plasma Version: 5.25.80
KDE Frameworks Version: 5.95.0
Qt Version: 5.15.4
Kernel Version: 5.18.0-arch1-1 (64-bit)
Graphics Platform: X11
Processors: 8 × Intel® Core™ i7-6700HQ CPU @ 2.60GHz
Memory: 15.6 GiB of RAM
Graphics Processor: NVIDIA GeForce GTX 970M/PCIe/SSE2


For an experiment, I moved the Minimize all Windows applet to the middle of a panel, to mitigate the risk of "edge case" / off-by-one / "≤ vs. <" kind of error. It is still not focusable, so something is definitely wrong with the applet itself, not the panel.
Comment 1 Nate Graham 2022-06-01 18:53:26 UTC
Apps need to opt into receiving focus. Minimize All Windows probably hasn't.
Comment 2 ratijas 2022-06-01 20:28:51 UTC
> Apps need to opt into receiving focus. Minimize All Windows probably hasn't.

Technically speaking, how? I don't see anything related to focus management in appletinterface.cpp
Comment 3 Nate Graham 2022-06-03 22:10:36 UTC
(In reply to ratijas from comment #2)
> > Apps need to opt into receiving focus. Minimize All Windows probably hasn't.
> Technically speaking, how? I don't see anything related to focus management
> in appletinterface.cpp

Check out the MouseArea in the Show Desktop widget. That's how it's implemented. Commit 15ee61c8fae2ad494458ff2a7ee0a21db5e58fb3in plasma-desktop may also be illuminating.

Let's use this bug report to track the issue with the Pager, which I can reproduce. We should track the Minimize All applet needing focus support in a separate bug report (or preferably just an MR< since it's pretty simple to implement.
Comment 4 ratijas 2022-06-18 02:25:38 UTC
Speaking of the original title of this bug report…

> Keyboard focus (Meta+Alt+P) does not focus over some applets
Comment 5 Bug Janitor Service 2022-06-18 02:26:27 UTC
A possibly relevant merge request was started @ https://invent.kde.org/plasma/plasma-desktop/-/merge_requests/986
Comment 6 ratijas 2022-06-18 14:07:50 UTC
Git commit 152d00d7768e83ae28aa9ac4ffc10784bf94bc17 by ivan tkachenko.
Committed on 18/06/2022 at 13:33.
Pushed by ratijas into branch 'master'.

Sync/unify minimizeall & showdesktop applets, add support for keyboard navigation

This turns out to be quite a refactoring, but in the end we get a
flattened component hierarchy, consistent structure and behavior, and
of course the keyboard navigation (Meta+Alt+P).

For feature parity, minimizeall applet now also have a parity action
from its sister-applet.

M  +89   -66   applets/minimizeall/package/contents/ui/main.qml
M  +96   -104  applets/showdesktop/package/contents/ui/main.qml
M  +8    -0    applets/showdesktop/plugin/showdesktop.cpp
M  +1    -0    applets/showdesktop/plugin/showdesktop.h