! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Iconview search popup placement incorrect when using multiple monitors
Status:
RESOLVED: FIXED

Comments

Description dwu 2018-12-29 14:52:05 CET
Created attachment 8207 
Screenshot depicting the observed behavior

When performing a type-ahead find in an exo_icon_view (for example Thunar's "View as Icons" or "View as Compact List") in a dual-head setup with displays next to each other and the view being on any but the leftmost screen, the search popup is positioned not as an overlay over the Thunar window but detached on the leftmost screen.

See attached screenshot for an example. The part with the darker background on the left of the screenshot is the right border of the left screen.

The issue seems to be a result of exi_icon_view_get_screen_dimensions() only taking the primary display into account which breaks the search window positioning checks performed in exo_icon_view_search_position_func().
Comment 1 dwu 2018-12-29 14:54:16 CET
Created attachment 8208 
Experimental patch

I currently use the attached patch against master as a temporary fix. However I'm not quite sure whether I understand the idea behind the call to gtk_window_move() in exo_icon_view_key_press_event(). If the intent is to move the window offscreen (as the comment says), then I'd expect my patch to break it. However the patch seems to work fine... even if I remove the call to gtk_window_move(), there are no visible changes to the behavior.
Comment 2 alexxcons editbugs 2019-01-03 00:29:34 CET
Created attachment 8220 
another search window glitch

Thanks for your contribution !

I can confirm the bug on my two monitors, and I can confirm that your patch slightly improves the situation.

However the position of the search dialog sometimes is still wired. Sometimes the dialog seems to open even completly outside the screen.

E.g. if entering a new folder , hovering the mouse on the middle-right of the icon view and pressing CTRL + F ( See screenshot )
(Ocassionally it works wine .. than I need to reopen thunar to reproduce the bug again)

I dont know the intention in exo to use gtk_window_move ... actually I would say that the whole current idea of opening the dialog outside of the thunar window is a nasty hack.
Opening additional dialogs at fixed positions usually is a bad idea, since the application does not control the window-arrangement. Afaik only the window manager should do things like that.

IMO a better solution could be to  open the dialog in the thunar toolbar (or it could be there by default )
To be checked if other xfce applications besides thunar use the exo-icon-view.
Comment 3 dwu 2019-01-03 17:20:06 CET
Created attachment 8222 
Revised patch

Thanks for testing and your feedback.

I agree that it would probably be best to move the search popup somewhere inside the icon view itself. However I'm not quite sure about the implications on other applications using the widget so as an interim solution I had a closer look at the implementation of the search popup placement in GtkTreeView (https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-24/gtk/gtktreeview.c#L15109) which is used in Thunar's "Details" view and seems to work as expected.

Based on this and your feedback I revised my previous patch:

- The placement logic in exo_icon_view_search_position_func() now follows the one in GtkTreeView, so both exo's and GTK's view should behave the same; there was one placement issue I missed previously.
- The issue you found when initiating a search via Ctrl-F seems to be related to the yet unclear call to gtk_window_move() in exo_icon_view_key_press_event() which I replaced with a call to exo_icon_view_search_position_func() to make sure the popup is placed correctly.
- When using GTK3 gdk_monitor_get_workarea() is now used to ensure that the search window does not cover the panel. As this was introduced only in GTK3 I added an alternative implementation based on gdk_screen_get_monitor_geometry() as a fallback for GTK2. However, this is currently untested and there are likely better solutions to make it more similar to the GTK3 version.
- exo_icon_view_get_screen_width() and exo_icon_view_get_screen_height() have been removed as they were not used anymore.

It would be great if you could have a look at the patch again and let me know about any issues you find. Thanks!
Comment 4 alexxcons editbugs 2019-01-03 22:44:10 CET
Congratulations, great work, it looks like this time you nailed it!

I as well tested the GTK2 version, runs fine for me.
( Just checkout + build the thunar branch "xfce4.12" , thats gtk2 based. )

Now it would be great if you could upload your diff / your commit to some page where I could add comments to the code .. e.g. gitlab or gitlab ?  If you like I could as well do so.

Some minor things to fix before push to master regarding code-style, sorry to be a bit pedantic here :)

I think you can just drop line 2937 .. seems to have no effect, it still seems to work without it.

A different thing: Please create your patch with "git format-patch" .. this allows metainfo, like author / email and a small explanation on what you did

E.g. this will build a patch of your latest commit:
git format-patch HEAD -1
Comment 5 dwu 2019-01-04 20:48:19 CET
Thanks for the prompt response and the offer to provide feedback on the coding style. I've pushed my changes to a Github repo for commenting.

Please find the current patch with one additional modification at: https://github.com/dwu/exo/commit/ef56e88969fe65d7385f8586097a4ebfd97b4d78

After thinking about the original code a bit more I think I now understand the intention of the previously unclear call to gtk_window_move(). Please see my annotations regarding that on the commit page mentioned above.
Comment 6 dwu 2019-01-06 10:17:17 CET
Created attachment 8227 
Revised patch

After some iterations and very helpful feedback from Alex please find the revised version of the patch attached.

The implementation is now similar to GTK's TreeView (https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-24/gtk/gtktreeview.c#L6083) and seems to work correctly on Thunar built with GTK3 and GTK2 for both find-as-you-type search as well as search operations initiated via Control+f.

I've so far not tested any other applications using exo's icon view.
Comment 7 Git Bot editbugs 2019-01-08 03:42:39 CET
Daniel Wutke referenced this bugreport in commit 020aba5f78cec1b571fbe64bb6748199708b2fc1

Fix icon view search popup placement (Bug #14994)

https://git.xfce.org/xfce/exo/commit?id=020aba5f78cec1b571fbe64bb6748199708b2fc1
Comment 8 Sean Davis editbugs 2019-01-08 03:46:02 CET
Thanks for the patch and all the testing! I've applied your changes to master.

Bug #14994

Reported by:
dwu
Reported on: 2018-12-29
Last modified on: 2019-01-08

People

Assignee:
Nick Schermer
CC List:
3 users

Version

Attachments

Screenshot depicting the observed behavior (227.35 KB, image/png)
2018-12-29 14:52 CET , dwu
no flags
Experimental patch (4.00 KB, patch)
2018-12-29 14:54 CET , dwu
no flags
another search window glitch (521.33 KB, image/png)
2019-01-03 00:29 CET , alexxcons
no flags
Revised patch (5.47 KB, patch)
2019-01-03 17:20 CET , dwu
no flags
Revised patch (14.43 KB, patch)
2019-01-06 10:17 CET , dwu
no flags

Additional information