From 9f101f432bb6d7b067094dd73c854236956b7034 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=90=D0=BB=D0=B5=D0=BA=D1=81=D0=B0=D0=BD=D0=B4=D1=8A?= =?UTF-8?q?=D1=80=20=D0=9A=D1=83=D1=80=D1=82=D0=B0=D0=BA=D0=BE=D0=B2?= Date: Mon, 3 Aug 2026 17:41:55 +0300 Subject: [PATCH] [Gtk4] Support MenuItem images on GTK4 GtkModelButton (GTK4's native menu row) never shows both an icon and text, so PUSH menu items with images rendered without their icon. Work around this by embedding a custom icon+label(+accelerator) GtkButton into the popover via gtk_popover_menu_add_child() for PUSH items. CASCADE, CHECK and RADIO items keep the native GtkModelButton, since GTK ties their submenu link / selection indicator to it and there is no way to reproduce that in a custom widget. Injection is self-healing: GTK's "custom" ids are single-use once their placeholder slot is destroyed (e.g. by menu repopulation), so a detached widget is re-injected under a fresh id rather than reusing the stale one, which previously left dynamically-rebuilt items (like Run/Debug) invisible. Fixes #2511 Assisted-by: Anthropic Claude Code (claude-sonnet-5) --- .../Eclipse SWT PI/gtk/library/gtk4.c | 42 ++ .../Eclipse SWT PI/gtk/library/gtk4_stats.h | 3 + .../Eclipse SWT PI/gtk/library/os.c | 14 + .../Eclipse SWT PI/gtk/library/os_stats.h | 1 + .../gtk/org/eclipse/swt/internal/gtk/OS.java | 6 + .../org/eclipse/swt/internal/gtk4/GTK4.java | 18 + .../gtk/org/eclipse/swt/widgets/Display.java | 9 + .../gtk/org/eclipse/swt/widgets/Menu.java | 19 + .../gtk/org/eclipse/swt/widgets/MenuItem.java | 388 ++++++++++++++++-- ...Test_org_eclipse_swt_widgets_MenuItem.java | 2 - 10 files changed, 470 insertions(+), 32 deletions(-) diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk4.c b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk4.c index 4f41025f6c9..c1fbf7a4cd1 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk4.c +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk4.c @@ -806,6 +806,20 @@ JNIEXPORT void JNICALL GTK4_NATIVE(gsk_1render_1node_1unref) } #endif +#ifndef NO_gtk_1actionable_1set_1action_1name +JNIEXPORT void JNICALL GTK4_NATIVE(gtk_1actionable_1set_1action_1name) + (JNIEnv *env, jclass that, jlong arg0, jbyteArray arg1) +{ + jbyte *lparg1=NULL; + GTK4_NATIVE_ENTER(env, that, gtk_1actionable_1set_1action_1name_FUNC); + if (arg1) if ((lparg1 = (*env)->GetByteArrayElements(env, arg1, NULL)) == NULL) goto fail; + gtk_actionable_set_action_name((GtkActionable *)arg0, (const char *)lparg1); +fail: + if (arg1 && lparg1) (*env)->ReleaseByteArrayElements(env, arg1, lparg1, 0); + GTK4_NATIVE_EXIT(env, that, gtk_1actionable_1set_1action_1name_FUNC); +} +#endif + #ifndef NO_gtk_1box_1append JNIEXPORT void JNICALL GTK4_NATIVE(gtk_1box_1append) (JNIEnv *env, jclass that, jlong arg0, jlong arg1) @@ -2270,6 +2284,22 @@ JNIEXPORT void JNICALL GTK4_NATIVE(gtk_1picture_1set_1paintable) } #endif +#ifndef NO_gtk_1popover_1menu_1add_1child +JNIEXPORT jboolean JNICALL GTK4_NATIVE(gtk_1popover_1menu_1add_1child) + (JNIEnv *env, jclass that, jlong arg0, jlong arg1, jbyteArray arg2) +{ + jbyte *lparg2=NULL; + jboolean rc = 0; + GTK4_NATIVE_ENTER(env, that, gtk_1popover_1menu_1add_1child_FUNC); + if (arg2) if ((lparg2 = (*env)->GetByteArrayElements(env, arg2, NULL)) == NULL) goto fail; + rc = (jboolean)gtk_popover_menu_add_child((GtkPopoverMenu *)arg0, (GtkWidget *)arg1, (const char *)lparg2); +fail: + if (arg2 && lparg2) (*env)->ReleaseByteArrayElements(env, arg2, lparg2, 0); + GTK4_NATIVE_EXIT(env, that, gtk_1popover_1menu_1add_1child_FUNC); + return rc; +} +#endif + #ifndef NO_gtk_1popover_1menu_1bar_1new_1from_1model JNIEXPORT jlong JNICALL GTK4_NATIVE(gtk_1popover_1menu_1bar_1new_1from_1model) (JNIEnv *env, jclass that, jlong arg0) @@ -2306,6 +2336,18 @@ JNIEXPORT jlong JNICALL GTK4_NATIVE(gtk_1popover_1menu_1new_1from_1model_1full) } #endif +#ifndef NO_gtk_1popover_1menu_1remove_1child +JNIEXPORT jboolean JNICALL GTK4_NATIVE(gtk_1popover_1menu_1remove_1child) + (JNIEnv *env, jclass that, jlong arg0, jlong arg1) +{ + jboolean rc = 0; + GTK4_NATIVE_ENTER(env, that, gtk_1popover_1menu_1remove_1child_FUNC); + rc = (jboolean)gtk_popover_menu_remove_child((GtkPopoverMenu *)arg0, (GtkWidget *)arg1); + GTK4_NATIVE_EXIT(env, that, gtk_1popover_1menu_1remove_1child_FUNC); + return rc; +} +#endif + #ifndef NO_gtk_1popover_1menu_1set_1menu_1model JNIEXPORT void JNICALL GTK4_NATIVE(gtk_1popover_1menu_1set_1menu_1model) (JNIEnv *env, jclass that, jlong arg0, jlong arg1) diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk4_stats.h b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk4_stats.h index f240d03cae9..67f07280c5c 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk4_stats.h +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/gtk4_stats.h @@ -84,6 +84,7 @@ typedef enum { gdk_1toplevel_1size_1set_1size_FUNC, gsk_1render_1node_1draw_FUNC, gsk_1render_1node_1unref_FUNC, + gtk_1actionable_1set_1action_1name_FUNC, gtk_1box_1append_FUNC, gtk_1box_1insert_1child_1after_FUNC, gtk_1box_1prepend_FUNC, @@ -186,9 +187,11 @@ typedef enum { gtk_1picture_1new_FUNC, gtk_1picture_1set_1can_1shrink_FUNC, gtk_1picture_1set_1paintable_FUNC, + gtk_1popover_1menu_1add_1child_FUNC, gtk_1popover_1menu_1bar_1new_1from_1model_FUNC, gtk_1popover_1menu_1get_1menu_1model_FUNC, gtk_1popover_1menu_1new_1from_1model_1full_FUNC, + gtk_1popover_1menu_1remove_1child_FUNC, gtk_1popover_1menu_1set_1menu_1model_FUNC, gtk_1popover_1new_FUNC, gtk_1popover_1set_1autohide_FUNC, diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c index 24178bd35c9..3a11b3f46f5 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os.c @@ -11984,6 +11984,20 @@ JNIEXPORT void JNICALL OS_NATIVE(g_1menu_1item_1set_1attribute) } #endif +#ifndef NO_g_1menu_1item_1set_1attribute_1value +JNIEXPORT void JNICALL OS_NATIVE(g_1menu_1item_1set_1attribute_1value) + (JNIEnv *env, jclass that, jlong arg0, jbyteArray arg1, jlong arg2) +{ + jbyte *lparg1=NULL; + OS_NATIVE_ENTER(env, that, g_1menu_1item_1set_1attribute_1value_FUNC); + if (arg1) if ((lparg1 = (*env)->GetByteArrayElements(env, arg1, NULL)) == NULL) goto fail; + g_menu_item_set_attribute_value((GMenuItem *)arg0, (const gchar *)lparg1, (GVariant *)arg2); +fail: + if (arg1 && lparg1) (*env)->ReleaseByteArrayElements(env, arg1, lparg1, 0); + OS_NATIVE_EXIT(env, that, g_1menu_1item_1set_1attribute_1value_FUNC); +} +#endif + #ifndef NO_g_1menu_1item_1set_1label JNIEXPORT void JNICALL OS_NATIVE(g_1menu_1item_1set_1label) (JNIEnv *env, jclass that, jlong arg0, jbyteArray arg1) diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h index f30ca89d0ad..c9585a87c9e 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.h @@ -969,6 +969,7 @@ typedef enum { g_1menu_1item_1new_1section_FUNC, g_1menu_1item_1new_1submenu_FUNC, g_1menu_1item_1set_1attribute_FUNC, + g_1menu_1item_1set_1attribute_1value_FUNC, g_1menu_1item_1set_1label_FUNC, g_1menu_1item_1set_1submenu_FUNC, g_1menu_1new_FUNC, diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java index a070c4d1329..d168bf5218a 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/OS.java @@ -2417,6 +2417,12 @@ public static final native long g_dbus_proxy_new_for_bus_sync(int bus_type, int * @param data cast=(const gchar *) */ public static final native void g_menu_item_set_attribute(long menu_item, byte[] attribute, byte[] format_string, long data); +/** + * @param menu_item cast=(GMenuItem *) + * @param attribute cast=(const gchar *) + * @param value cast=(GVariant *) + */ +public static final native void g_menu_item_set_attribute_value(long menu_item, byte[] attribute, long value); /* GSimpleActionGroup */ public static final native long g_simple_action_group_new(); diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk4/GTK4.java b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk4/GTK4.java index 3c33dfcfb79..ff37680966a 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk4/GTK4.java +++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk4/GTK4.java @@ -75,6 +75,13 @@ public class GTK4 { */ public static final native void gtk_rgb_to_hsv(float r, float g, float b, float[] h, float[] s, float[] v); + /* GtkActionable */ + /** + * @param actionable cast=(GtkActionable *) + * @param action_name cast=(const char *) + */ + public static final native void gtk_actionable_set_action_name(long actionable, byte[] action_name); + /* GtkBox */ /** * @param box cast=(GtkBox *) @@ -590,6 +597,17 @@ public class GTK4 { public static final native void gtk_text_set_tabs(long entry, long tabs); /* GtkPopoverMenu */ + /** + * @param popover cast=(GtkPopoverMenu *) + * @param child cast=(GtkWidget *) + * @param id cast=(const char *) + */ + public static final native boolean gtk_popover_menu_add_child(long popover, long child, byte[] id); + /** + * @param popover cast=(GtkPopoverMenu *) + * @param child cast=(GtkWidget *) + */ + public static final native boolean gtk_popover_menu_remove_child(long popover, long child); /** * @param model cast=(GMenuModel *) * @param flags cast=(GtkPopoverMenuFlags) diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java index 4a1bc89cce3..71879be9b6f 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Display.java @@ -136,6 +136,15 @@ public class Display extends Device implements Executor { long snapshotDrawProc, keyPressReleaseProc, focusProc, windowActiveProc, enterMotionProc, leaveProc, scrollProc, resizeProc, layoutProc, activateProc, gesturePressReleaseProc; long menuItemsChangedProc; + /** + * GTK4 only: set while SWT is mutating a GMenu, i.e. between the + * {@code g_menu_remove} and {@code g_menu_insert_item} of an item refresh, when + * the model no longer matches SWT's own item bookkeeping. The "items-changed" + * emitted by the removal re-enters menu wiring synchronously, so custom menu + * widget injection - which itself refreshes the model, by position - must not + * run until the model is whole again (see MenuItem#refreshMenuModelGTK4). + */ + boolean menuModelMutating; long notifyProc; long computeSizeProc; Callback windowCallback2, windowCallback3, windowCallback4, windowCallback5, windowCallback6; diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Menu.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Menu.java index 19467e4b2d3..b35c398edd1 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Menu.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Menu.java @@ -937,7 +937,9 @@ long gtk_map (long widget) { * The POP_UP GtkPopoverMenu has been mapped. Its handle IS the GtkPopoverMenu, * and nested GtkPopoverMenu children for any CASCADE submenus already exist in * its widget tree. Connect SHOW/HIDE signals for those nested submenus now. + * Also inject any custom icon widgets into the popover. */ + injectCustomMenuIcons(); connectCascadeSubMenuSignals(this, handle); } } @@ -1038,6 +1040,7 @@ private void connectDropDownMenuSignals() { wireSubMenuPopover(menuItem.menu, popover); } if (menuItem.menu.popoverHandle != 0) { + menuItem.menu.injectCustomMenuIcons(); connectCascadeSubMenuSignals(menuItem.menu); } } @@ -1065,12 +1068,27 @@ private void connectCascadeSubMenuSignals(Menu menu, long parentPopoverHandle) { if (item.menu.popoverHandle != nestedPopover) { wireSubMenuPopover(item.menu, nestedPopover); } + item.menu.injectCustomMenuIcons(); connectCascadeSubMenuSignals(item.menu); } } } } +/** + * (Re-)injects the custom icon+label widgets of this menu's PUSH items into the + * GtkPopoverMenu, for those items that have one. A no-op for items whose widget + * is already attached. + */ +void injectCustomMenuIcons() { + if (items == null || display.menuModelMutating) return; + for (MenuItem item : items) { + if (item.customWidgetHandle != 0) { + item.injectCustomWidgetGTK4(); + } + } +} + /** * Recursively searches the widget subtree rooted at {@code parentWidget} for the * nested GtkPopoverMenu whose GMenuModel is {@code targetModel}, returning its @@ -1137,6 +1155,7 @@ long gtk_show (long widget) { sendEvent (SWT.Show); /* Wire cascade submenu SHOW/HIDE signals once the DROP_DOWN popover is shown. */ if (GTK.GTK4 && (style & SWT.DROP_DOWN) != 0 && popoverHandle != 0) { + injectCustomMenuIcons(); connectCascadeSubMenuSignals(this, popoverHandle); } if (OS.ubuntu_menu_proxy_get() != 0) { diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/MenuItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/MenuItem.java index b28f41db7e1..70d1f0fe4ac 100644 --- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/MenuItem.java +++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/MenuItem.java @@ -82,6 +82,10 @@ public class MenuItem extends Item { * API. */ String actionName; + /** GTK4 only: custom widget for icon+text display in GtkPopoverMenu */ + long customWidgetHandle, customImageHandle, customLabelHandle, customAccelHandle; + String customId; + private static int customIdSeq = 0; /** * Constructs a new instance of this class given its parent @@ -355,18 +359,35 @@ void createHandle(int index) { section = parent.new Section(this); int itemsToMove = selectedSection.sectionItems.size() - sectionRelativeIndex; - for (int i = 0; i < itemsToMove; i++) { - MenuItem removedItem = selectedSection.sectionItems.remove(sectionRelativeIndex); - section.sectionItems.add(removedItem); + /* + * Moving items between sections walks the model out of step with + * sectionItems; hold off injection until the split is complete (see + * refreshMenuModelGTK4). + */ + boolean wasMutating = display.menuModelMutating; + display.menuModelMutating = true; + try { + for (int i = 0; i < itemsToMove; i++) { + MenuItem removedItem = selectedSection.sectionItems.remove(sectionRelativeIndex); + section.sectionItems.add(removedItem); + + OS.g_menu_remove(selectedSection.getSectionHandle(), sectionRelativeIndex); + OS.g_menu_insert_item(modelHandle, section.sectionItems.indexOf(removedItem), removedItem.handle); + removedItem.section = section; + } - OS.g_menu_remove(selectedSection.getSectionHandle(), sectionRelativeIndex); - OS.g_menu_insert_item(modelHandle, section.sectionItems.indexOf(removedItem), removedItem.handle); - removedItem.section = section; + int sectionInsertIndex = parent.sections.indexOf(selectedSection) + 1; + parent.sections.add(sectionInsertIndex, section); + OS.g_menu_insert_item(parent.modelHandle, sectionInsertIndex, handle); + } finally { + display.menuModelMutating = wasMutating; } - - int sectionInsertIndex = parent.sections.indexOf(selectedSection) + 1; - parent.sections.add(sectionInsertIndex, section); - OS.g_menu_insert_item(parent.modelHandle, sectionInsertIndex, handle); + /* + * The split rebuilt the moved items' rows, destroying any injected custom + * widgets, while injection was suppressed; re-inject them now that the + * model is whole again. + */ + parent.injectCustomMenuIcons(); } else { section = selectedSection; selectedSection.sectionItems.add(sectionRelativeIndex, this); @@ -783,6 +804,16 @@ void releaseWidget() { * leaks the action for the lifetime of the parent's action group. */ if (parent.actionGroup != 0 && actionId != null) OS.g_action_map_remove_action(parent.actionGroup, Converter.javaStringToCString(actionId)); + if (customWidgetHandle != 0) { + detachCustomMenuWidget(); + display.removeWidget(customWidgetHandle); + OS.g_object_unref(customWidgetHandle); + customWidgetHandle = 0; + customImageHandle = 0; + customLabelHandle = 0; + customAccelHandle = 0; + customId = null; + } } else { long accelGroup = getAccelGroup(); if (accelGroup != 0) removeAccelerator(accelGroup); @@ -800,22 +831,40 @@ void releaseWidget() { @Override void destroyWidget() { if (GTK.GTK4) { - if ((style & SWT.SEPARATOR) != 0) { - Section aboveSection = parent.sections.get(parent.sections.indexOf(section) - 1); - aboveSection.sectionItems.addAll(section.sectionItems); - - for (MenuItem item : section.sectionItems) { - item.section = aboveSection; - OS.g_menu_insert_item(aboveSection.getSectionHandle(), aboveSection.sectionItems.indexOf(item), item.handle); - } + /* + * Removing this item (or merging its section away) walks the model out of + * step with SWT's bookkeeping; hold off custom widget injection until the + * model is whole again (see refreshMenuModelGTK4). + */ + boolean wasMutating = display.menuModelMutating; + display.menuModelMutating = true; + try { + if ((style & SWT.SEPARATOR) != 0) { + Section aboveSection = parent.sections.get(parent.sections.indexOf(section) - 1); + aboveSection.sectionItems.addAll(section.sectionItems); + + for (MenuItem item : section.sectionItems) { + item.section = aboveSection; + OS.g_menu_insert_item(aboveSection.getSectionHandle(), aboveSection.sectionItems.indexOf(item), item.handle); + } - OS.g_menu_remove(parent.modelHandle, parent.sections.indexOf(section)); + OS.g_menu_remove(parent.modelHandle, parent.sections.indexOf(section)); - parent.sections.remove(section); - } else { - OS.g_menu_remove(section.getSectionHandle(), section.sectionItems.indexOf(this)); - section.sectionItems.remove(this); + parent.sections.remove(section); + } else { + OS.g_menu_remove(section.getSectionHandle(), section.sectionItems.indexOf(this)); + section.sectionItems.remove(this); + } + } finally { + display.menuModelMutating = wasMutating; } + /* + * A section merge rebuilt the moved items' rows, destroying any injected + * custom widgets, while injection was suppressed; re-inject them now that + * the model is whole again. (This item's own widget is already gone: + * releaseWidget ran first.) + */ + parent.injectCustomMenuIcons(); parent.items.remove(this); parent = null; @@ -1072,9 +1121,6 @@ public void setID (int id) { */ @Override public void setImage (Image image) { - //TODO: GTK4 Menu images with text are no longer supported - if (GTK.GTK4) return; - checkWidget(); if (this.image == image) return; if ((style & SWT.SEPARATOR) != 0) return; @@ -1085,6 +1131,10 @@ public void setImage (Image image) { } private void _setImage (Image image) { + if (GTK.GTK4) { + _setImageGTK4(image); + return; + } if (image != null) { ImageList imageList = parent.imageList; if (imageList == null) imageList = parent.imageList = new ImageList (); @@ -1130,6 +1180,279 @@ private void _setImage (Image image) { } } +private void _setImageGTK4(Image image) { + // PUSH and CHECK items use a custom icon+label GtkButton (CHECK with an inert + // GtkCheckButton indicator inside, see createCustomMenuWidget). The other styles + // must keep their default GtkModelButton rendering: + // - CASCADE: GtkMenuSectionBox checks the submenu link before the "custom" attribute, + // so a cascade item never gets a placeholder slot for a custom widget. + // - RADIO: its action is stateful-string with a target, which needs + // gtk_actionable_set_detailed_action_name, and a lone GtkCheckButton renders a + // checkmark rather than a radio indicator unless grouped. GtkModelButton hides + // the icon when it has text anyway, so RADIO stays label + indicator (no icon). + if ((style & (SWT.CASCADE | SWT.RADIO)) != 0) { + return; + } + if (image != null) { + long pixbuf = ImageList.createPixbuf(image); + if (pixbuf != 0) { + long texture = GDK.gdk_texture_new_for_pixbuf(pixbuf); + OS.g_object_unref(pixbuf); + if (texture != 0) { + boolean firstTime = customWidgetHandle == 0; + if (firstTime) { + createCustomMenuWidget(); + } + GTK4.gtk_image_set_from_paintable(customImageHandle, texture); + OS.g_object_unref(texture); + if (firstTime) { + // Attempt injection now in case the popover already exists (item + // added to an already-open menu). If it is not ready yet, injection + // is retried - and self-heals - from Menu.injectCustomMenuIcons() + // when the menu is shown. + injectCustomWidgetGTK4(); + } + } + } + } else { + if (customWidgetHandle != 0) { + destroyCustomMenuWidget(); + refreshMenuModelGTK4(); + } + } +} + +private void createCustomMenuWidget() { + customWidgetHandle = GTK.gtk_button_new(); + OS.g_object_ref_sink(customWidgetHandle); + GTK.gtk_widget_add_css_class(customWidgetHandle, Converter.javaStringToCString("flat")); + + long hbox = GTK.gtk_box_new(GTK.GTK_ORIENTATION_HORIZONTAL, 6); + if ((style & SWT.CHECK) != 0) { + /* + * An inert GtkCheckButton serves as the selection indicator: bound to the + * item's stateful action so its checkmark follows the action state + * (including setSelection), but excluded from hit testing and focus so + * every click activates the enclosing button instead. Using the same flat + * GtkButton row as PUSH items keeps height and hover feedback identical. + */ + long indicator = GTK.gtk_check_button_new(); + OS.g_object_set(indicator, Converter.javaStringToCString("can-target"), false, 0); + GTK4.gtk_widget_set_focusable(indicator, false); + if (actionName != null) { + GTK4.gtk_actionable_set_action_name(indicator, Converter.javaStringToCString(actionName)); + } + GTK4.gtk_box_append(hbox, indicator); + } + customImageHandle = GTK.gtk_image_new(); + GTK4.gtk_box_append(hbox, customImageHandle); + + customLabelHandle = GTK.gtk_label_new_with_mnemonic(null); + GTK.gtk_label_set_xalign(customLabelHandle, 0.0f); + // Expand the label so the accelerator label is pushed to the trailing edge, + // matching the layout of a normal GtkModelButton menu row. + GTK.gtk_widget_set_hexpand(customLabelHandle, true); + GTK4.gtk_box_append(hbox, customLabelHandle); + + customAccelHandle = GTK.gtk_label_new(null); + GTK.gtk_label_set_xalign(customAccelHandle, 1.0f); + GTK.gtk_widget_add_css_class(customAccelHandle, Converter.javaStringToCString("dim-label")); + GTK4.gtk_box_append(hbox, customAccelHandle); + + GTK4.gtk_button_set_child(customWidgetHandle, hbox); + updateCustomWidgetLabels(); + + if (actionName != null) { + GTK4.gtk_actionable_set_action_name(customWidgetHandle, Converter.javaStringToCString(actionName)); + } + + /* + * Unlike GtkModelButton, a plain GtkButton does not pop down its enclosing + * popover when clicked, which would leave the menu open after activating the + * item; do it from gtk_clicked(). The button's class handler has already + * activated the action (dispatching SWT.Selection) by the time it runs. + */ + display.addWidget(customWidgetHandle, this); + OS.g_signal_connect_closure(customWidgetHandle, OS.clicked, display.getClosure(CLICKED), false); + + // Mark this GMenuItem slot as "custom" so GtkPopoverMenu creates a placeholder. + // injectCustomWidgetGTK4() reassigns a fresh id before actually embedding. + reassignCustomId(); +} + +@Override +long gtk_clicked (long widget) { + /* Only ever connected on the custom menu row (GTK4), see createCustomMenuWidget. */ + long popover = getParentPopoverHandle(); + if (popover != 0) GTK.gtk_popover_popdown(popover); + return 0; +} + +/** + * Updates the custom widget's label and accelerator sub-labels from {@link #text}. + * SWT menu text carries the accelerator display after a tab (e.g. "Run\tCtrl+F11"); + * a GtkModelButton renders that via its "accel" property, but our custom GtkButton + * must render it explicitly in a trailing, right-aligned label. + */ +private void updateCustomWidgetLabels() { + if (customLabelHandle == 0) return; + String full = text != null ? text : ""; + String label = full; + String accel = ""; + int tab = full.indexOf('\t'); + if (tab != -1) { + label = full.substring(0, tab); + accel = full.substring(tab + 1); + } + char[] chars = fixMnemonic(label); + GTK.gtk_label_set_text_with_mnemonic(customLabelHandle, Converter.wcsToMbcs(chars, true)); + if (customAccelHandle != 0) { + GTK.gtk_label_set_text(customAccelHandle, Converter.wcsToMbcs(accel, true)); + GTK.gtk_widget_set_visible(customAccelHandle, !accel.isEmpty()); + } +} + +private void destroyCustomMenuWidget() { + // Clear the "custom" attribute so the next model rebuild shows a normal model button + OS.g_menu_item_set_attribute_value(handle, Converter.javaStringToCString("custom"), 0); + detachCustomMenuWidget(); + if (customWidgetHandle != 0) { + display.removeWidget(customWidgetHandle); + OS.g_object_unref(customWidgetHandle); + } + customWidgetHandle = 0; + customImageHandle = 0; + customLabelHandle = 0; + customAccelHandle = 0; + customId = null; +} + +/** + * Cleanly detaches an injected custom widget from its GtkPopoverMenu placeholder + * slot before the widget's last reference is dropped, going through + * gtk_popover_menu_remove_child() - the documented counterpart of + * gtk_popover_menu_add_child() - so any bookkeeping the popover keeps for the + * child is released along with the widget itself. Detaching before the unref + * matters: the placeholder holds the parent reference, and dropping our own + * reference while GTK still has the widget in its tree would leave the popover + * pointing at freed memory. + */ +private void detachCustomMenuWidget() { + if (customWidgetHandle == 0) return; + if (GTK.gtk_widget_get_parent(customWidgetHandle) == 0) return; + /* + * Find the GtkPopoverMenu the widget was actually added to by walking up + * from the widget itself: the cached menu popover handle can be stale when + * GTK has rebuilt the menu since injection. + */ + long popover = GTK.gtk_widget_get_parent(customWidgetHandle); + while (popover != 0 && !GTK4.GTK_IS_POPOVER_MENU(popover)) { + popover = GTK.gtk_widget_get_parent(popover); + } + if (popover != 0) { + GTK4.gtk_popover_menu_remove_child(popover, customWidgetHandle); + } + /* Defensive: if no owning popover was found, at least unparent. */ + if (GTK.gtk_widget_get_parent(customWidgetHandle) != 0) { + GTK.gtk_widget_unparent(customWidgetHandle); + } +} + +/** + * Pushes this item's current GMenuItem attributes into the parent GMenu, by + * removing and re-inserting it at its position (GMenu snapshots an item's + * attributes on insertion, so mutating the GMenuItem alone has no effect). + *

+ * The removal emits "items-changed", which synchronously re-enters the menu + * wiring. Injection must be held off for the duration: it refreshes by position + * too, and while this item is removed every later position in the section is + * shifted by one, so a nested refresh would remove the wrong item and leave a + * duplicate behind. + */ +void refreshMenuModelGTK4() { + boolean wasMutating = display.menuModelMutating; + display.menuModelMutating = true; + try { + OS.g_menu_remove(section.getSectionHandle(), section.getItemPosition(this)); + OS.g_menu_insert_item(section.getSectionHandle(), section.getItemPosition(this), handle); + } finally { + display.menuModelMutating = wasMutating; + } +} + +/** + * Ensures this item's custom icon+label widget is embedded into its GtkPopoverMenu + * placeholder slot. Safe to call repeatedly (e.g. every time the menu is shown). + * + * A GTK "custom" id is single-use: GtkMenuSectionBox keeps every registered id in an + * append-only hash table, and destroying the placeholder (which happens whenever the + * model item is removed+reinserted - by separator/section restructuring, cascade + * changes, etc.) unparents our widget without freeing the id. Reinserting under the + * same id then hits "Duplicate custom ID" and no new slot is created, orphaning the + * widget so the row renders empty. We therefore treat "widget currently has no + * parent" as the trigger to (re)establish it under a FRESH id: set the new id, refresh + * the model so GtkMenuTracker materialises a new placeholder, then embed the widget. + */ +void injectCustomWidgetGTK4() { + if (customWidgetHandle == 0 || customId == null) return; + if (display.menuModelMutating) return; + long popoverHandle = getParentPopoverHandle(); + if (popoverHandle == 0) return; + // Already embedded in a live slot: nothing to do. + if (GTK.gtk_widget_get_parent(customWidgetHandle) != 0) return; + // Mint a fresh id and rematerialise the placeholder. The previous id (if any) + // is now burned in GTK's custom_slots and cannot be reused. + reassignCustomId(); + refreshMenuModelGTK4(); + boolean added = GTK4.gtk_popover_menu_add_child(popoverHandle, customWidgetHandle, + Converter.javaStringToCString(customId)); + if (!added) { + // No placeholder slot was created (e.g. a cascade item, whose submenu link + // wins over the custom attribute). Fall back to a plain GtkModelButton so the + // item stays visible (label only, no icon) rather than an empty placeholder. + fallbackToModelButtonGTK4(); + } +} + +/** + * Assigns a fresh, never-before-used "custom" id to this item and writes it onto the + * GMenuItem's "custom" attribute. Used both at creation and whenever a stale id must + * be replaced (see {@link #injectCustomWidgetGTK4()}). + */ +private void reassignCustomId() { + customId = "swt-menu-" + (++customIdSeq); + long variant = OS.g_variant_new_string(Converter.javaStringToCString(customId)); + OS.g_menu_item_set_attribute_value(handle, Converter.javaStringToCString("custom"), variant); + // g_menu_item_set_attribute_value sinks the floating ref; do not unref variant +} + +/** + * Reverts this item to plain GtkModelButton rendering after a failed custom-widget + * injection: clears the "custom" attribute, releases the widget we built, and + * re-inserts the model item so GtkMenuTracker rebuilds it as a normal (visible) + * button. Prevents an un-injectable item from showing as an empty placeholder slot. + */ +private void fallbackToModelButtonGTK4() { + OS.g_menu_item_set_attribute_value(handle, Converter.javaStringToCString("custom"), 0); + if (customWidgetHandle != 0) { + display.removeWidget(customWidgetHandle); + OS.g_object_unref(customWidgetHandle); + customWidgetHandle = 0; + customImageHandle = 0; + customLabelHandle = 0; + customAccelHandle = 0; + customId = null; + } + refreshMenuModelGTK4(); +} + +long getParentPopoverHandle() { + if ((parent.style & SWT.POP_UP) != 0) { + return parent.handle; + } + return parent.popoverHandle; +} + /** * Sets the receiver's pull down menu to the argument. * Only CASCADE menu items can have a @@ -1185,8 +1508,7 @@ public void setMenu (Menu menu) { OS.g_menu_item_set_submenu(handle, 0); } - OS.g_menu_remove(section.getSectionHandle(), section.getItemPosition(this)); - OS.g_menu_insert_item(section.getSectionHandle(), section.getItemPosition(this), handle); + refreshMenuModelGTK4(); /* * If a DROP_DOWN is attached while its parent is already mapped (contributions @@ -1332,8 +1654,14 @@ public void setText (String string) { GTK.gtk_accelerator_name(maskKeysym.keysym, maskKeysym.mask) ); } - OS.g_menu_remove(section.getSectionHandle(), section.getItemPosition(this)); - OS.g_menu_insert_item(section.getSectionHandle(), section.getItemPosition(this), handle); + if (customWidgetHandle != 0) { + // The custom widget renders its own label; do not refresh the model position. + // GTK never forgets a registered "custom" id, so refreshing again here would + // create a duplicate, orphaned placeholder and drop the item from view. + updateCustomWidgetLabels(); + } else { + refreshMenuModelGTK4(); + } } else { if (labelHandle != 0 && GTK.GTK_IS_LABEL (labelHandle)) { GTK.gtk_label_set_text_with_mnemonic (labelHandle, buffer); diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_MenuItem.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_MenuItem.java index 5ea3cbc497d..c8a512590b3 100644 --- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_MenuItem.java +++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_MenuItem.java @@ -29,7 +29,6 @@ import org.eclipse.swt.widgets.Menu; import org.eclipse.swt.widgets.MenuItem; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; /** @@ -208,7 +207,6 @@ public void test_setEnabledZ_cascade() { cascadeItem.dispose(); } -@Tag("gtk4-todo") @Override @Test public void test_setImageLorg_eclipse_swt_graphics_Image() {