Do not give menus keyboard focus (again) #2330
Conversation
wmww
force-pushed
the
popup-kb-focus-again
branch
from
da2f4b2
to
5da4e3e
Compare
wmww
force-pushed
the
popup-kb-focus-again
branch
from
5da4e3e
to
1c6f8d5
Compare
src/server/shell/abstract_shell.cpp
Outdated
| std::vector<std::shared_ptr<ms::Surface>> result; | ||
| for (auto item = surface; item; item = item->parent()) | ||
| { | ||
| result.insert(begin(result), item); |
There was a problem hiding this comment.
I know we're not going to have a lot of surfaces, but let's try not to be accidentally quadratic 😉
The order of the surfaces in this array isn't important to any of the other code that I can see?
src/server/shell/abstract_shell.cpp
Outdated
| return surface; | ||
| } | ||
|
|
||
| auto get_active_surfaces(std::shared_ptr<ms::Surface> surface) -> std::vector<std::shared_ptr<ms::Surface>> |
There was a problem hiding this comment.
This isn't getting the active surfaces; it's getting the chain of parent surfaces. We need that chain because we want a surface to be active if any of its children are active, but I think that's for the callsite, not the function name.
|
Empirically this does seem to fix the problem with kate menus. I agree with @RAOF that some of the code isn't clearly doing the right thing. Will try reworking that and then do some further testing with other client stacks. |
AlanGriffiths
force-pushed
the
popup-kb-focus-again
branch
from
6cba00b
to
4a80f63
Compare
|
Looks good! I tweaked comments/names and added some tests. I also proposed a relevant test to WLCS here: canonical/wlcs#223. I'm beginning to think this is not the right place for the popup-dismissing logic. Since the order is required by the Wayland protocol and the frontend has all the info it needs, perhaps the shell should just mark popups non-active and the frontend should dismiss them as needed. That way we don't have to worry about if the frontend processes close requests sequentially, and could hypothetically support protocols with different defined behavior. Out of scope for this PR, but might propose that in the future. |
2330: Do not give menus keyboard focus (again) r=Saviq a=wmww Fixes #2324, and refactors `AbstractShell::notify_focus_locked()` in an attempt to make future changes easier to understand. Co-authored-by: William Wold <wm@wmww.sh> Co-authored-by: Alan Griffiths <alan@octopull.co.uk>
|
Build failed: |
|
bors retry |
2330: Do not give menus keyboard focus (again) r=Saviq a=wmww Fixes #2324, and refactors `AbstractShell::notify_focus_locked()` in an attempt to make future changes easier to understand. Co-authored-by: William Wold <wm@wmww.sh> Co-authored-by: Alan Griffiths <alan@octopull.co.uk>
|
Build failed: |
|
Failed twice with "Cannot find cached LXD image for lxd:alpine-3.15" time for brute force... |
Fixes #2324, and refactors
AbstractShell::notify_focus_locked()in an attempt to make future changes easier to understand.