eclipse-platform / eclipse.platform.swt

Eclipse SWT
https://www.eclipse.org/swt/
Eclipse Public License 2.0
114 stars 130 forks source link

[GTK3] Context menu sporadically shows empty area on scrolling down #220

Open trancexpress opened 2 years ago

trancexpress commented 2 years ago

Describe the bug See: https://bugs.eclipse.org/bugs/show_bug.cgi?id=564910 and https://bugzilla.redhat.com/show_bug.cgi?id=2043027

Eclipse on GTK3 sporadically shows empty areas in context menus, that can be scrolled down infinitely.

To Reproduce Reproduction steps with SWT:

  1. Run the snippet below.
  2. Open the context menu, scroll to bottom.
  3. Close the menu by clicking on the Text widget.
  4. Press 'd', open context menu and scroll up and down.
  5. Press 'd' again, open context menu and scroll down.
  6. If necessary, repeat until the bug is seen.

SWT snippet to reproduce the problem with:

public static void main(String[] args) {
    Display display = new Display();
    Shell shell = new Shell(display);
    shell.setText("Test popup menu scrolling");
    shell.setLayout(new FillLayout(SWT.VERTICAL));
    shell.setSize(500, 400);

    Text bn = new Text(shell, SWT.MULTI | SWT.V_SCROLL | SWT.H_SCROLL);
    Menu popupMenu = new Menu(bn);

    List<MenuItem> temporaryItems = new ArrayList<>();

    StringBuilder t = new StringBuilder();
    int n = 120;
    for (int i = 0; i < n; ++i) {
        t.append("some text line " + i + " some text line " + i + " some text line " + i + " some text line " + i + " some text line " + i + " some text line " + i);
        t.append(System.lineSeparator());
    }
    bn.setText(t.toString());
    bn.addKeyListener(new KeyAdapter() {
        @Override
        public void keyPressed(KeyEvent e) {
            if (e.keyCode == 'd') {
                if (temporaryItems.isEmpty()) {
                    MenuItem tmp = new MenuItem(popupMenu, SWT.NONE);
                    tmp.setText("temporary menu item");
                    temporaryItems.add(tmp);
                } else {
                    for (MenuItem tmp : temporaryItems) {
                        tmp.dispose();
                    }
                }
            }
        }
    });

    bn.setMenu(popupMenu);

    n = 12;
    for (int i = 0; i < n; ++i) {
        createMenuItem(popupMenu, i, "New ");
        createMenuItem(popupMenu, i, "Refresh ");
        new MenuItem(popupMenu, SWT.SEPARATOR);
        createMenuItem(popupMenu, i, "Next ");
    }
    new MenuItem(popupMenu, SWT.SEPARATOR);
    createMenuItem(popupMenu, n, "Last ");

    shell.open();
    while (!shell.isDisposed()) {
        if (!display.readAndDispatch()) {
            display.sleep();
        }
    }
}

private static void createMenuItem(Menu popupMenu, int i, String text) {
    MenuItem newItem = new MenuItem(popupMenu, SWT.NONE);
    newItem.setText(text + i);
}

Expected behavior Context menus have no empty areas with no items, scrolling up and down does not result in such empty areas and instead is clamped at the menu top and bottom.

Screenshots

https://user-images.githubusercontent.com/24752155/174958699-3246e372-d83d-4e81-8fba-397df5c08317.mp4

Environment:

  1. Select the platform(s) on which the behavior is seen:

      • [ ] All OS
      • [ ] Windows
      • [x] Linux
      • [ ] macOS
  2. Additional OS info (e.g. OS version, Linux Desktop, etc)

RHEL 7.9, RHEL 9

  1. JRE/JDK version

OpenJDK 8, OpenJDK 11, OpenJDK 17

Version since Since at least 4.15, probably since Eclipse on GTK3 (I think that is Eclipse 4.6 or Eclipse 4.7).

Workaround (or) Additional context No known workaround.

trancexpress commented 2 years ago

GTK+ maintainer(s) from RHEL have commented that the menu behavior is coming from the combo behavior. Where the combo opens with the mouse pointer at some entry, possibly showing an entire screen worth of empty area. We had an Eclipse bug for this, combos in Eclipse don't do that: https://bugs.eclipse.org/bugs/show_bug.cgi?id=438992

Anyway, the maintainer(s) has expressed reluctance to fix the bug since they don't know whether there will be side effects.

trancexpress commented 2 years ago

The workaround in https://bugs.eclipse.org/bugs/show_bug.cgi?id=438992 was to call this method of GtkCombo:

void
gtk_combo_box_set_wrap_width (GtkComboBox *combo_box,
                              gint         width)
{
  GtkComboBoxPrivate *priv;

  g_return_if_fail (GTK_IS_COMBO_BOX (combo_box));
  g_return_if_fail (width >= 0);

  priv = combo_box->priv;

  if (width != priv->wrap_width)
    {
      priv->wrap_width = width;

      gtk_combo_box_check_appearance (combo_box);

      if (GTK_IS_TREE_MENU (priv->popup_widget))
        _gtk_tree_menu_set_wrap_width (GTK_TREE_MENU (priv->popup_widget), priv->wrap_width);

      g_object_notify (G_OBJECT (combo_box), "wrap-width");
    }
}

Since we were told the problem in menus was coming from the code for combos (I don't want to know), maybe we can use the same workaround in SWT also for menus... The code above is calling a menu method after all. That method is private (I think) but there is PROP_WRAP_WIDTH ("wrap-width") to call the method when setting menu properties.

Trying to set it in SWT Menu results in:

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 cc84e72f97..6b09e8dc5f 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   
@@ -410,6 +410,7 @@ public class OS extends C {
        public static final byte[] margin_bottom = ascii("margin-bottom");
        public static final byte[] margin_top = ascii("margin-top");
        public static final byte[] scrollbar_spacing = ascii("scrollbar-spacing");
+       public static final byte[] wrap_width = ascii("wrap-width");

        /** Actions */
        public static final byte[] action_copy_clipboard = ascii("clipboard.copy");
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 46df196ca1..5fdd11b1ad 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 
@@ -511,11 +511,13 @@ void createHandle (int index) {
                        long vboxHandle = parent.vboxHandle;
                        GTK3.gtk_container_add(vboxHandle, handle);
                        gtk_box_set_child_packing(vboxHandle, handle, false, true, 0, GTK.GTK_PACK_START);
+                       OS.g_object_set(handle, OS.wrap_width, 1, 0);
                } else {
                        handle = GTK3.gtk_menu_new();
                        if (handle == 0) error(SWT.ERROR_NO_HANDLES);
                        menuHandle = handle;
                        OS.g_object_ref_sink(menuHandle);
+                       OS.g_object_set(handle, OS.wrap_width, 1, 0);
                }
        }
 }
(SWT:39664): GLib-GObject-WARNING **: 09:55:20.887: g_object_set_is_valid_property: object class 'GtkMenu' has no property named 'wrap-width'