[PATCH 1/2] Introduce keyboard grabbing protocol for Xwayland
Jonas Ådahl
jadahl at gmail.com
Wed Apr 12 08:32:45 UTC 2017
On Mon, Apr 03, 2017 at 09:45:46AM -0400, Olivier Fourdan wrote:
> Hey Peter,
>
> Thanks for the review!
>
> > woohoo, grabs. My favourite topic! ;)
> >
> > Mostly ok, a few complaints regarding the documentation but the protocol is
> > fine from my POV.
> >
> > On Wed, Mar 22, 2017 at 05:27:22PM +0100, Olivier Fourdan wrote:
> > > This patch introduces a new protocol for grabbing the keyboard from
> > > Xwayland.
> > >
> > > This is needed for X11 applications that map an override redirect window
> > > (ths not focused by the window manager) and issue an active grab on the
> >
> > /me buys a 'u'
>
> oh, 'u' are so overpriced these days... ^_~
>
> > > keyboard to capture all keyboard events.
> > >
> > > Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> > > ---
> > > Makefile.am | 2 +
> > > configure.ac | 2 +-
> > > unstable/xwayland-keyboard-grab/README | 4 +
> > > .../xwayland-keyboard-grab-unstable-v1.xml | 101
> > > +++++++++++++++++++++
> > > 4 files changed, 108 insertions(+), 1 deletion(-)
> > > create mode 100644 unstable/xwayland-keyboard-grab/README
> > > create mode 100644
> > > unstable/xwayland-keyboard-grab/xwayland-keyboard-grab-unstable-v1.xml
> > >
> > > diff --git a/Makefile.am b/Makefile.am
> > > index e693afa..d100c13 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -12,6 +12,8 @@ unstable_protocols = \
> > > unstable/tablet/tablet-unstable-v2.xml \
> > > unstable/xdg-foreign/xdg-foreign-unstable-v1.xml \
> > > unstable/idle-inhibit/idle-inhibit-unstable-v1.xml \
> > > + unstable/xwayland-keyboard-grab/xwayland-keyboard-grab-unstable-v1.xml \
> > > +
> > > unstable/keyboard-shortcuts-inhibit/keyboard-shortcuts-inhibit-unstable-v1.xml
> > > \
> > > $(NULL)
> > >
> > > stable_protocols = \
> > > diff --git a/configure.ac b/configure.ac
> > > index fbb0ec2..e98bceb 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -1,7 +1,7 @@
> > > AC_PREREQ([2.64])
> > >
> > > m4_define([wayland_protocols_major_version], [1])
> > > -m4_define([wayland_protocols_minor_version], [7])
> > > +m4_define([wayland_protocols_minor_version], [8])
> > > m4_define([wayland_protocols_version],
> > > [wayland_protocols_major_version.wayland_protocols_minor_version])
> > >
> > > diff --git a/unstable/xwayland-keyboard-grab/README
> > > b/unstable/xwayland-keyboard-grab/README
> > > new file mode 100644
> > > index 0000000..dbe45a5
> > > --- /dev/null
> > > +++ b/unstable/xwayland-keyboard-grab/README
> > > @@ -0,0 +1,4 @@
> > > +Xwayland keyboard grabbing protocol
> > > +
> > > +Maintainers:
> > > +Olivier Fourdan <ofourdan at redhat.com>
> > > diff --git
> > > a/unstable/xwayland-keyboard-grab/xwayland-keyboard-grab-unstable-v1.xml
> > > b/unstable/xwayland-keyboard-grab/xwayland-keyboard-grab-unstable-v1.xml
> > > new file mode 100644
> > > index 0000000..31dc365
> > > --- /dev/null
> > > +++
> > > b/unstable/xwayland-keyboard-grab/xwayland-keyboard-grab-unstable-v1.xml
> > > @@ -0,0 +1,101 @@
> > > +<?xml version="1.0" encoding="UTF-8"?>
> > > +<protocol name="xwayland_keyboard_grab_unstable_v1">
> > > +
> > > + <copyright>
> > > + Copyright © 2017 Red Hat Inc.
> > > +
> > > + Permission is hereby granted, free of charge, to any person obtaining a
> > > + copy of this software and associated documentation files (the
> > > "Software"),
> > > + to deal in the Software without restriction, including without limitation
> > > + the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > > + and/or sell copies of the Software, and to permit persons to whom the
> > > + Software is furnished to do so, subject to the following conditions:
> > > +
> > > + The above copyright notice and this permission notice (including the next
> > > + paragraph) shall be included in all copies or substantial portions of the
> > > + Software.
> > > +
> > > + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> > > OR
> > > + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > > + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> > > + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > > OTHER
> > > + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > > + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> > > + DEALINGS IN THE SOFTWARE.
> > > + </copyright>
> > > +
> > > + <description summary="Protocol for grabbing the keyboard from Xwayland">
> > > + This protocol specifies a way for Xwayland to request all keyboard
> > > + events to be forwarded to a surface even when the surface does not
> > > + have keyboard focus.
> > > +
> > > + Unlike the X11 protocol, Wayland doesn't have the notion of
> > > + active grab on the keyboard.
> > > +
> > > + When an X11 client acquires an active grab on the keyboard, all
> > > + key events are reported only to the grabbing X11 client.
> > > + When running in Xwayland, X11 applications may acquire an active
> > > + grab but that cannot be translated to the Wayland compositor who
> > > + may set the input focus to some other surface, thus breaking the
> > > + X11 client assumption that all key events are reported.
> > > +
> > > + When an X11 client requests a keyboard grab, the Wayland compositor
> > > + may either inform or ask the user for the right to forward all key
> > > + events to the given client surface.
> >
> > this is confusing :) paragraph 3 talks aobut what's not working and
> > paragraph 4 about what this protocol should fix. How about something like:
> >
> > This protocol is application-specific to meet the needs of the X11
> > protocol through Xwayland. It provides a way for XWayland to request
> > all keyboard events to be forwarded to a surface even when the
> > surface does not have keyboard focus.
> >
> > In the X11 protocol, a client may request an "active grab" on the
> > keyboard. On success, all key events are reported only to the
> > grabbing X11 client. For details, see XGrabKeyboard(3).
> >
> > The core Wayland protocol does not have a notion of an active
> > keyboard grab. When running in Xwayland, X11 applications may
> > acquire an active grab inside XWayland but that cannot be translated
> > to the Wayland compositor who may set the input focus to some other
> > surface. In doing so, it breaks the X11 client assumption that all
> > key events are reported to the grabbing client.
> >
> > This protocol specifies a way for Xwayland to request all keyboard
> > be directed to the given surface. The protocol does not guarantee
> > that the compositor will honor this request and it does not
> > prescribe user interfaces on how to handle the respond. For example,
> > a compositor may inform the user that all key events are now
> > forwarded to the given client surface, or it may ask the user for
> > permission to do so.
> >
> > Warning! Things may go boom... etc. etc.
>
> Yeap, I like that wording! I would just use "Xwayland" rather than "XWayland".
>
> > > +
> > > + Warning! The protocol described in this file is experimental and
> > > + backward incompatible changes may be made. Backward compatible
> > > + changes may be added together with the corresponding interface
> > > + version bump.
> > > + Backward incompatible changes are done by bumping the version
> > > + number in the protocol and interface names and resetting the
> > > + interface version. Once the protocol is to be declared stable,
> > > + the 'z' prefix and the version number in the protocol and
> > > + interface names are removed and the interface version number is
> > > + reset.
> > > + </description>
> > > +
> > > + <interface name="zwp_keyboard_grab_manager_v1" version="1">
> >
> > you've missed a big chance here by not calling it keyboard_grabber :)
>
> Aw snap! I didn't think of it! :)
>
> > > + <description summary="context object for keyboard grab_manager">
> > > + A global interface used for grabbing the keyboard.
> > > + </description>
> > > +
> > > + <request name="destroy" type="destructor">
> > > + <description summary="destroy the keyboard grab manager">
> > > + Destroy the keyboard grab manager.
> > > + </description>
> > > + </request>
> > > +
> > > + <request name="grab_keyboard">
> > > + <description summary="grab the keyboard to a surface">
> > > + The grab_keyboard request can be used by the client to ask for a
> > > + grab of the keyboard, effectively reporting all key events to a
> > > + surface.
> >
> > 'can be used' is superfluous:
> > The grab_keyboard request asks for a grab of the keyboard,
> > effectively reporting all key events to a surface.
> >
> >
> > > +
> > > + The protocol provides no guarantee that the grab is ever satisfied,
> > > + and does not require the compositor to send an error if the grab
> > > + cannot ever be satisfied. It is thus possible to request a keyboard
> > > + grab that will never be effective.
> >
> > I think this requires more specifics here. This is an application-specific
> > protocol so we can be a bit more precise in what the behaviour is that we
> > expect. First: "satisfied" is ambiguous in a keyboard grab - does that mean
> > it never activate at all? that it's an effective grab but no key events
> > may be sent (e.g. hotkey filtering)? that the grab is effective but may be
> > interrupted or terminated by focus changes invisible to Xwayland? All these
> > need to be spelled out. So something like:
> >
> > The protocol
> > * does not guarantee that the grab itself is applied for a surface,
> > the grab request may be silently ignored by the compositor,
> > * does not guarantee that any events are sent to this client event
> > if the grab is applied to a surface,
> > * does not guarantee that events sent to this client are exhaustive,
> > a compositor may filter some events for its own consumption
> > * does not guarantee that events sent to this client are continuous,
> > a compositor may change and return focus while the grab is
> > nominally active
>
> For that last point, I'd rather use:
>
> * does not guarantee that events sent to this client are continuous,
> a compositor may change and reroute keyboard events while the grab
> is nominally active.
>
> > hmm, and thinking about the last point: do we need to specify what the focus
> > behaviour is if a grab is active? I'm assuming 'no' because there is no
> > notification whatsoever whether it ever activates anyway, but...
>
> No, indeed, that's precisely the main difference with the shortcuts inhibitor logic for the wayland native apps.
>
> Here, keyboards events are routed to the surface regardless of the focus, just like an X11 grab.
Should this part really be respected though? In what circumstances does
it make sense to route input events to Xwayland when a Wayland client is
the one focused?
>
> > Last question: how will you deal with XGrabKeyboard() requests on already
> > grabbed keyboards? I can send that request twice with different grab windows
> > and it'll change the grab over (iirc). Xwayland will destroy the current
> > grab and request a new one?
>
> Yeap, good point, "XGrabKeyboard overrides any active keyboard grab by the client" so Xwayland needs to destroy any current grab before setting a new one in this case.
>
FWIW, this will create a minor race, where any key presses between the
.destroy and .grab requests will be ungrabbed.
Jonas
> Thanks!
> Olivier
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
More information about the wayland-devel
mailing list