Review of attachment 288708 [details] [review]:

Doesn't quite work right: with this patch, gtk3-widget-factory looses the separator above 'Dessert', but there is no iconic section directly above or below it.

::: gtk/gtkmenusectionbox.c
@@ +54,3 @@
+  gint     n_items;
+  gboolean previous_is_iconic;
+} menu_data;

GTK coding style would probably leave out the struct name (_menu_data) if you are going for a typedef anyway. Also, We'd use camel case: MenuData.

Created attachment 288799 [details]
popover menus tester

I add an app that I made to test the popover menus

Created attachment 288800 [details] [review]
popover menu separator fix ( with dynamic items insertion support )

fix separators with iconic sections.

Separators are also updated in case of dynamic insertion
( often used with menu items for plugins )

Review of attachment 288800 [details] [review]:

Can you explain the changes to the reference handling for box->separator ?

Previously, we just kept a pointer that we clear when the separator is destroyed together with its container. You change things to keep a reference to the separator, and remove the clearing of the pointer, but we still have code that looks at box->separator == NULL in places. Also, I don't see where the reference you take is ever getting cleared, so I would say that we're leaking separators now.

Before, a separator was added or removed definitely
and we were using a weak pointer to be aware of that.

Actually, looking at Gedit gear menu, when you activate
a plugin adding a menu item, we have no separator for the section
below because it's already destroyed ( there's no need for it with
no menu item above it )

To be able to do dynamic's insertion/removal of an item
with the correct separators, we now keep the separator around ( sink ),
ready to be added or removed to his container.

The separator is destroyed in dispose.

I have keeped the test:   if (box->separator == NULL) return;

to avoid this :

Gtk-CRITICAL **: gtk_widget_get_parent: assertion 'GTK_IS_WIDGET (widget)' failed

So there is some cases where box->separator is still NULL :

An item call gtk_menu_section_box_sync_separators but have no separator at all
( gtk_menu_section_box_sync_item call gtk_menu_section_box_sync_separators )

(In reply to comment #5)

> with the correct separators, we now keep the separator around ( sink ),

The ref_sink you're calling there is a bit misleading - the floating ref is already taken by adding the separator to the container. It would be clearer to
just call g_object_ref. Or call ref_sink first thing after creating the separator, to make it clear that you are taking the floating ref.

> The separator is destroyed in dispose.

Calling destroy() on the separator is not dropping the extra ref you're taking. It will not be finalized unless you unref it.

Created attachment 288981 [details] [review]
 popover menu separator fix ( with dynamic items insertion support )

Created attachment 288982 [details] [review]
 popover menu separator fix ( with dynamic items insertion support )

i forget an unused var : GtkWidget *parent_widget;