After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 774065 - Entry completion drop-down is misplaced on external monitor when on Wayland
Entry completion drop-down is misplaced on external monitor when on Wayland
Status: RESOLVED OBSOLETE
Product: gtk+
Classification: Platform
Component: Widget: GtkEntry
3.22.x
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
: 787771 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-11-07 16:17 UTC by Debarshi Ray
Modified: 2018-05-02 17:43 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Work-in-progress patch using gdk_window_move_to_rect instead of gtk_window_move (5.17 KB, patch)
2017-12-29 16:32 UTC, Guillaume Ayoub
accepted-commit_now Details | Review
Patch using gdk_window_move_to_rect instead of gtk_window_move (4.87 KB, patch)
2017-12-29 17:52 UTC, Guillaume Ayoub
reviewed Details | Review

Description Debarshi Ray 2016-11-07 16:17:08 UTC
If you run gtk3-demo -> Entry -> Entry Completion inside a Wayland session on an external monitor and type "total", the entry completion drop-down is placed somewhere far away from the GtkEntry. Depending on the exact layout, you might see it somewhere outside the GtkWindow or might not see it at all.

It works as expected if the GtkWindow is placed on the laptop's built-in display.

Versions:
gtk3-3.22.2-1.fc25.x86_64
mutter-3.22.1-6.fc25.x86_64

The bug is visible regardless of whether the external monitor is set as primary or not.
Comment 1 Jonas Ådahl 2016-11-10 07:10:38 UTC
IIRC, the reason for this is that we pretend to have a single monitor associated to a window, while using this to figure out how to position the window. The same way that menus, combo boxes etc were positioned before gdk_window_move_to_rect.

What needs to be done here is to port GtkEntryCompletion to use gdk_window_move_to_rect API instead of using the calculation manually.
Comment 2 Jonas Ådahl 2016-11-10 07:14:37 UTC
Moving to the GtkEntry widget since the fixing should be done there, and it most likely affects both Wayland and Mir.
Comment 3 Guillaume Ayoub 2017-12-27 10:48:32 UTC
(In reply to Jonas Ådahl from comment #2)
> Moving to the GtkEntry widget since the fixing should be done there, and it
> most likely affects both Wayland and Mir.

I'm interested in fixing this issue, I (somehow) understand the current code but I can't find any documentation about what gdk_window_move_to_rect does. Does anybody got something to read about that? Otherwise I'll try to do something like what's done in gtkmenu.c.
Comment 4 Timm Bäder 2017-12-27 17:03:22 UTC
Have you read https://blog.gtk.org/2016/07/15/future-of-relative-window-positioning/ ? It gives a more prose overview of the new API. All the subtle menu positioning/sizing problems really need someone to tackle them so working on them would be appreciated.
Comment 5 Guillaume Ayoub 2017-12-27 21:26:14 UTC
(In reply to Timm Bäder from comment #4)
> Have you read
> https://blog.gtk.org/2016/07/15/future-of-relative-window-positioning/ ? It
> gives a more prose overview of the new API. All the subtle menu
> positioning/sizing problems really need someone to tackle them so working on
> them would be appreciated.

Thank you, that's what I was looking for. I'll try to use gdk_window_move_to_rect as it's done in gtkmenu.c and post a patch if I can get anything interesting.
Comment 6 Guillaume Ayoub 2017-12-29 16:32:10 UTC
Created attachment 366081 [details] [review]
Work-in-progress patch using gdk_window_move_to_rect instead of gtk_window_move

This patch replaces the gtk_window_move call with gdk_window_move_to_rect. It's a patch for the gtk-3-22 branch.

**It's probably an awful patch because I write C less that once a year. Moreover, I don't know GTK+ internal stuff at all.** But at least it's a start :).

The patch lets GDK find the right position to display the entry completion popup, instead of letting the GTK entry find the position itself. It fixes the bug for Epiphany, the gtk-demo app and a custom testing Python app \o/.

Unfortunately, it doesn't work for Geary (where the position was broken anyway for me), it puts the popup at (0, 6) pixels from the top-left-corner of the window.

I did not test it on X.


Random thoughts:

- The popup used to be a subsurface, but subsurfaces cannot be moved using gdk_window_move_to_rect. I don't know whether it's intended or not.

- Popup width and height are managed by the completion widget as they used to be, as they "require" inner content logic (they depend on the tree-view and its filtered results). They don't depend on the available screen area anymore.

- I had to call gtk_widget_show in gtk_entry_completion_popup. I don't know why, I'm not sure it's the right place.

- gtk_widget_get_allocation is called twice on completion->priv->entry, but I don't know how to cast allocation into a GdkRectangle :/.

- The tree-view is not scrolled to the last found item when the popup doesn't fit below the entry and is put above. We could fix that by scrolling after gdk_window_move_to_rect has been called.

- I've used dirty constants when calling gdk_window_move_to_rect. Maybe we can remove the WEST part of the gravity constants, or flip them according to text direction (that's what's done in menus).

- The popup position is different from the previous implementation. For example, the popup was just below the text in the previous implementation, it's now below the entry. Just telling, I suppose some people care…
Comment 7 Guillaume Ayoub 2017-12-29 17:52:13 UTC
Created attachment 366086 [details] [review]
Patch using gdk_window_move_to_rect instead of gtk_window_move

This patch fixes the bug in Geary too. The entry now works everywhere perfectly for me. I've also removed the gtk_widget_get_allocation call.

There's nothing more I can do now, I probably need help from GTK+ devs to fix GTK+- and C-related problems, and tell what's next.

Last but not least: drawing the popup seems to be slower with gdk_window_move_to_rect than before.
Comment 8 Jan-Michael Brummer 2018-01-27 10:33:26 UTC
Thanks a lot for your work. I've tested your patch and it is at least working for me. Official review pending.
Comment 9 Michael Catanzaro 2018-01-27 18:03:42 UTC
*** Bug 787771 has been marked as a duplicate of this bug. ***
Comment 10 Jonas Ådahl 2018-01-31 06:41:29 UTC
Review of attachment 366086 [details] [review]:

Thanks a lot for this! Here is some review:

Have this been tested on both input grabbing and not input grabbing entry completion? Do we have test cases for both kinds?

Another question is how this deals with entry completion windows being moved, as move_to_rect() can't deal with moving visible windows right now. Do we have test cases for that? Do we even support that?

Lastly, for the future, please attach patches generated from "git format-patch" with commit message and author information.

::: gtk/gtkentrycompletion.c
@@ +609,3 @@
   priv->popup_window = gtk_window_new (GTK_WINDOW_POPUP);
+  /* Using subsurface prevents gdk_window_move_to_rect from working */
+  /* gtk_window_set_use_subsurface (GTK_WINDOW (priv->popup_window), TRUE); */

We should just remove this, not comment it out.
Comment 11 Guillaume Ayoub 2018-02-01 15:17:20 UTC
(In reply to Jonas Ådahl from comment #10)
> Review of attachment 366086 [details] [review] [review]:
> 
> Thanks a lot for this!

I've done my best, but it looks like an ugly patch, not a real solution. I've tried with Xorg and it's broken :/.

> Here is some review:
> 
> Have this been tested on both input grabbing and not input grabbing entry
> completion? Do we have test cases for both kinds?

No, I don't even know what you're talking about…

> Another question is how this deals with entry completion windows being
> moved, as move_to_rect() can't deal with moving visible windows right now.
> Do we have test cases for that? Do we even support that?

It's possible to move the input window when the dropdown window is open, for example by maximizing/minimizing epiphany using keyboard shortcuts. With the patch, the dropdown window doesn't move with the input window. I don't know how it was before the patch.

> Lastly, for the future, please attach patches generated from "git
> format-patch" with commit message and author information.

I'll try to remember that!

> ::: gtk/gtkentrycompletion.c
> @@ +609,3 @@
>    priv->popup_window = gtk_window_new (GTK_WINDOW_POPUP);
> +  /* Using subsurface prevents gdk_window_move_to_rect from working */
> +  /* gtk_window_set_use_subsurface (GTK_WINDOW (priv->popup_window), TRUE);
> */
> 
> We should just remove this, not comment it out.

Of course.
Comment 12 Guillaume Ayoub 2018-03-08 13:47:24 UTC
I've updated to Gtk+-3.22.28 (with Gnome libs 3.27.92), the bug is fixed for me. Applying the patch even breaks my current installation.
Comment 13 Guillaume Ayoub 2018-03-08 16:36:30 UTC
(In reply to Guillaume Ayoub from comment #12)
> I've updated to Gtk+-3.22.28 (with Gnome libs 3.27.92), the bug is fixed for
> me. Applying the patch even breaks my current installation.

Forget that, sorry for the noise. I was using Xorg instead of Wayland.

So, as said before, it's broken with the patch on Xorg and broken without the patch on Wayland.
Comment 14 GNOME Infrastructure Team 2018-05-02 17:43:54 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gtk/issues/699.