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.

Sign up for GitHub

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

Jump to bottom

feature: implementation of xdg_activation_v1 because I want my auto focuses #3639

Merged
merged 8 commits into from
Oct 30, 2024
Merged

feature: implementation of xdg_activation_v1 because I want my auto focuses #3639

merged 8 commits into from
Oct 30, 2024

Conversation

mattkae
Copy link
Contributor

@mattkae mattkae commented Oct 16, 2024 •
edited
Loading

Screencast.from.2024-10-16.16-21-05.webm

What's new?

  • Added the xdg_activation_v1 protocol
  • Implemented the xdg_activation_v1 protocol

To test

  • Run miral-shell
  • Open gnome-terminal
  • Run gedit
  • Focus gnome-terminal again
  • Run gedit again
  • Note that gedit gets focus 🪄

Similarly:

  • Open up emacs
  • Open up firefox
  • In emacs, write https://google.com
  • Ctrl + Click the link
  • Watch as firefox gets focus!

Open questions

  • The "timeout" is not configurable at the moment, but perhaps it should be. However, I don't think we do per-protocol options AFAICT

Todos

  • I should probably write some WLCS tests!

@mattkae mattkae marked this pull request as ready for review October 16, 2024 20:20
@mattkae mattkae requested a review from a team as a code owner October 16, 2024 20:20
@AlanGriffiths
Copy link
Contributor

  • I don't currently have a way to check if a serial is valid, so that is left as a TODO. If anyone knows, please direct me in the right spot!

If you look around WindowWlSurfaceRole::input_event_for() you'll see existing code checks

AlanGriffiths
AlanGriffiths requested changes Oct 17, 2024
View reviewed changes
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

Interesting

src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
AlanGriffiths
AlanGriffiths requested changes Oct 17, 2024
View reviewed changes
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

More nits. Main thing left is the serial validation TODO

src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
@Saviq
Copy link
Collaborator

Saviq commented Oct 17, 2024

Ref.:

AlanGriffiths
AlanGriffiths requested changes Oct 18, 2024
View reviewed changes
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

A couple more tweaks, but looking good

src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
@mattkae
Validating input in the activation method rather than the commit, and…
e6be816 
… making sure that tokens are invalidated if a keyboard event comes in
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 29, 2024 •
edited
Loading

Everyone contributing to this PR have now signed the CLA. Thanks!

@mattkae mattkae force-pushed the feature/xdg_activation branch 2 times, most recently from cb81f45 to 53a2904 Compare October 29, 2024 14:24
@mattkae
pr feedback: listening for focus changes and ensuring that the newly …
38b7187 
…focused surface is the same session that requested the token
AlanGriffiths
AlanGriffiths requested changes Oct 29, 2024
View reviewed changes
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

Nearly there now

src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
src/server/frontend_wayland/xdg_activation_v1.cpp Outdated Show resolved Hide resolved
AlanGriffiths
AlanGriffiths approved these changes Oct 29, 2024
View reviewed changes
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

LGTM

@AlanGriffiths
Copy link
Contributor

@Saviq you OK with this version?

Saviq
Saviq approved these changes Oct 30, 2024
View reviewed changes
Copy link
Collaborator

@Saviq Saviq left a comment

Choose a reason for hiding this comment

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

Seems to work!

Just need to be default :)

@mattkae mattkae mentioned this pull request Oct 30, 2024
Open
AlanGriffiths
AlanGriffiths approved these changes Oct 30, 2024
View reviewed changes
Copy link
Contributor

@AlanGriffiths AlanGriffiths left a comment

Choose a reason for hiding this comment

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

Yes, even better now

@AlanGriffiths AlanGriffiths added this pull request to the merge queue Oct 30, 2024
Merged via the queue into main with commit faa942b Oct 30, 2024
39 checks passed
@AlanGriffiths AlanGriffiths deleted the feature/xdg_activation branch October 30, 2024 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Saviq Saviq Saviq approved these changes

@AlanGriffiths AlanGriffiths AlanGriffiths approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mattkae @AlanGriffiths @Saviq