activation_token: Provide GTK 4 helper

[A6GibKm]

cc @jsparber

I noticed that cargo clippy --features=gtk4 -- -W clippy::pedantic will complain:

warning: casting from `*const glib::Class<gtk4::gdk4::AppLaunchContext>` to a more-strictly-aligned pointer (`*mut gtk4::gio::gio_sys::GAppLaunchContextClass`) (1 < 8 bytes)
  --> src/activation_token/mod.rs:63:17
   |
63 |                 std::ptr::addr_of!((*context.class().parent()?)) as *mut _;
   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#cast_ptr_alignment
   = note: `-W clippy::cast-ptr-alignment` implied by `-W clippy::pedantic`
   = help: to override `-W clippy::pedantic` add `#[allow(clippy::cast_ptr_alignment)]`

[A6GibKm]

Should this add the v2_76 feature to glib?

[bilelmoussaoui]

Should this add the v2_76 feature to glib?

We will need to figure out a way to expose that, yes. Enabling it by default is a no-no

[jsparber]

I'm not sure having a helper like this is helpful. That your helper introduces more confusion. I have the worry that users may think they can keep the ActivationToken object around, but they actually can't since it may be invalid.

Maybe a better way would be to hide the activation token from the user all together and call g_app_launch_context_get_startup_notify_id () just right before we send the DBus message if the user opts in.

For context have a look on how the token is generated: https://gitlab.gnome.org/GNOME/gtk/-/blob/main/gdk/wayland/gdkapplaunchcontext-wayland.c#L65

[A6GibKm]

I'm not sure having a helper like this is helpful. That your helper introduces more confusion. I have the worry that users may think they can keep the ActivationToken object around, but they actually can't since it may be invalid.

Maybe a better way would be to hide the activation token from the user all together and call g_app_launch_context_get_startup_notify_id () just right before we send the DBus message if the user opts in.

For context have a look on how the token is generated: https://gitlab.gnome.org/GNOME/gtk/-/blob/main/gdk/wayland/gdkapplaunchcontext-wayland.c#L65

Alternatively, one can make the token non-clonable+require ownership, hence single use. But yeah, this is something to take into account.

[jsparber]

I'm not sure having a helper like this is helpful. That your helper introduces more confusion. I have the worry that users may think they can keep the ActivationToken object around, but they actually can't since it may be invalid.
Maybe a better way would be to hide the activation token from the user all together and call g_app_launch_context_get_startup_notify_id () just right before we send the DBus message if the user opts in.
For context have a look on how the token is generated: https://gitlab.gnome.org/GNOME/gtk/-/blob/main/gdk/wayland/gdkapplaunchcontext-wayland.c#L65

Alternatively, one can make the token non-clonable+require ownership, hence single use. But yeah, this is something to take into account.

It could be an option to get the activation token from the launch context only on deref().

Would be good to have some usage examples.

[bilelmoussaoui]

I'm not sure having a helper like this is helpful. That your helper introduces more confusion. I have the worry that users may think they can keep the ActivationToken object around, but they actually can't since it may be invalid.

Well, one can make all the methods that takes an ActivationToken not take a reference.

Maybe a better way would be to hide the activation token from the user all together and call g_app_launch_context_get_startup_notify_id () just right before we send the DBus message if the user opts in.

That assumes the library is targetting gtk only, which is not.

For context have a look on how the token is generated: https://gitlab.gnome.org/GNOME/gtk/-/blob/main/gdk/wayland/gdkapplaunchcontext-wayland.c#L65

Well, it seems there is no way to destroy the xdg-activation? so what would make it invalid per your sentence above?

Imho, we are already doing exactly this for the WindowIdentifier. Except in those cases, we have a proper way to unexport the handle under wayland which we are properly handling under wayland.

[A6GibKm]

I'm not sure having a helper like this is helpful. That your helper introduces more confusion. I have the worry that users may think they can keep the ActivationToken object around, but they actually can't since it may be invalid.

Well, one can make all the methods that takes an ActivationToken not take a reference.

If we want to be strict:

• Remove the current From<String/str> and make a unsafe fn from_string method (since those are clonable)
• Remove the Clone trait
• Do not take references to activation tokens on any method that uses them (this is already the case, afaik)
• Remove deserialize to enforce that from_string is unsafe

I feel we can get away with just removing the Clone trait and documenting it better.

[jsparber]

Well, it seems there is no way to destroy the xdg-activation? so what would make it invalid per your sentence above?

It's not that the token gets invalidated. But it may not be accepted any more for activation from Mutter. The token is tight to the user interaction. See: https://wayland.app/protocols/xdg-activation-v1

[jsparber]

Imho, we are already doing exactly this for the WindowIdentifier. Except in those cases, we have a proper way to unexport the handle under wayland which we are properly handling under wayland.

Yeah, i saw how you handle WindowIdentifier and it matches the suggestion here. But activation token works differently.

That assumes the library is targetting gtk only, which is not.

I didn't mend that we shouldn't remove the current way of setting the activation token.

[bilelmoussaoui]

Yeah, i saw how you handle WindowIdentifier and it matches the suggestion here. But activation token works differently.

What are the differences?

I didn't mend that we shouldn't remove the current way of setting the activation token.

Understood that

It's not that the token gets invalidated. But it may not be accepted any more for activation from Mutter

I don't see where in the spec it says anything like that?

[jsparber]

What are the differences?

The activation token should be created with a recent serial (generally obtained from an input event). The window handle on the other side is constant and valid till it's unexported.

I don't see where in the spec it says anything like that?

https://wayland.app/protocols/xdg-activation-v1#xdg_activation_token_v1:request:set_serial
Some compositors might refuse to activate toplevels when the token doesn't have a valid and recent enough event serial.

[bilelmoussaoui]

Sure, but nothing in the currently suggested API makes it sound like you can keep it around. We can just make any API that takes an ActivationToken takes it by value instead of by ref, remove Clone implementation and you are done? Basically what @A6GibKm suggested above.

[jsparber]

I think consuming and not allowing clones of ActivationToken is a good idea .

My concern is that the developer gets the impression that the ActivationToken is tide to the Gtk::Window which is mostly false. It's tide to the GdkWalyandDisplay, the current user input (serial) and current focus window (wayland surface).

Also we really don't want developers to call ActicationToken::from_window() at a random time. We need to make sure that the developer obtains the token immediately after user input that activates a different window/app (focus and/or launched) and that the developer make the portal request as quickly as possible before future user input happens.

I'm not sure that good documentation is enough.

[bilelmoussaoui]

Also we really don't want developers to call ActicationToken::from_window() at a random time. We need to make sure that the developer obtains the token immediately after user input that activates a different window/app (focus and/or launched) and that the developer make the portal request as quickly as possible before future user input happens.

Wait what, you would call the portal the moment the user clicks on a button. Why would you create it beforehand ? I am not sure about your worry here. People would just copy paste the examples, so if the examples are correct I don't see what more we could do here.

[jsparber]

Wait what, you would call the portal the moment the user clicks on a button.

Yes, that's how focus stealing prevention works. I really need to finish my blogpost about this.

[bilelmoussaoui]

I understand what you are saying here, but your "requirements" can not be worked around by a "better API", it is the programmer's job. If you think this could be documented a bit better, sure.
Otherwise, I don't really see any solution here

[bilelmoussaoui]

As the required glib patch have made it to a release, can we get this updated please? thanks!

[A6GibKm]

The bindings still don't accept an option https://gtk-rs.org/gtk-rs-core/git/docs/gio/prelude/trait.AppLaunchContextExt.html#method.startup_notify_id. One option is to use the method via ffi and the other is to wait for bindings and implement it then.

EDIT: it seems the nullable part has not reached gtk-rs-core's master.

[bilelmoussaoui]

The bindings still don't accept an option https://gtk-rs.org/gtk-rs-core/git/docs/gio/prelude/trait.AppLaunchContextExt.html#method.startup_notify_id. One option is to use the method via ffi and the other is to wait for bindings and implement it then.

EDIT: it seems the nullable part has not reached gtk-rs-core's master.

We have not updated gir-files in a long time. I will try to take care of that this weekend

[bilelmoussaoui]

EDIT: it seems the nullable part has not reached gtk-rs-core's master.

it is there now