Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

xdg-activation-v1: new protocol implementation #2718

Merged
merged 2 commits into from
Jun 2, 2021

Conversation

emersion
Copy link
Member

@emersion emersion commented Feb 4, 2021

@emersion emersion force-pushed the xdg-activation branch 2 times, most recently from c38cf54 to f04ed15 Compare March 11, 2021 18:40
@emersion emersion force-pushed the xdg-activation branch 2 times, most recently from 77757e9 to 42cf571 Compare March 23, 2021 10:15
emersion added a commit to emersion/sway that referenced this pull request Mar 23, 2021
emersion added a commit to emersion/sway that referenced this pull request Mar 23, 2021
@emersion emersion marked this pull request as ready for review May 4, 2021 07:20
emersion added a commit to emersion/sway that referenced this pull request May 4, 2021
emersion added a commit to emersion/sway that referenced this pull request May 4, 2021
@dnkl
Copy link

dnkl commented May 9, 2021

Getting a crash when I set a surface on the token, but nothing else:

struct xdg_activation_token_v1 *token = xdg_activation_v1_get_activation_token(term->wl->xdg_activation);
xassert(token != NULL);
xdg_activation_token_v1_add_listener(token, &activation_token_listener, term);
xdg_activation_token_v1_set_surface(token, term->window->surface);
xdg_activation_token_v1_commit(token);

Backtrace:

(gdb) bt full
#0  token_handle_commit (client=0x55555636c620, token_resource=0x555556397870)
    at ../../subprojects/wlroots/types/wlr_xdg_activation_v1.c:75
        token = 0x555556392770
        token_str = "4a0e176c7b01ce178bbaa2da310b0323\000"
        __PRETTY_FUNCTION__ = "token_handle_commit"
#1  0x00007ffff6f90acd in  () at /usr/lib/libffi.so.7

(gdb) p *token
$1 = {activation = 0x555555ebb710, surface = 0x5555563825e0, seat = 0x0, serial = 0, app_id = 0x0, link = {prev = 0x555556392798, 
    next = 0x555556392798}, token = 0x0, resource = 0x0, seat_destroy = {link = {prev = 0x5555563927b8, next = 0x5555563927b8}, 
    notify = 0x0}, surface_destroy = {link = {prev = 0x555556382b18, next = 0x555556382988}, 
    notify = 0x7ffff7f707aa <token_handle_surface_destroy>}}
	if (token->surface != token->seat->keyboard_state.focused_surface) {
		wlr_log(WLR_DEBUG, "Rejecting token commit request: "
			"surface doesn't have keyboard focus");
		goto error;
	}

The reason I did this was because without setting a surface, I got:

00:00:11.373 [DEBUG] [wlr] [types/wlr_xdg_activation_v1.c:71] Rejecting token commit request: surface has been destroyed

I was under the impression setting the surface was optional? What am I missing?

/**
 * @ingroup iface_xdg_activation_token_v1
 *
 * The requesting client can specify a surface to associate the token
 * being created with it.
 *
 * Must be triggered before commit. This information is optional.
 */

@emersion
Copy link
Member Author

Should be fixed now. I've tested with foot's xdg-activation branch and it seems to work as expected.

Setting the surface is optional, but is recommended so that the compositor can better track where the request comes from. A compositor might reject requests without a surface coming from clients which have surfaces opened, for instance.

emersion added a commit to emersion/sway that referenced this pull request May 10, 2021
@dnkl
Copy link

dnkl commented May 10, 2021

Thanks, works for me as well now. I've updated foot's xdg-activation branch to set the token surface to the top-level surface, and everything appears to (still) be working as expected.

@emersion
Copy link
Member Author

Compositors should probably:

  1. Invalidate all tokens on (keyboard?) focus change, assuming the user has moved on to something else and focus shouldn't be transferred. Except it may still be desirable to retain tokens to mark toplevels as urgent.
  2. Immediately destroy new tokens for serials older than the current focus. With the same caveats as above.

@djpohly
Copy link
Contributor

djpohly commented May 23, 2021

I'm trying to implement urgency in dwl using foot as a testcase, but I'm getting a null dereference in wlr_xdg_activation_v1.c line 75:

if (token->surface != token->seat->keyboard_state.focused_surface) {

with token->seat being NULL. A few lines later, the code does check to see if (token->seat != NULL) - maybe a few lines need to be reordered?

@dnkl
Copy link

dnkl commented May 23, 2021

It looks kind of like it (the xdg-activation) branch has been reverted to the original version, where Sway also crashed.

677d6a2 is the last version I tested (and found working) with foot. It looks vastly different.

@emersion
Copy link
Member Author

Hm, sorry, I probably messed up a rebase. I restored the branch.

emersion added a commit to emersion/sway that referenced this pull request Jun 1, 2021
@emersion
Copy link
Member Author

emersion commented Jun 1, 2021

Added a token timeout to avoid these dangling token issues.

@emersion emersion added this to the 0.14.0 milestone Jun 1, 2021
Copy link
Member

@kennylevinsen kennylevinsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

types/wlr_xdg_activation_v1.c Outdated Show resolved Hide resolved
types/wlr_xdg_activation_v1.c Outdated Show resolved Hide resolved
There isn't always a good time to prune old tokens. Compositors
which only implement a "give focus on activation" logic can prune
tokens on focus change. However other compositors might want to
implement other semantics, e.g. "mark urgent on activation". In this
case a focus change shouldn't invalidate other tokens.

Additionally, some tokens aren't necessarily tied to a seat.

To avoid ending up with an ever-growing list of tokens, add a timeout.
@emersion emersion merged commit 76f51a9 into swaywm:master Jun 2, 2021
@emersion emersion deleted the xdg-activation branch June 2, 2021 09:18
emersion added a commit to emersion/sway that referenced this pull request Jun 2, 2021
emersion added a commit to swaywm/sway that referenced this pull request Jun 2, 2021
GorrillaRibs pushed a commit to GorrillaRibs/sway-borders that referenced this pull request Jun 4, 2021
RagnarGrootKoerkamp pushed a commit to RagnarGrootKoerkamp/sway that referenced this pull request Jun 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants