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

[Jonas Ådahl]

On Wed, Jun 08, 2016 at 11:13:09PM +0200, Benoit Gschwind wrote:
> Hello Jonas,
> 
> For this patch I'm more cautious, your proposal look very complex, and I
> don't understand why. I will try to explain my point of view with my
> current knowledge and then draft a proposal from my own analysis. To be
> fair, I did not follow the entire discussion about grab from the
> beginning, thus I could be inaccurate. If I'm wrong fell free to correct
> myself.
> 
> If I look at the previous protocol, the xdg_popup implied a grab. That
> what simple and easy to explain. This simple approach quickly become too
> simple because it does not allow to create xdg_popup without grabbing
> the mouse pointer, which is needed to make tooltips. Thus you made the
> following proposal.
> 
> Currently, your proposal is about separating xdg_popup creation from
> grabbing the mouse pointer. To do a tooltips, it's easy, the client
> create a xdg_popup as he does "a old grabbing popup". The new xdg_popup
> does not grab inputs by default, that's the purpose of this patch. Now,
> if the client want to create a popup that grab inputs, he have to create
> a popup then request the grab, until now it still simple, even if the
> client have to do it on every grabbing popup.

Not really. From a semantical point of view, the creation of the popup
and the grabbing is still atomic, as in, you can't create a popup, map
it, and then grab; you can only create a grabbing popup.

The difference is how you create the grabbing popup. Before this patch
you did:

  wl_surface = wl_compositor.create_surface()
  xdg_surface = xdg_shell.create_xdg_surface(wl_surface)
  xdg_popup = xdg_surface.get_popup(parent, seat, serial, x, y)
  wl_surface.commit()  <--- this completes the creation

Now you would do:

  wl_surface = wl_compositor.create_surface()
  xdg_surface = xdg_shell.create_xdg_surface(wl_surface)
  xdg_popup = xdg_surface.get_popup(parent, x, y)
  xdg_popup.grab(seat, serial)
  wl_surface.commit()  <--- this completes the creation

This way we get the exact same behaviour as before, while allowing us to
create a xdg_popup, that doesn't grab anything.

> 
> This look good till now, but you tried to give an explicit semantics in
> the case of nested popups or in case of multiple popups. This semantics
> look logic but is complex, and moreover you nullify your complex
> semantics by writing:
> 
> "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."

This is just the same as before. This patch doesn't really change how
anything works with nested popups. It just makes it possible to create a
popup that doesn't grab.

> 
> Which mean, if I understand correctly, the client just have to create
> one grabbing popup and all surface managed by the client get the mouse
> grabbed, in particular all nested tooltips popup I can imagine.
> 
> This instance show that the proposal is uselessly too complex.
> 
> 
> 
> Now I will discuss the grabbing more widely in the objective to draft an
> alternate proposal.
> 
> The grabbing was an issue in X11 because any client was able to grab
> mouse and keyboard easily and a very simple client can lock your screen,
> or block the lockscreen. For this reason previous xdg_popup tried to
> take care that a client cannot lock your desktop without being granted
> by the user.
> 
> If I analyses the previous protocol the choice was basically:
> 
> - (1) the client can grab the mouse only on identified valid mouse event
> or keyboard event,
> - (2) the compositor can cancel the grab at anytime he want. In
> particular, implicitly cancel the grab when the last xdg_popup is destroyed.
> 
> The first rule was enforced by forcing the client to create and map a
> xdb_popup. I do not understand this enforcement, because he ca be easily
> tricked by creating an 1x1 window hidden somewhere.
> 
> Thus following my analysis I suggest to not create complex grab
> semantics and suggest to stick to rule (1) and (2) which could be
> implemented by :
> 
> - a global grab request linked to a mouse or keyboard event.
> - a global ungrab request that the client call when the grab terminate
> - a global cancel_grab event that can be used at anytime by the compositor.
> 
> By global I mean not bound to a surface, as I showed, that does not
> solve anything. The grab request are linked to a mouse event or a
> keyboard event, imply a valid xdg_surface to be active and disallow a
> grab that is not driven by user action.

It needs to be bound to a surface, so the compositor knows what to keep
focused. It also needs to know what popups are part of a popup chain, so
that it can choose to dismiss the correct popups.

> 
> Those kind of grab can be canceled by the compositor, thus they cannot
> be used to lock the screen or the user.
> 
> Another random implementation thought, this maybe implemented as an
> object like get_grab_handler.
> 
> I think I will get the objection that in previous behavior the grab was
> automatically stopped when the xdg_popup was destroyed but in the
> current draft it's not automatic. To this I just reply that xdg_popup
> are not destroyed automatically by clients. Thus yes in both case the
> client can grab mouse pointer indefinitely if the compositor doesn't
> provide a "force cancel grab" mechanism like a bind to ESC key.

A grab is still 'keep sending input events to surface X even though its
not under the pointer/touch point'. If you don't bind a grab to a
surface X, then you don't know where to send anything.


Jonas

> 
> The draft is simple to explain and simple to implement.
> 
> Best regards.
> --
> Benoit Gschwind