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 <ofour...@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 <ofour...@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-de...@lists.x.org: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: https://lists.x.org/mailman/listinfo/xorg-devel _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
