Created attachment 338266 [details] [review]
toolbar: port from gtk_menu_popup()

Created attachment 338267 [details] [review]
toolbar: port from gtk_menu_popup()

gtk_menu_popup() was deprecated in GTK+ 3.22. By replacing the call with
one of the recommended ones, some code has also become redundant.

Review of attachment 338267 [details] [review]:

much better

::: src/nautilus-toolbar.c
@@ +254,3 @@
+     * “button-release-event” seems to not get fired at this point,
+     * so call this to squelch the warning about an already-removed source.
+     */

returning FALSE removes the source. The problem is that you want to set the resource id to 0 in your priv data.
This comment is slightly misleading. I think you can just say "reset the resource id" or actually just set the id to 0 directly, although I prefer the function with some comment.

(In reply to Carlos Soriano from comment #3)
> returning FALSE removes the source. The problem is that you want to set the
> resource id to 0 in your priv data.

Oh, yeah, right. I did not even notice.

Created attachment 338348 [details] [review]
toolbar: port from gtk_menu_popup()

gtk_menu_popup() was deprecated in GTK+ 3.22. By replacing the call with
one of the recommended ones, some code has also become redundant.

Review of attachment 338348 [details] [review]:

::: src/nautilus-toolbar.c
@@ +255,3 @@
+     * it tries to remove a removed source.
+     * Avoid that by doing the cleaning up ourselves.
+     */

this is common glib/gtk+ handling. Not sure it worth so lengthy comment. Instead, something shorter like "reset the id" would be enough...

@@ +258,3 @@
+    unschedule_menu_popup_timeout (data->self);
+
+    return G_SOURCE_REMOVE;

ah yeah, forgot to mention this. Thanks for noticing!

Created attachment 338367 [details] [review]
toolbar: port from gtk_menu_popup()

gtk_menu_popup() was deprecated in GTK+ 3.22. By replacing the call with
one of the recommended ones, some code has also become redundant.

(In reply to Carlos Soriano from comment #6)
> Not sure it worth so lengthy comment.

Oh my. I never thought I could be too explicit, haha.

Okay, so the ID is cleared in the callbacks of “button-press-event” and “button-release-event“, but, since the latter does not fire after the menu is popped up, it does not get cleared. I was explaining that and not the actual source removal.

Review of attachment 338367 [details] [review]:

yeah thanks!

Attachment 338367 [details] pushed as 0782ed9 - toolbar: port from gtk_menu_popup()