I have no idea how that can happen. Do you have any plugin?

I cannot reproduce, either.

Download and run live image [1] and see for yourselves, no installation needed.

However, that extra blank item is displayed only for some directories. I see it on all default dirs in $HOME, but in the root directory, I see it only on /tmp and nothing else. Really weird.

[1] http://dl.fedoraproject.org/pub/alt/stage/25_Beta-1.1/Workstation/x86_64/iso/Fedora-Workstation-Live-x86_64-25_Beta-1.1.iso

I'm running f25 as my main workstation and I cannot reproduce. But I use X11 due to a drivers bug with wayland.

So it might be Wayland related? Can you try with X11?

Confirmed, I only see this on Wayland, not on X11.

Also, this happens only in context menu of folders, not files.

Carlos, I think this also from your clipboard rework. :)
It’s the “paste-into” action, specifically.

Copying something into the clipboard will show you the extra menu item that gets disabled after the menu is drawn.

Ah, no, it is 1f57c5b.

Looks like a problem in GTK+, as it works on X.
The breaking change here was using gtk_menu_popup_at_pointer() instead of gtk_menu_popup().

One part common with a downstream bug for evolution:
https://bugzilla.redhat.com/show_bug.cgi?id=1394993

The problem there was that the menu items are updated only after the menu is shown. It doesn't matter which function is used to show the popup menu, the problem is that the menu content changes after it is shown. While X11 resizes whole menu container, the Wayland does not.

It's not only about hiding items, but also about showing them, in which case the menu receives "scrolling" arrows up and down, even the screen has enough estate to show all the items (and it looks really weird when you have a menu of 4 items, then you add there one item and a separator and the menu becomes scrolling with showing effectively only one and a half line).

There are side-effects of the menu resize though. Imagine you show a menu with 20 items. The menu is placed "naturally" beside the mouse pointer. If you then hide 15 items, then (currently only on X11) the menu resizes in a way that it keeps its left-top corner at the same place, which (most likely) makes the menu misplaced. Having items ready before the menu is shown is all fine (of course).

(In reply to Ernestas Kulik from comment #10)
> Looks like a problem in GTK+, as it works on X.
> The breaking change here was using gtk_menu_popup_at_pointer() instead of
> gtk_menu_popup().

I don't see this here, reverting 1f57c5b makes no difference.

However, making update_context_menus_if_pending() a no-op fixes the issue, the menus are correctly size without update_context_menus_if_pending() in src/nautilus-files-view.c

Sorry, I mean, doesn't seem so much how the menu popup is called but rather updating its content which changes its size.

(In reply to Olivier Fourdan from comment #13)
> Sorry, I mean, doesn't seem so much how the menu popup is called but rather
> updating its content which changes its size.

Yes, you’re right. I’ve only now started noticing that there’s a race condition. Reverting the commit really doesn’t make a difference and it’s random how the menu appears.

However, there haven’t really been reports of this up until that switch from the deprecated API, hence my finger pointing.

(In reply to Ernestas Kulik from comment #14)
> [...]
> However, there haven’t really been reports of this up until that switch from
> the deprecated API, hence my finger pointing.

No worries, I'm mostly thinking out loud, in case it rings a bell with others :)

Anyway, Looking at gtk_menu_realize(), we can see that the gtk_widget_get_allocation() is wrong on both X11 and Wayland backend, it's too large initally.

Event though a later gtk_menu_size_allocate() is supposed to make it right, on Wayland it's the size set in create_dynamic_positioner() (as we are using xdg_posiionner now for this) that takes precedence, i.e. the wrong one.

That explains why the bug shows on Wayland and not on X11.

*** Bug 775035 has been marked as a duplicate of this bug. ***

Created attachment 340744 [details]
Simple reproducer

(In reply to Ernestas Kulik from comment #14)
> Yes, you’re right. I’ve only now started noticing that there’s a race
> condition [...]

You're spot on here, it *is* as race condition as demonstrated by this simple reproducer.

For this to happen, you need to schedule an update of the content immediately *after* showing the menu (showing the menu means mapping the window which, for an xdg_popup under Wayland, means creating the xdg_positionner with the initial given size).

Scheduling the menu content update synchronously or much later (with a larger timeout) would avoid the the race condition.

I am somehow tempted to think it's an application issue, I don't think it's wise or advisable for the application to update the menu content in a timeout callback, is it?

(In reply to Olivier Fourdan from comment #18)
> I am somehow tempted to think it's an application issue, I don't think it's
> wise or advisable for the application to update the menu content in a
> timeout callback, is it?

I am tempted to think that as well now. Reassigning back to Nautilus.

(In reply to Olivier Fourdan from comment #18)
> I am somehow tempted to think it's an application issue, I don't think it's
> wise or advisable for the application to update the menu content in a
> timeout callback, is it?

I agree, though there are proper use-cases where you've no other option than run the checks asynchronously. For the evolution those are the copy/paste actions, which are visible only if there is anything to copy and more importantly for the later when there is anything usable to paste. That means that the update_actions() before the popup menu is shown runs an asynchronous function to examine the clipboard content, which finishes only after the pop up menu is shown. While it sounds simple here (just wait with the menu after the clipboard is examined), the reality is that the related code is buried far from the place where the popup menu is shown, hidden deep in signal emissions and so on. This is far from an ideal world with only synchronous calls. (Once the gtk+ will add synchronous clipboard API we are all fine, of course. ;) )

(In reply to Olivier Fourdan from comment #15)
> Event though a later gtk_menu_size_allocate() is supposed to make it right,
> on Wayland it's the size set in create_dynamic_positioner() (as we are using
> xdg_posiionner now for this) that takes precedence, i.e. the wrong one.

I guess you are going to fix it separately then, right?

(In reply to Milan Crha from comment #20)
> > Event though a later gtk_menu_size_allocate() is supposed to make it right,
> > on Wayland it's the size set in create_dynamic_positioner() (as we are using
> > xdg_posiionner now for this) that takes precedence, i.e. the wrong one.
> 
> I guess you are going to fix it separately then, right?

How? It's a race condition between the application and the compositor, whoever comes last has the final word...

I have a workaround for gdk wayland backend though, it seems to work...

Created attachment 340757 [details] [review]
[PATCH gtk-3-22] wayland: Fix a race condition with xdg_popup resize

When resizing an xdg_popup immediately after the initial mapping, there
is a race condition between the client and the compositor which is
processing the initial size given by the xdg_positioner, leading to the
xdg_popup to be eventually of the wrong size.

Only way to make sure the size is correct in that case is to hide and
show the window again. Considering this occurs before the initial
configure is processed, it should not be noticeable.

Moving back to gtk+ for review of the patch.

Review of attachment 340757 [details] [review]:

Looks like bad application behavior, but a reasonable work-around.

::: gdk/wayland/gdkwindow-wayland.c
@@ +208,3 @@
   struct zxdg_imported_v1 *imported_transient_for;
+
+  gboolean pending_configure;

This looks unused.

(In reply to Jonas Ådahl from comment #25)
> This looks unused.

Oops, right!

Created attachment 346569 [details] [review]
[PATCH gtk-3-22 v2] wayland: Fix a race condition with xdg_popup resize

v2: Remove unused fields as per review.

Review of attachment 346569 [details] [review]:

Looks good, just a spelling error in the comment.

::: gdk/wayland/gdkwindow-wayland.c
@@ +1023,3 @@
     return;
 
+  /* For xdg_popup using an xdg_positionner, there is a race condition if

spelling error (nn -> n) :P

(In reply to Jonas Ådahl from comment #28)
> +  /* For xdg_popup using an xdg_positionner, there is a race condition if
> 
> spelling error (nn -> n) :P

Oh, right, I always that that one wrong for some reason... :)

Comment on attachment 346569 [details] [review]
[PATCH gtk-3-22 v2] wayland: Fix a race condition with xdg_popup resize

attachment 346569 [details] [review] pushed to branch gtk-3-22 as commit 68188fc - wayland: Fix a race condition with xdg_popup resize

attachment 346569 [details] [review] pushed to git master as commit 83b54ba - wayland: Fix a race condition with xdg_popup resize