[PATCH wayland-protocols v3 6/7] xdg-shell: Make xdg_popup non-grabbing by default

Tue Jun 7 14:39:09 UTC 2016

Hi, a couple of comments below:

2016-05-26 7:32 GMT+03:00 Jonas Ådahl <jadahl at gmail.com>:
> Turn xdg_popup into plain temporary child surfaces without any grabbing
> or mapping order requirements by default.
>
> In order to create grabbing popup chains, a new request 'grab' is
> introduced which enables more or less the same semantics and
> requirements as xdg_popup previously had related to grabbing, stacking
> and mapping order.
>
> This enables using xdg_popup for creating tooltips and other user
> interface elements that does not want to take an explicit grab.
>
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> Signed-off-by: Mike Blumenkrantz <zmike at samsung.com>
> Reviewed-by: Yong Bakos <ybakos at humanoriented.com>
> ---
>  unstable/xdg-shell/xdg-shell-unstable-v6.xml | 110 +++++++++++++++++----------
>  1 file changed, 68 insertions(+), 42 deletions(-)
>
> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> index 2fdcf9b..07cbbc9 100644
> --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml
> @@ -158,16 +158,11 @@
>         This creates an xdg_popup object for the given xdg_surface and gives the
>         associated wl_surface the xdg_popup role.
>
> -       This request must be used in response to some sort of user action like a
> -       button press, key press, or touch down event.
> -
>         See the documentation of xdg_popup for more details about what an
>         xdg_popup is and how it is used.
>        </description>
>        <arg name="id" type="new_id" interface="zxdg_popup_v6"/>
>        <arg name="parent" type="object" interface="zxdg_surface_v6"/>
> -      <arg name="seat" type="object" interface="wl_seat" summary="the wl_seat of the user event"/>
> -      <arg name="serial" type="uint" summary="the serial of the user event"/>
>        <arg name="x" type="int"/>
>        <arg name="y" type="int"/>
>      </request>
> @@ -670,46 +665,30 @@
>
>    <interface name="zxdg_popup_v6" version="1">
>      <description summary="short-lived, popup surfaces for menus">
> -      A popup surface is a short-lived, temporary surface that can be
> -      used to implement menus. It takes an explicit grab on the surface
> -      that will be dismissed when the user dismisses the popup. This can
> -      be done by the user clicking outside the surface, using the keyboard,
> -      or even locking the screen through closing the lid or a timeout.
> -
> -      When the popup is dismissed, a popup_done event will be sent out,
> -      and at the same time the surface will be unmapped. The xdg_popup
> -      object is now inert and cannot be reactivated, so clients should
> -      destroy it. Explicitly destroying the xdg_popup object will also
> -      dismiss the popup and unmap the surface.
> -
> -      Clients will receive events for all their surfaces during this
> -      grab (which is an "owner-events" grab in X11 parlance). This is
> -      done so that users can navigate through submenus and other
> -      "nested" popup windows without having to dismiss the topmost
> -      popup.
> -
> -      Clients that want to dismiss the popup when another surface of
> -      their own is clicked should dismiss the popup using the destroy
> +      A popup surface is a short-lived, temporary surface. It can be used to
> +      implement for example menus, popovers, tooltips and other similar user
> +      interface concepts.
> +
> +      A popup can be made to take an explicit grab. See xdg_popup.grab for
> +      details.
> +
> +      When the popup is dismissed, a popup_done event will be sent out, and at
> +      the same time the surface will be unmapped. See the xdg_popup.popup_done
> +      event for details.
> +
> +      Explicitly destroying the xdg_popup object will also dismiss the popup and
> +      unmap the surface. Clients that want to dismiss the popup when another
> +      surface of their own is clicked should dismiss the popup using the destroy
>        request.
>
>        The parent surface must have either the xdg_toplevel or xdg_popup surface
>        role.
>
> -      Specifying an xdg_popup for the parent means that the popups are
> -      nested, with this popup now being the topmost popup. Nested
> -      popups must be destroyed in the reverse order they were created
> -      in, e.g. the only popup you are allowed to destroy at all times
> -      is the topmost one.
> -
> -      If there is an existing popup when creating a new popup, the
> -      parent must be the current topmost popup.
> -
> -      A parent surface must be mapped before the new popup is mapped.
> +      A newly created xdg_popup will be stacked on top of all previously created
> +      xdg_popup surfaces associated with the same xdg_toplevel.
>
> -      When compositors choose to dismiss a popup, they will likely
> -      dismiss every nested popup as well. When a compositor dismisses
> -      popups, it will follow the same dismissing order as required
> -      from the client.
> +      The parent of an xdg_popup must be mapped (see the xdg_surface
> +      description) before the xdg_popup itself.
>
>        The x and y arguments passed when creating the popup object specify
>        where the top left of the popup should be placed, relative to the
> @@ -718,9 +697,6 @@
>
>        The client must call wl_surface.commit on the corresponding wl_surface
>        for the xdg_popup state to take effect.
> -
> -      For a surface to be mapped by the compositor the client must have
> -      committed both the xdg_popup state and a buffer.
>      </description>
>
>      <request name="destroy" type="destructor">
> @@ -733,6 +709,56 @@
>        </description>
>      </request>
>
> +    <request name="grab">
> +      <description summary="make the popup take an explicit grab">
> +       This request makes the created popup take an explicit grab. An explicit
> +       grab will be dismissed when the user dismisses the popup, or when the
> +       client destroys the xdg_popup. This can be done by the user clicking
> +       outside the surface, using the keyboard, or even locking the screen
> +       through closing the lid or a timeout.
> +
> +       If the compositor denies the grab, the popup will be immediately
> +       dismissed.
> +
> +       This request must be used in response to some sort of user action like a
> +       button press, key press, or touch down event. The serial number of the
> +       event should be passed as 'serial'.
> +
> +       The parent of a grabbing popup must either be a xdg_toplevel surface or
> +       another xdg_popup with an explicit grab. If the parent is another
> +       xdg_popup it means that the popups are nested, with this popup now being
> +       the topmost popup.
> +
> +       Nested popups must be destroyed in the reverse order they were created
> +       in, e.g. the only popup you are allowed to destroy at all times is the
> +       topmost one.
> +
> +       When compositors choose to dismiss a popup, they may dismiss every
> +       nested grabbing popup as well. When a compositor dismisses popups, it
> +       will follow the same dismissing order as required from the client.
> +
> +       If there currently is an existing popup with an explicit grab, the
> +       parent of this popup must be that grabbing popup. The new popup will
> +       take over the grab previously held by the current topmost grabbing
> +       popup. If the topmost grabbing popup is destroyed, the grab will be
> +       returned to the parent popup, if the parent popup previously had an
> +       explicit grab.

You're saying above that if the parent is a popup it must have an explicit grab,
so saying here that the grab will return to the parent only if the parent had a
grab before is confusing.

> +
> +       If the parent is a grabbing popup which has already been dismissed, this
> +       popup will be immediately dismissed. If the parent is a popup that did
> +       not take an explicit grab, an error will be raised.

Can we clarify which error it is?

> +
> +       Clients will receive events for all their surfaces during this grab
> +       (which is an "owner-events" grab in X11 parlance). This is done so that
> +       users can navigate through submenus and other "nested" popup windows
> +       without having to dismiss the topmost popup.
> +      </description>

This doesn't explain what happens if the parent is a popup but the
serial and/or seat
passed here are different. Should it be an error? Should they be ignored?

> +
> +      <arg name="seat" type="object" interface="wl_seat"
> +          summary="the wl_seat of the user event"/>
> +      <arg name="serial" type="uint" summary="the serial of the user event"/>
> +    </request>
> +
>      <event name="popup_done">
>        <description summary="popup interaction is done">
>         The popup_done event is sent out when a popup is dismissed by the
> --
> 2.5.5
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel