[v4,wayland-protocols] virtual-keyboard: Add new virtual keyboard protocol

Submitted by Dorota Czaplejewicz on June 25, 2018, 6:36 p.m.

Details

Message ID 20180625183606.32205-1-dorota.czaplejewicz@puri.sm
State Superseded
Headers show
Series "virtual-keyboard: Add new virtual keyboard protocol" ( rev: 4 ) in Wayland

Not browsing as part of any series.

Commit Message

Dorota Czaplejewicz June 25, 2018, 6:36 p.m.
Provides the ability to emulate keyboards by applications. Complementary to input-method protocol.

The interface is a mirror copy of wl_keyboard, with removed serials, and added seat binding.
---
Hello,

thank you for giving me a lot of useful feedback in the last round. I applied the suggestions and this is the new incarnation of the protocol. Changes to v3, in no particular order:

- added clarification to the destructor
- namespaced enum references
- made virtual keyboard creation non-fatal

The responses about keeping a virtual keyboard on a separate connection and about using protocol errors as part of the authentication mechanism were all in agreement, and they made sense to me too. As a result, there is a whole new mechanism for that.

The client now requests a creation of a virtual keyboard, but does not immediately get a valid instance. It must wait for one of two events in virtual_keyboard_manager: `virtual_keyboard_create_failed` for a failure or `virtual_keyboard_created` for success. I deliberately kept them inside the manager instead of virtual_keyboard, since I don't like the idea of creating an object and immediately sending a message that it's useless (in case of failure).

I also deliberately kept the names long in order not to forget what's being manipulated.

The serial number is there to match up requests on different seats with failures. I imagine that a compositor might spam the user(s) with a number of permission requests for different seats, and the user would deal with them in a random order. Serials prevent the answers from getting mixed up.

I am not happy only with one aspect of this: the amount of time between create request and response is indefinite, opening a loophole for the compositor never to reply. I decided that it doesn't warrant a protocol error however.

As usual, feedback is welcome.

Cheers,
Dorota Czaplejewicz

 Makefile.am                                        |   1 +
 unstable/virtual-keyboard/README                   |   2 +
 .../virtual-keyboard-unstable-v1.xml               | 154 +++++++++++++++++++++
 3 files changed, 157 insertions(+)
 create mode 100644 unstable/virtual-keyboard/README
 create mode 100644 unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml

Patch hide | download patch | download mbox

diff --git a/Makefile.am b/Makefile.am
index 1aa13cf..17cedc1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -19,6 +19,7 @@  unstable_protocols =								\
 	unstable/keyboard-shortcuts-inhibit/keyboard-shortcuts-inhibit-unstable-v1.xml \
 	unstable/xdg-output/xdg-output-unstable-v1.xml				\
 	unstable/input-timestamps/input-timestamps-unstable-v1.xml	\
+	unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml	\
 	$(NULL)
 
 stable_protocols =								\
diff --git a/unstable/virtual-keyboard/README b/unstable/virtual-keyboard/README
new file mode 100644
index 0000000..a2c646d
--- /dev/null
+++ b/unstable/virtual-keyboard/README
@@ -0,0 +1,2 @@ 
+Virtual keyboard protocol
+
diff --git a/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml b/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
new file mode 100644
index 0000000..173f3bc
--- /dev/null
+++ b/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
@@ -0,0 +1,154 @@ 
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="virtual_keyboard_unstable_v1">
+  <copyright>
+    Copyright © 2008-2011  Kristian Høgsberg
+    Copyright © 2010-2013  Intel Corporation
+    Copyright © 2012-2013  Collabora, Ltd.
+    Copyright © 2018       Purism SPC
+
+    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>
+
+  <interface name="zwp_virtual_keyboard_v1" version="1">
+    <description summary="virtual keyboard">
+      The virtual keyboard provides an application with requests which emulate
+      the behaviour of a physical keyboard.
+
+      This interface can be used by clients on its own to provide raw input
+      events, or it can accompany the input method protocol.
+    </description>
+
+    <request name="keymap">
+      <description summary="keyboard mapping">
+        Provide a file descriptor to the compositor which can be memory-mapped
+        to provide a keyboard mapping description.
+      </description>
+      <arg name="format" type="uint" enum="wl_keyboard.keymap_format"
+        summary="keymap format"/>
+      <arg name="fd" type="fd" summary="keymap file descriptor"/>
+      <arg name="size" type="uint" summary="keymap size, in bytes"/>
+    </request>
+    
+    <enum name="error">
+      <entry name="no_keymap" value="0" summary="No keymap was set"/>
+    </enum>
+
+    <request name="key">
+      <description summary="key event">
+        A key was pressed or released.
+        The time argument is a timestamp with millisecond granularity, with an
+        undefined base. All requests regarding a single object must share the
+        same clock.
+
+        Keymap must be set before issuing this request.
+      </description>
+      <arg name="time" type="uint"
+        summary="timestamp with millisecond granularity"/>
+      <arg name="key" type="uint" summary="key that produced the event"/>
+      <arg name="state" type="uint" enum="wl_keyboard.key_state"
+        summary="physical state of the key"/>
+    </request>
+
+    <request name="modifiers">
+      <description summary="modifier and group state">
+        Notifies the compositor that the modifier and/or group state has
+        changed, and it should update state.
+
+        Keymap must be set before issuing this request.
+      </description>
+      <arg name="mods_depressed" type="uint" summary="depressed modifiers"/>
+      <arg name="mods_latched" type="uint" summary="latched modifiers"/>
+      <arg name="mods_locked" type="uint" summary="locked modifiers"/>
+      <arg name="group" type="uint" summary="keyboard layout"/>
+    </request>
+
+    <request name="destroy" type="destructor">
+      <description summary="destroy the virtual keyboard object"/>
+    </request>
+  </interface>
+
+  <interface name="zwp_virtual_keyboard_manager_v1" version="1">
+    <description summary="virtual keyboard manager">
+      A virtual keyboard manager allows an application to provide keyboard
+      input events as if they came from a physical keyboard.
+
+      If the compositor enables a keyboard to perform arbitrary actions, it
+      should prevent untrusted clients from using this interface.
+    </description>
+
+    <request name="create_virtual_keyboard">
+      <description summary="create a new virtual keyboard">
+        Asks to create a new virtual keyboard associated to a seat.
+
+        If the client is found unauthorized to create a virtual keyboard, then
+        it must receive the virtual_keyboard_create_failed event, with the
+        reason field set to unauthorized.
+
+        Otherwise, the client must receive the virtual_keyboard_created event.
+
+        Serial numbers used in consecutive create_virtual_keyboard requests
+        must form a sequence without repetitions.
+      </description>
+      <arg name="seat" type="object" interface="wl_seat"/>
+      <arg name="serial" type="uint"/>
+    </request>
+
+    <enum name="virtual_keyboard_create_failed_reason">
+      <entry name="unauthorized" value="0"
+        summary="client not authorized to use the request"/>
+    </enum>
+
+    <event name="virtual_keyboard_create_failed">
+      <description summary="creation of a new virtual keyboard failed">
+        Indicates that a new virtual keyboard instance could not be created.
+
+        Reason contains the reason why creation did not succeed.
+
+        Serial is the serial number of the corresponding
+        create_virtual_keyboard request.
+      </description>
+      <arg name="reason" type="uint"
+        enum="virtual_keyboard_create_failed_reason"
+        summary="indicate the reason for this failure"/>
+      <arg name="serial" type="uint" description="serial number of request"/>
+    </event>
+
+    <event name="virtual_keyboard_created">
+      <description summary="successfully created a new virtual keyboard">
+        Provides the client with the zwp_virtual_keyboard_v1 which was
+        previously requested using create_virtual_keyboard.
+
+        Serial is the serial number of the corresponding
+        create_virtual_keyboard request.
+      </description>
+      <arg name="id" type="new_id" interface="zwp_virtual_keyboard_v1"
+        summary="the id of the newly created virtual keyboard"/>
+      <arg name="serial" type="uint" description="serial number of request"/>
+    </event>
+
+    <request name="destroy" type="destructor">
+      <description summary="destroy the virtual keyboard manager object">
+        Destroys the virtual keyboard manager.
+
+        Existing zwp_virtual_keyboard_v1 objects remain valid.
+      </description>
+    </request>
+  </interface>
+</protocol>

Comments

> Provides the ability to emulate keyboards by applications. Complementary to
> input-method protocol.
>
> The interface is a mirror copy of wl_keyboard, with removed serials, and added
> seat binding.
>
> ---
>
> Hello,
>
> thank you for giving me a lot of useful feedback in the last round. I applied
> the suggestions and this is the new incarnation of the protocol. Changes to v3,
> in no particular order:
>
> - added clarification to the destructor namespaced enum references made virtual
> - keyboard creation non-fatal
>
> The responses about keeping a virtual keyboard on a separate connection and
> about using protocol errors as part of the authentication mechanism were all in
> agreement, and they made sense to me too. As a result, there is a whole new
> mechanism for that.
>
> The client now requests a creation of a virtual keyboard, but does not
> immediately get a valid instance. It must wait for one of two events in
> virtual_keyboard_manager: `virtual_keyboard_create_failed` for a failure or
> `virtual_keyboard_created` for success. I deliberately kept them inside the
> manager instead of virtual_keyboard, since I don't like the idea of creating an
> object and immediately sending a message that it's useless (in case of failure).
>
> I also deliberately kept the names long in order not to forget what's being
> manipulated.
>
> The serial number is there to match up requests on different seats with
> failures. I imagine that a compositor might spam the user(s) with a number of
> permission requests for different seats, and the user would deal with them in a
> random order. Serials prevent the answers from getting mixed up.
>
> I am not happy only with one aspect of this: the amount of time between create
> request and response is indefinite, opening a loophole for the compositor never
> to reply. I decided that it doesn't warrant a protocol error however.
>
> As usual, feedback is welcome.

Hi,

Sorry for the delay.

I'm not sure I like this new design.

First, this is using server-side created resources (the a `new_id` argument in
an event). One should be really careful when doing so, see Pekka's post on the
matter [1].

Then, this serial mechanism makes me uneasy. A more idiomatic approach would be
to use a proper object for this.

Finally, I'm not even sure this security mechanism belongs here. I think adding
this mechanism to potentially all privileged protocols will result in duplicated
code and "pollutes" protocols. Here are some other solutions:

* Have one keyboard daemon they spawn themselves. In this case they can only
  expose this interface on the daemon's display. This is e.g. weston's approach
  for its private protocols, and will be sway's approach for privileged
  protocols.
* Allow other clients to use this interface too, but don't let them know if the
  keyboard they created is active or is ignored. This could possibly lead to
  bad UX.
* Allow other clients to use this interface too, and use another protocol to
  manage authorizations. Basically the idea would be not to expose this
  interface, and require the client to request access to this privileged
  interface through an authorizer protocol. Someone already mentioned it, this
  is Orbital's approach [2]. This allows the compositor to spawn a dialog asking
  to the user "do you want to allow <client> to create a virtual keyboard?".

What do you think?

An event on the keyboard to signal when the keyboard is no longer active
(ie. when the compositor decides to destroy it) might still be useful, maybe.

Everything else looks good to me. Thanks for your work!

Simon

[1]: http://ppaalanen.blogspot.com/2014/07/wayland-protocol-design-object-lifespan.html
[2]: https://github.com/giucam/orbital/blob/master/protocol/orbital-authorizer.xml

> Cheers,
> Dorota Czaplejewicz
>
>  Makefile.am                                        |   1 +
>  unstable/virtual-keyboard/README                   |   2 +
>  .../virtual-keyboard-unstable-v1.xml               | 154 +++++++++++++++++++++
>  3 files changed, 157 insertions(+)
>  create mode 100644 unstable/virtual-keyboard/README
>  create mode 100644 unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
>
> diff --git a/Makefile.am b/Makefile.am
> index 1aa13cf..17cedc1 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -19,6 +19,7 @@ unstable_protocols =								\
>  	unstable/keyboard-shortcuts-inhibit/keyboard-shortcuts-inhibit-unstable-v1.xml \
>  	unstable/xdg-output/xdg-output-unstable-v1.xml				\
>  	unstable/input-timestamps/input-timestamps-unstable-v1.xml	\
> +	unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml	\
>  	$(NULL)
>
>  stable_protocols =								\
> diff --git a/unstable/virtual-keyboard/README b/unstable/virtual-keyboard/README
> new file mode 100644
> index 0000000..a2c646d
> --- /dev/null
> +++ b/unstable/virtual-keyboard/README
> @@ -0,0 +1,2 @@
> +Virtual keyboard protocol
> +
> diff --git a/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml b/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
> new file mode 100644
> index 0000000..173f3bc
> --- /dev/null
> +++ b/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
> @@ -0,0 +1,154 @@
> +<?xml version="1.0" encoding="UTF-8"?>
> +<protocol name="virtual_keyboard_unstable_v1">
> +  <copyright>
> +    Copyright © 2008-2011  Kristian Høgsberg
> +    Copyright © 2010-2013  Intel Corporation
> +    Copyright © 2012-2013  Collabora, Ltd.
> +    Copyright © 2018       Purism SPC
> +
> +    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>
> +
> +  <interface name="zwp_virtual_keyboard_v1" version="1">
> +    <description summary="virtual keyboard">
> +      The virtual keyboard provides an application with requests which emulate
> +      the behaviour of a physical keyboard.
> +
> +      This interface can be used by clients on its own to provide raw input
> +      events, or it can accompany the input method protocol.
> +    </description>
> +
> +    <request name="keymap">
> +      <description summary="keyboard mapping">
> +        Provide a file descriptor to the compositor which can be memory-mapped
> +        to provide a keyboard mapping description.
> +      </description>
> +      <arg name="format" type="uint" enum="wl_keyboard.keymap_format"
> +        summary="keymap format"/>
> +      <arg name="fd" type="fd" summary="keymap file descriptor"/>
> +      <arg name="size" type="uint" summary="keymap size, in bytes"/>
> +    </request>
> +
> +    <enum name="error">
> +      <entry name="no_keymap" value="0" summary="No keymap was set"/>
> +    </enum>
> +
> +    <request name="key">
> +      <description summary="key event">
> +        A key was pressed or released.
> +        The time argument is a timestamp with millisecond granularity, with an
> +        undefined base. All requests regarding a single object must share the
> +        same clock.
> +
> +        Keymap must be set before issuing this request.
> +      </description>
> +      <arg name="time" type="uint"
> +        summary="timestamp with millisecond granularity"/>
> +      <arg name="key" type="uint" summary="key that produced the event"/>
> +      <arg name="state" type="uint" enum="wl_keyboard.key_state"
> +        summary="physical state of the key"/>
> +    </request>
> +
> +    <request name="modifiers">
> +      <description summary="modifier and group state">
> +        Notifies the compositor that the modifier and/or group state has
> +        changed, and it should update state.
> +
> +        Keymap must be set before issuing this request.
> +      </description>
> +      <arg name="mods_depressed" type="uint" summary="depressed modifiers"/>
> +      <arg name="mods_latched" type="uint" summary="latched modifiers"/>
> +      <arg name="mods_locked" type="uint" summary="locked modifiers"/>
> +      <arg name="group" type="uint" summary="keyboard layout"/>
> +    </request>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy the virtual keyboard object"/>
> +    </request>
> +  </interface>
> +
> +  <interface name="zwp_virtual_keyboard_manager_v1" version="1">
> +    <description summary="virtual keyboard manager">
> +      A virtual keyboard manager allows an application to provide keyboard
> +      input events as if they came from a physical keyboard.
> +
> +      If the compositor enables a keyboard to perform arbitrary actions, it
> +      should prevent untrusted clients from using this interface.
> +    </description>
> +
> +    <request name="create_virtual_keyboard">
> +      <description summary="create a new virtual keyboard">
> +        Asks to create a new virtual keyboard associated to a seat.
> +
> +        If the client is found unauthorized to create a virtual keyboard, then
> +        it must receive the virtual_keyboard_create_failed event, with the
> +        reason field set to unauthorized.
> +
> +        Otherwise, the client must receive the virtual_keyboard_created event.
> +
> +        Serial numbers used in consecutive create_virtual_keyboard requests
> +        must form a sequence without repetitions.
> +      </description>
> +      <arg name="seat" type="object" interface="wl_seat"/>
> +      <arg name="serial" type="uint"/>
> +    </request>
> +
> +    <enum name="virtual_keyboard_create_failed_reason">
> +      <entry name="unauthorized" value="0"
> +        summary="client not authorized to use the request"/>
> +    </enum>
> +
> +    <event name="virtual_keyboard_create_failed">
> +      <description summary="creation of a new virtual keyboard failed">
> +        Indicates that a new virtual keyboard instance could not be created.
> +
> +        Reason contains the reason why creation did not succeed.
> +
> +        Serial is the serial number of the corresponding
> +        create_virtual_keyboard request.
> +      </description>
> +      <arg name="reason" type="uint"
> +        enum="virtual_keyboard_create_failed_reason"
> +        summary="indicate the reason for this failure"/>
> +      <arg name="serial" type="uint" description="serial number of request"/>
> +    </event>
> +
> +    <event name="virtual_keyboard_created">
> +      <description summary="successfully created a new virtual keyboard">
> +        Provides the client with the zwp_virtual_keyboard_v1 which was
> +        previously requested using create_virtual_keyboard.
> +
> +        Serial is the serial number of the corresponding
> +        create_virtual_keyboard request.
> +      </description>
> +      <arg name="id" type="new_id" interface="zwp_virtual_keyboard_v1"
> +        summary="the id of the newly created virtual keyboard"/>
> +      <arg name="serial" type="uint" description="serial number of request"/>
> +    </event>
> +
> +    <request name="destroy" type="destructor">
> +      <description summary="destroy the virtual keyboard manager object">
> +        Destroys the virtual keyboard manager.
> +
> +        Existing zwp_virtual_keyboard_v1 objects remain valid.
> +      </description>
> +    </request>
> +  </interface>
> +</protocol>
> --
> 2.14.4
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
On Thu, 12 Jul 2018 18:15:32 -0400
Simon Ser <contact@emersion.fr> wrote:

> > Provides the ability to emulate keyboards by applications. Complementary to
> > input-method protocol.
> >
> > The interface is a mirror copy of wl_keyboard, with removed serials, and added
> > seat binding.
> >
> > ---
> >
> > Hello,
> >
> > thank you for giving me a lot of useful feedback in the last round. I applied
> > the suggestions and this is the new incarnation of the protocol. Changes to v3,
> > in no particular order:
> >
> > - added clarification to the destructor namespaced enum references made virtual
> > - keyboard creation non-fatal
> >
> > The responses about keeping a virtual keyboard on a separate connection and
> > about using protocol errors as part of the authentication mechanism were all in
> > agreement, and they made sense to me too. As a result, there is a whole new
> > mechanism for that.
> >
> > The client now requests a creation of a virtual keyboard, but does not
> > immediately get a valid instance. It must wait for one of two events in
> > virtual_keyboard_manager: `virtual_keyboard_create_failed` for a failure or
> > `virtual_keyboard_created` for success. I deliberately kept them inside the
> > manager instead of virtual_keyboard, since I don't like the idea of creating an
> > object and immediately sending a message that it's useless (in case of failure).
> >
> > I also deliberately kept the names long in order not to forget what's being
> > manipulated.
> >
> > The serial number is there to match up requests on different seats with
> > failures. I imagine that a compositor might spam the user(s) with a number of
> > permission requests for different seats, and the user would deal with them in a
> > random order. Serials prevent the answers from getting mixed up.
> >
> > I am not happy only with one aspect of this: the amount of time between create
> > request and response is indefinite, opening a loophole for the compositor never
> > to reply. I decided that it doesn't warrant a protocol error however.
> >
> > As usual, feedback is welcome.  
> 
> Hi,
> 
> Sorry for the delay.
> 
> I'm not sure I like this new design.
> 
> First, this is using server-side created resources (the a `new_id` argument in
> an event). One should be really careful when doing so, see Pekka's post on the
> matter [1].
> 
Thanks, this was an insightful read. If not in this protocol, this kept me from making mistakes in my input-method revision.

> Then, this serial mechanism makes me uneasy. A more idiomatic approach would be
> to use a proper object for this.
> 
Do you mean an object defining that an attempt was made, a sort of "creation_attempt" which would later receive the result of the attempt?

> Finally, I'm not even sure this security mechanism belongs here. I think adding
> this mechanism to potentially all privileged protocols will result in duplicated
> code and "pollutes" protocols. Here are some other solutions:
> 
> * Have one keyboard daemon they spawn themselves. In this case they can only
>   expose this interface on the daemon's display. This is e.g. weston's approach
>   for its private protocols, and will be sway's approach for privileged
>   protocols.
> * Allow other clients to use this interface too, but don't let them know if the
>   keyboard they created is active or is ignored. This could possibly lead to
>   bad UX.
> * Allow other clients to use this interface too, and use another protocol to
>   manage authorizations. Basically the idea would be not to expose this
>   interface, and require the client to request access to this privileged
>   interface through an authorizer protocol. Someone already mentioned it, this
>   is Orbital's approach [2]. This allows the compositor to spawn a dialog asking
>   to the user "do you want to allow <client> to create a virtual keyboard?".
> 
> What do you think?
> 
I'm in favor of option 3, which will be useful for other protocols as well, which are not necessarily in position to be kept in a permanent privileged process, e.g. screen recording.

It would also make me happier as a protocol author, because the protocol becomes more compact and easier to maintain without explicit managing.

> An event on the keyboard to signal when the keyboard is no longer active
> (ie. when the compositor decides to destroy it) might still be useful, maybe.
> 
After chatting out of band (thanks Simon for the clarification!), we painted an image where the compositor doesn't want the keyboard to exist any more, e.g. because the user revoked permission, or because it was turned off by the user.

If it was only about permissions, it would seem like a good idea for the authorization protocol to handle disabling the virtual keyboard. Unfortunately, it seems to be only possible to render the global inert. That gets complicated because the global is a factory and its children need to be implicitly inert as well.

There are other protocols which implement an event that notifies the client that an object is inert, e.g. zwp_linux_buffer_params_v1.{created,failed} or zwp_confined_pointer_v1.unconfined. Some objects that sound like they should be force-destroyable don't have this, e.g. zxdg_toplevel_v6. It's not clear to me what the separation is and where virtual-keyboard falls.

I won't be adding such an event now, but I'm happy to get educated and then do the right thing.

> Everything else looks good to me. Thanks for your work!

Thank you for reviewing!

--Dorota


> 
> Simon
> 
> [1]: http://ppaalanen.blogspot.com/2014/07/wayland-protocol-design-object-lifespan.html
> [2]: https://github.com/giucam/orbital/blob/master/protocol/orbital-authorizer.xml
> 


> > Cheers,
> > Dorota Czaplejewicz
> >
> >  Makefile.am                                        |   1 +
> >  unstable/virtual-keyboard/README                   |   2 +
> >  .../virtual-keyboard-unstable-v1.xml               | 154 +++++++++++++++++++++
> >  3 files changed, 157 insertions(+)
> >  create mode 100644 unstable/virtual-keyboard/README
> >  create mode 100644 unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 1aa13cf..17cedc1 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -19,6 +19,7 @@ unstable_protocols =								\
> >  	unstable/keyboard-shortcuts-inhibit/keyboard-shortcuts-inhibit-unstable-v1.xml \
> >  	unstable/xdg-output/xdg-output-unstable-v1.xml				\
> >  	unstable/input-timestamps/input-timestamps-unstable-v1.xml	\
> > +	unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml	\
> >  	$(NULL)
> >
> >  stable_protocols =								\
> > diff --git a/unstable/virtual-keyboard/README b/unstable/virtual-keyboard/README
> > new file mode 100644
> > index 0000000..a2c646d
> > --- /dev/null
> > +++ b/unstable/virtual-keyboard/README
> > @@ -0,0 +1,2 @@
> > +Virtual keyboard protocol
> > +
> > diff --git a/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml b/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
> > new file mode 100644
> > index 0000000..173f3bc
> > --- /dev/null
> > +++ b/unstable/virtual-keyboard/virtual-keyboard-unstable-v1.xml
> > @@ -0,0 +1,154 @@
> > +<?xml version="1.0" encoding="UTF-8"?>
> > +<protocol name="virtual_keyboard_unstable_v1">
> > +  <copyright>
> > +    Copyright © 2008-2011  Kristian Høgsberg
> > +    Copyright © 2010-2013  Intel Corporation
> > +    Copyright © 2012-2013  Collabora, Ltd.
> > +    Copyright © 2018       Purism SPC
> > +
> > +    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>
> > +
> > +  <interface name="zwp_virtual_keyboard_v1" version="1">
> > +    <description summary="virtual keyboard">
> > +      The virtual keyboard provides an application with requests which emulate
> > +      the behaviour of a physical keyboard.
> > +
> > +      This interface can be used by clients on its own to provide raw input
> > +      events, or it can accompany the input method protocol.
> > +    </description>
> > +
> > +    <request name="keymap">
> > +      <description summary="keyboard mapping">
> > +        Provide a file descriptor to the compositor which can be memory-mapped
> > +        to provide a keyboard mapping description.
> > +      </description>
> > +      <arg name="format" type="uint" enum="wl_keyboard.keymap_format"
> > +        summary="keymap format"/>
> > +      <arg name="fd" type="fd" summary="keymap file descriptor"/>
> > +      <arg name="size" type="uint" summary="keymap size, in bytes"/>
> > +    </request>
> > +
> > +    <enum name="error">
> > +      <entry name="no_keymap" value="0" summary="No keymap was set"/>
> > +    </enum>
> > +
> > +    <request name="key">
> > +      <description summary="key event">
> > +        A key was pressed or released.
> > +        The time argument is a timestamp with millisecond granularity, with an
> > +        undefined base. All requests regarding a single object must share the
> > +        same clock.
> > +
> > +        Keymap must be set before issuing this request.
> > +      </description>
> > +      <arg name="time" type="uint"
> > +        summary="timestamp with millisecond granularity"/>
> > +      <arg name="key" type="uint" summary="key that produced the event"/>
> > +      <arg name="state" type="uint" enum="wl_keyboard.key_state"
> > +        summary="physical state of the key"/>
> > +    </request>
> > +
> > +    <request name="modifiers">
> > +      <description summary="modifier and group state">
> > +        Notifies the compositor that the modifier and/or group state has
> > +        changed, and it should update state.
> > +
> > +        Keymap must be set before issuing this request.
> > +      </description>
> > +      <arg name="mods_depressed" type="uint" summary="depressed modifiers"/>
> > +      <arg name="mods_latched" type="uint" summary="latched modifiers"/>
> > +      <arg name="mods_locked" type="uint" summary="locked modifiers"/>
> > +      <arg name="group" type="uint" summary="keyboard layout"/>
> > +    </request>
> > +
> > +    <request name="destroy" type="destructor">
> > +      <description summary="destroy the virtual keyboard object"/>
> > +    </request>
> > +  </interface>
> > +
> > +  <interface name="zwp_virtual_keyboard_manager_v1" version="1">
> > +    <description summary="virtual keyboard manager">
> > +      A virtual keyboard manager allows an application to provide keyboard
> > +      input events as if they came from a physical keyboard.
> > +
> > +      If the compositor enables a keyboard to perform arbitrary actions, it
> > +      should prevent untrusted clients from using this interface.
> > +    </description>
> > +
> > +    <request name="create_virtual_keyboard">
> > +      <description summary="create a new virtual keyboard">
> > +        Asks to create a new virtual keyboard associated to a seat.
> > +
> > +        If the client is found unauthorized to create a virtual keyboard, then
> > +        it must receive the virtual_keyboard_create_failed event, with the
> > +        reason field set to unauthorized.
> > +
> > +        Otherwise, the client must receive the virtual_keyboard_created event.
> > +
> > +        Serial numbers used in consecutive create_virtual_keyboard requests
> > +        must form a sequence without repetitions.
> > +      </description>
> > +      <arg name="seat" type="object" interface="wl_seat"/>
> > +      <arg name="serial" type="uint"/>
> > +    </request>
> > +
> > +    <enum name="virtual_keyboard_create_failed_reason">
> > +      <entry name="unauthorized" value="0"
> > +        summary="client not authorized to use the request"/>
> > +    </enum>
> > +
> > +    <event name="virtual_keyboard_create_failed">
> > +      <description summary="creation of a new virtual keyboard failed">
> > +        Indicates that a new virtual keyboard instance could not be created.
> > +
> > +        Reason contains the reason why creation did not succeed.
> > +
> > +        Serial is the serial number of the corresponding
> > +        create_virtual_keyboard request.
> > +      </description>
> > +      <arg name="reason" type="uint"
> > +        enum="virtual_keyboard_create_failed_reason"
> > +        summary="indicate the reason for this failure"/>
> > +      <arg name="serial" type="uint" description="serial number of request"/>
> > +    </event>
> > +
> > +    <event name="virtual_keyboard_created">
> > +      <description summary="successfully created a new virtual keyboard">
> > +        Provides the client with the zwp_virtual_keyboard_v1 which was
> > +        previously requested using create_virtual_keyboard.
> > +
> > +        Serial is the serial number of the corresponding
> > +        create_virtual_keyboard request.
> > +      </description>
> > +      <arg name="id" type="new_id" interface="zwp_virtual_keyboard_v1"
> > +        summary="the id of the newly created virtual keyboard"/>
> > +      <arg name="serial" type="uint" description="serial number of request"/>
> > +    </event>
> > +
> > +    <request name="destroy" type="destructor">
> > +      <description summary="destroy the virtual keyboard manager object">
> > +        Destroys the virtual keyboard manager.
> > +
> > +        Existing zwp_virtual_keyboard_v1 objects remain valid.
> > +      </description>
> > +    </request>
> > +  </interface>
> > +</protocol>
> > --
> > 2.14.4
> >
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel  
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
On Tue, 24 Jul 2018 15:10:29 +0200
Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> wrote:

> On Thu, 12 Jul 2018 18:15:32 -0400
> Simon Ser <contact@emersion.fr> wrote:
> 
> > Hi,
> > 
> > Sorry for the delay.
> > 
> > I'm not sure I like this new design.
> > 
> > Finally, I'm not even sure this security mechanism belongs here. I think adding
> > this mechanism to potentially all privileged protocols will result in duplicated
> > code and "pollutes" protocols. Here are some other solutions:
> > 

--- snip ---

> > * Allow other clients to use this interface too, and use another protocol to
> >   manage authorizations. Basically the idea would be not to expose this
> >   interface, and require the client to request access to this privileged
> >   interface through an authorizer protocol. Someone already mentioned it, this
> >   is Orbital's approach [2]. This allows the compositor to spawn a dialog asking
> >   to the user "do you want to allow <client> to create a virtual keyboard?".
> > 
> > What do you think?
> >   
> I'm in favor of option 3, which will be useful for other protocols as well, which are not necessarily in position to be kept in a permanent privileged process, e.g. screen recording.
> 
> It would also make me happier as a protocol author, because the protocol becomes more compact and easier to maintain without explicit managing.
> 

Thinking a bit more about the topic, this patch with the authentication features removed would end up exactly the same as patch v3. Patch v3 is already implemented in wlroots too.

Hereby I withdraw this update. Please refer to 20180622152045.10563-1-dorota.czaplejewicz@puri.sm which can be read here: https://lists.freedesktop.org/archives/wayland-devel/2018-June/038629.html

Regards,
Dorota Czaplejewicz
On July 29, 2018 6:55 PM, Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> wrote:
> On Tue, 24 Jul 2018 15:10:29 +0200
> Dorota Czaplejewicz <dorota.czaplejewicz@puri.sm> wrote:
>
> > On Thu, 12 Jul 2018 18:15:32 -0400
> > Simon Ser <contact@emersion.fr> wrote:
> >
> > > Hi,
> > >
> > > Sorry for the delay.
> > >
> > > I'm not sure I like this new design.
> > >
> > > Finally, I'm not even sure this security mechanism belongs here. I think adding
> > > this mechanism to potentially all privileged protocols will result in duplicated
> > > code and "pollutes" protocols. Here are some other solutions:
> > >
>
> --- snip ---
>
> > > * Allow other clients to use this interface too, and use another protocol to
> > >   manage authorizations. Basically the idea would be not to expose this
> > >   interface, and require the client to request access to this privileged
> > >   interface through an authorizer protocol. Someone already mentioned it, this
> > >   is Orbital's approach [2]. This allows the compositor to spawn a dialog asking
> > >   to the user "do you want to allow <client> to create a virtual keyboard?".
> > >
> > > What do you think?
> > >
> > I'm in favor of option 3, which will be useful for other protocols as well, which are not necessarily in position to be kept in a permanent privileged process, e.g. screen recording.
> >
> > It would also make me happier as a protocol author, because the protocol becomes more compact and easier to maintain without explicit managing.
> >
>
> Thinking a bit more about the topic, this patch with the authentication features removed would end up exactly the same as patch v3. Patch v3 is already implemented in wlroots too.
>
> Hereby I withdraw this update. Please refer to 20180622152045.10563-1-dorota.czaplejewicz@puri.sm which can be read here: https://lists.freedesktop.org/archives/wayland-devel/2018-June/038629.html

Hi,

While the interfaces, requests and events are the same, there are some wording
changes from v3 to v4 and some enum reference fixes:

- added clarification to the destructor
- namespaced enum references

Is it possible to backport these changes to v3?

Thanks,

Simon

> Regards,
> Dorota Czaplejewicz