GNOME Bugzilla – Bug 776225
[wayland] dropdown placed somewhere in the screen
Last modified: 2017-01-17 08:48:52 UTC
Created attachment 342134 [details] gtk+ menu position bug (image) Dropdown sometimes placed somewhere in the screen. Please see the attached screenshot from gnome-recipes.
I cannot reproduce this using current gtk-3-22 branch with current recipes. Can you give more details your setup and on how to reproduce? In comment 0 you mention it happens sometimes, does it mean it's random?
Quick update, to reproduce, one needs to have combo inside a popover, which is not so common but can be found in recipes at commit c127b94. The problem is that gtk_menu_position() will position the menu relative to the widget using the widget's window, but when it's inside a popover, the popover is itself a subsurface, and the popover relative position is not taken into account. In other words, the menu is placed relative to the widget location but applied to the toplevel window, not the actual subsurface when using a popover. Not sure how to fix this though...
It's even worse than the combo box, I just realised that even the context popup menu on text fields is misplaced as well, it's located according to the toplevel coordinates instead of the subsurface coordinates (it doesn't show up at the pointer location). I had /some/ patch for the combo issue but it makes the text field context menu even more wrong. Meh.
Created attachment 342924 [details] [review] wayland: Handle subsurface as popup parent When a subsurface is used as a parent of a popup, GDK needs to traverse up to the transient-for as the next parent, to properly find the parent used by the popup positioner. This is because the parent of a popup must always either be an xdg_popup or an xdg_surface, but traversing the "parent" (in GDK terms) upwards from a subsurface will end up on the fake root window before we hit the actual parent (in Wayland terms). --- I have only tested this with the widget factory (I found a place where a popup is a child of a popover on Page 3, by pressing "Open" then opening a context menu in the text entry. This patch fixes that case, and I suspect other misplaced popups that had to pass by a subsurface should be fixed as well, if there is no other issue involved as well.
Created attachment 342930 [details] [review] [PATCH gtk-3-22] wayland: translate subsurfaces coordinates When mapping a popup window from a popover, which under Wayland is translated as a subsurface with the root window as parent, the menu appears misplaced, located according to the toplevel location instead of the subsurface location. When the parent of a popup window is a subsurface, translate the coordinates back to the toplevel coordinates so that move_to_rect() correctly locates the popup in the subsurface. -- Note, it's *meh* but it appears to do the job... i.e., I'm not proud of this patch but failed to find a better solution without a lot of potential impact elsewhere...
Review of attachment 342930 [details] [review]: Self rejecting my own patch, actually, I prefer Jonas' patch.
Review of attachment 342924 [details] [review]: Yeah, I ended up with the same conclusions using a different patch, but I prefer yours - Tested, works fine with the original test case.
Review of attachment 342924 [details] [review]: Couldn't we save one of the two calls to get_effective_parents() by using a temp var instead?
Review of attachment 342924 [details] [review]: Yea, and should probably look at impl->display_server.wl_subsurface rather than parent->window_type to be more reliable. Should maybe un-obfuscate that while loop while at it.
Created attachment 342933 [details] [review] wayland: Handle subsurface as popup parent When a subsurface is used as a parent of a popup, GDK needs to traverse up to the transient-for as the next parent, to properly find the parent used by the popup positioner. This is because the parent of a popup must always either be an xdg_popup or an xdg_surface, but traversing the "parent" (in GDK terms) upwards from a subsurface will end up on the fake root window before we hit the actual parent (in Wayland terms). ---- This one untangles the while loop and uses a temp variable for the effectev parent. Got slightly messier, but also maybe easier to read; not sure which one I prefer.
Review of attachment 342933 [details] [review]: I like this one better, but leave it to you to chose - Either way, both patches works.
Comment on attachment 342933 [details] [review] wayland: Handle subsurface as popup parent Attachment 342933 [details] pushed as 5bae71f - wayland: Handle subsurface as popup parent
Leaving open until a similar patch has landed on master. Can't do that quite yet; I crash on mapping a popover on master now (something about a damage region being NULL), so I can't test.
Created attachment 343556 [details] [review] [PATCH master] wayland: Handle subsurface as popup parent Forward port of attachment 342933 [details] [review] (git 5bae71f) to master. gdk_window_get_effective_parent() has been removed along with the removal of offscreen windows (commit 70935f) so this patch simply use gdk_window_get_parent() instead of gdk_window_get_effective_parent() in gtk-3-22 Seems to work, popovers are currently misplaced and font rendering seems broken on master but this is unrelated to this patch.
Review of attachment 343556 [details] [review]: Makes sense to me.
Comment on attachment 343556 [details] [review] [PATCH master] wayland: Handle subsurface as popup parent attachment 343556 [details] [review] pushed to git master as commit 7ca6d75 - wayland: Handle subsurface as popup parent