Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement activation tokens for X11 #4

Merged
merged 8 commits into from
Jul 20, 2023

Conversation

notgull
Copy link

@notgull notgull commented Jul 13, 2023

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

cc rust-windowing#2955

Adds the parallel activation context things to X11. None of this code has been tested.

It seems like the actual activation token from request_activation_token doesn't really matter, so I've just implemented it using the recommendation that the specs give.

This is based on rust-windowing#2825, while the activation_token branch for this repo seems to be based on an older commit. Hence the extra noise.

Copy link
Owner

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

I'll test it once we have issues resolved.

I've also rebased my branch, so it should be easier now.

src/platform_impl/linux/x11/activation.rs Outdated Show resolved Hide resolved
src/platform_impl/linux/x11/activation.rs Show resolved Hide resolved
src/platform_impl/linux/x11/activation.rs Outdated Show resolved Hide resolved
src/platform_impl/linux/x11/activation.rs Outdated Show resolved Hide resolved
src/platform_impl/linux/x11/activation.rs Outdated Show resolved Hide resolved
src/platform_impl/linux/x11/mod.rs Show resolved Hide resolved
src/platform_impl/linux/x11/mod.rs Outdated Show resolved Hide resolved
src/platform_impl/linux/x11/activation.rs Outdated Show resolved Hide resolved
src/platform_impl/linux/x11/activation.rs Outdated Show resolved Hide resolved
src/platform_impl/linux/x11/activation.rs Show resolved Hide resolved
src/platform_impl/linux/x11/activation.rs Outdated Show resolved Hide resolved
src/platform_impl/linux/x11/activation.rs Outdated Show resolved Hide resolved
src/platform_impl/linux/x11/activation.rs Outdated Show resolved Hide resolved
@psychon
Copy link

psychon commented Jul 13, 2023

I'll test it once we have issues resolved.

Random idea for that: libstartup-notification contains test/test-watch-xmessages.c and test-monitor.c. I'm not 100% sure what the difference between the two is, but they could be used for some basic testing: Run one of these programs and check that it prints the expected events.

@kchibisov
Copy link
Owner

You could also test on alacritty if you have something that can pass startup notify.

alacritty/alacritty#7072

That's how I've done wayland testing ^

kchibisov and others added 4 commits July 14, 2023 17:08
The utils in this module should help the users to activate the windows
they create, as well as manage activation tokens environment variables.

The API is essential for Wayland in the first place, since some
compositors may decide initial focus of the window based on whether
the activation token was during the window creation.

Fixes rust-windowing#2279.
Comment on lines 1713 to 1721
let atoms = self.xconn.atoms();
let title = {
let title_bytes = self
.xconn
.get_property(self.xwindow, atoms[_NET_WM_NAME], atoms[UTF8_STRING])
.expect("Failed to get WM_NAME property");

String::from_utf8(title_bytes).map_err(|_| NotSupportedError::new())?
};
Copy link
Owner

Choose a reason for hiding this comment

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

Don't we have a title() getter already? I'm pretty sure you can just self.title().

Copy link
Author

Choose a reason for hiding this comment

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

The title() getter returns an empty string.

// Get the activation token and then put it in the event queue.
let token = self
.xconn
.request_activation_token(&title)
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe we should request the token right before sending it back to the user?

Copy link
Author

Choose a reason for hiding this comment

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

Good idea, I've implemented this.

@kchibisov
Copy link
Owner

@notgull I'm not sure whether we should really add this testing example, because it requires installing the desktop file. How were you testing it btw in your setup?

@kchibisov kchibisov merged commit 3fd22a4 into kchibisov:activation-token Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants