On Thu, 26 Feb 2015 00:52:36 -0800 "Jasper St. Pierre" <[email protected]> wrote:
> On Thu, Feb 26, 2015 at 12:48 AM, Jonas Ådahl <[email protected]> wrote: > > > On Thu, Feb 26, 2015 at 09:59:21AM +0200, Pekka Paalanen wrote: > > > On Fri, 13 Feb 2015 14:01:58 +0800 > > > Jonas Ådahl <[email protected]> wrote: > > > > > > > From: "Jasper St. Pierre" <[email protected]> > > > > > > > > Either in destroy or get_xdg_popup. > > > > > > > > [jadahl: Verify that the new popup is the top most when mapping instead > > > > of creating. Some renaming.] > > > > --- > > > > desktop-shell/shell.c | 58 > > +++++++++++++++++++++++++++++++++++++++++++++----- > > > > protocol/xdg-shell.xml | 7 ++++++ > > > > 2 files changed, 60 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c > > > > index 4990c4d..6697222 100644 > > > > --- a/desktop-shell/shell.c > > > > +++ b/desktop-shell/shell.c > > > > @@ -3370,12 +3370,40 @@ touch_popup_grab_end(struct weston_touch > > *touch) > > > > } > > > > } > > > > > > > > -static void > > > > +static struct shell_surface * > > > > +get_top_xdg_popup(struct shell_seat *shseat) > > > > +{ > > > > + struct shell_surface *shsurf; > > > > + > > > > + if (wl_list_empty(&shseat->popup_grab.surfaces_list)) { > > > > + return NULL; > > > > + } else { > > > > + shsurf = > > container_of(shseat->popup_grab.surfaces_list.next, > > > > + struct shell_surface, > > > > + popup.grab_link); > > > > + assert(shell_surface_is_xdg_popup(shsurf)); > > > > > > Hi, > > > > > > are you sure this assert() does not trigger when a client is using > > > wl_shell popups? > > > > > > It looks like if a client tried to open a wl_shell popup while one is > > > already open, this would explode. > > > > > > map() -> shell_map_popup() -> add_popup_grab() -> get_top_xdg_popup() > > > > > > Am I missing something? > > > > No you are right. If a client mixes both wl_shell and xdg_shell, we'd be > > in trouble. > > > > If a client simply maps two wl_shell popups, then we're also in trouble, > since get_top_xdg_popup is called unconditionally by add_popup_grab, which > is also called from set_popup. Yes, this is exactly the case I was worrying about. I don't think we need to make sure mixing wl_shell and xdg_shell works. IMHO, if a client makes a top-level via wl_shell or xdg_shell, then it better use the same shell interfaces for everything that touches that top-level. But I wouldn't go as far as enforcing that in the code. I do care about wl_shell working alone. > > > > static void > > > > @@ -3417,6 +3447,15 @@ remove_popup_grab(struct shell_surface *shsurf) > > > > { > > > > struct shell_seat *shseat = shsurf->popup.shseat; > > > > > > > > + if (shell_surface_is_xdg_popup(shsurf) && > > > > + get_top_xdg_popup(shseat) != shsurf) { > > > > + wl_resource_post_error(shsurf->resource, > > > > + > > XDG_POPUP_ERROR_NOT_THE_TOPMOST_POPUP, > > > > + "xdg_popup was destroyed while it > > was " > > > > + "not the topmost popup."); > > > > > > Hmm, I'm confused. What does "top-most popup" mean? > > > > > > If there is a chain of popups because of sub-menus: > > > > > > surface -> popup A -> popup B > > > > > > Then which popup is the top-most? The root of the popup hierarchy or > > > the most recently created popup? > > > > > > It seems add_popup_grab() pushes the new popup to the head of the > > > popup_grab.surfaces_list, so top-most means the most recently mapped > > > popup, right? So that's popup B in the example. > > > > "Topmost" means the one that's on top of all the others, so, popup B. If > you have a better word, feel free to say so! It was the best I could think > of. I'm fine with the word, as long as there is a doc that explains it somewhere. It does describe the situation correctly from z-order perspective. > > > You can ignore this question if the documentation rewrite explains > > > "top-most". > > > > --- a/protocol/xdg-shell.xml > > > > +++ b/protocol/xdg-shell.xml > > > > @@ -398,6 +398,13 @@ > > > > xdg_popup surfaces are always transient for another surface. > > > > </description> > > > > > > > > + <enum name="error"> > > > > + <description summary="xdg_popup error values"> > > > > + These errors can be emitted in response to xdg_popup requests. > > > > + </description> > > > > + <entry name="not_the_topmost_popup" value="0" summary="The > > client tried to destroy a non-toplevel popup"/> > > > > > > What is a toplevel popup? Did you mean top-most? > > > > Whoops. The "not_the_topmost" should probably clue you in that, yes, I did > :) Cool. Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
