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

Update smithay and implement shortcuts inhibit protocol #37

Merged
merged 2 commits into from
Sep 21, 2022

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Sep 20, 2022

We may want to restrict what applications can use this protocol in some way? I guess the ecosystem is still figuring out how to restrict which Wayland protocols sandboxed (Flatpak) apps can use. For now, always allow.

Includes updates for split between `smithay::wayland::output` and
`smithay::output`.
TODO: restrict what apps can call this in some way.
Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

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

Looks good. Slight nitpick

return FilterResult::Intercept(Some(
action.clone(),
));
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we want to have an else here in the meantime? Just one complicated (maybe even hard-coded) shortcut to release the lock for the user?

As stated in the protocol:

The Wayland compositor is however under no obligation to disable all of its shortcuts, and may keep some special key combo for its own use, including but not limited to one allowing the user to forcibly restore normal keyboard events routing in the case of an unwilling client. The compositor may also use the same key combo to reactivate an existing shortcut inhibitor that was previously deactivated on user request.
When the compositor restores its own keyboard shortcuts, an "inactive" event is emitted to notify the client that the keyboard shortcuts inhibitor is not effectively active for the surface and seat any more, and the client should not expect to receive all keyboard events.

When the keyboard shortcuts inhibitor is inactive, the client has no way to forcibly reactivate the keyboard shortcuts inhibitor.

The user can chose to re-enable a previously deactivated keyboard shortcuts inhibitor using any mechanism the compositor may offer, in which case the compositor will send an "active" event to notify the client.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was thinking it could have some shortcut that works anyway. I wonder if there is anything that would be obvious to a user trying to get out of a program with shortcuts inhibited... not sure what other compositors are doing.

}

fn new_inhibitor(&mut self, inhibitor: KeyboardShortcutsInhibitor) {
// TODO: Restrict what apps can inhibit shortcuts
Copy link
Member

Choose a reason for hiding this comment

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

https://gitlab.freedesktop.org/wayland/wayland-protocols/-/merge_requests/68 should allow hiding a protocol to specific apps.

But even then, I would argue it should be exposed by default for new applications, so we can show a permission dialog on first-use. We can decline any request anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

For Flatpak, ideally "inhibit system shortcuts" could be listed as one of the permissions an app requires when you install it, just as permissions are shown on install on Android.

@ids1024 ids1024 merged commit f64b90b into master_jammy Sep 21, 2022
@ids1024 ids1024 deleted the shortcuts-inhibit_jammy branch September 21, 2022 18:20
@ids1024
Copy link
Member Author

ids1024 commented Sep 21, 2022

Merging for now, we can add a shortcut that isn't inhibited or work on policy for what apps can use this later.

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.

None yet

2 participants