xdg-activation: add options to make it more strict #2527
Conversation
plugins/protocols/xdg-activation.cpp
Outdated
|
|
||
| if (reject_token) | ||
| { | ||
| if (token == last_token) |
There was a problem hiding this comment.
While this technically works, why not simply use the token's destroy event? This here (+leaving the pointer dangling) seems like quite the hack.
There was a problem hiding this comment.
I think I had a reason for it, but cannot remember what it was :)
Although I originally did something more complicated and might be a leftover from that. I'll try again and see.
There was a problem hiding this comment.
OK, I tried and it seems to work well with the destroy signal as well
dkondor
force-pushed
the
xdg_activation_strict
branch
from
ee1a51b to
d1247b9
Compare
plugins/protocols/xdg-activation.cpp
Outdated
| return; | ||
| } | ||
| auto token = static_cast<struct wlr_xdg_activation_token_v1*>(data); | ||
| if (token == last_token) |
There was a problem hiding this comment.
Is it possible that this check is ever false?
There was a problem hiding this comment.
Actually, it should not matter, probably I added it as extra paranoia or as a remnant of a previous attempt where I kept this signal connected for multiple tokens (technically, it can be false if we already set last_token = nullptr on line 64, but that does not matter). I'll remove the check to make things more clear.
plugins/protocols/xdg-activation.cpp
Outdated
| return; | ||
| } | ||
|
|
||
| LOGI("Activating view"); |
There was a problem hiding this comment.
I know this message was there before, however I happened to look at this, do we want to keep this at LOGI or maybe demote to LOGD ?
There was a problem hiding this comment.
Sure, I think LOGD is fine
|
I've added a commit to address the comments -- do you need me to squash commits and / or rebase on main given the recent changes? |
|
OK, so the build failure looks weird and it seems related to the changes to master -- I'll rebase and try to fix them |
These allow: - reject requests without a view supplied - only allow activating with the last issued token Both are disabled by default. Also added an option to control the timeout used by wlroots for invalidating tokens.
dkondor
force-pushed
the
xdg_activation_strict
branch
from
bcc2d49 to
6c7e9c9
Compare
This adds three options to configure xdg-activation and allow more strict behavior. These include:
Currently, wlroots already rejects token creation if the client supplied a wl_surface which is not focused. However, if there is no surface, creating a token always succeeds, basically allowing any app to focus itself (e.g. using
gtk_window_present()which falls back to self-generated tokens with no surface if none is available in the environment). This can be a good thing e.g. when interacting with already running apps from the terminal, but can lead to unexpected focus changes as well. With this PR, this behavior can be changed according to individual preferences.A small test program is here: https://github.com/dkondor/xdg_activation_test -- it is a case of a parent passing tokens to a child process. Without this PR, all three cases in it always work. With the new, restrictive options enabled, only the first case works.
Note that this still does not do full focus stealing prevention, for that, we would need to keep track if the surface that requested the token loses focus. I would consider looking into that next if there is interest.
If this gets merged, I'm happy to add some notes to the Wiki on the options (and also on xdg-activation more generally).