From 272bf0d8ebce8f12e9bb0de2c389d3a18d319afb Mon Sep 17 00:00:00 2001

From: Eric Williams

Date: Wed, 28 Feb 2018 16:10:48 -0500

Subject: Bug 431423: [GTK3] Context menu should appear by pointer when invoked

 via mouse

Some context menus are positioned incorrectly when the size (number of

items) of the menu changes between clicks.

The cause of this bug is due to the way SWT populates its menus: often

it is done asynchronously outside of SWT based on an SWT.Show listener.

The result is that the menu items are added/removed just before showing,

and GTK sometimes doesn't have enough time to properly adjust the size

of the menu internally. Specifically, the GdkWindow of the toplevel

widget (GtkWindow) for the menu lags in height. Since GTK thinks the

menu is taller/shorter than it is, the menu is positioned at the wrong

y-coordinate.

The fix is to check if the number of menu items has changed from one

pop-up to the next. If so, calculate the preferred height of the menu

and resize the GdkWindow of the menu's toplevel GtkWindow to reflect

this change. This way the internal sizing calculations in GTK will be

accurate and the menu will be positioned correctly.

The bug is fixed on GTK3.22 with X11 only, as this uses the new GTK3.22

GtkMenu API. Older GTK3 versions use the outdated GtkMenu API and fixing

these cases would not be worth the effort. Wayland is not affected by

this bug.

Reliable steps to reproduce the issue can be found here:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=431423#c19

Tested JFace/SWT menu snippets, AllNonBrowser JUnit tests, and

ControlExample on GTK3.22 with X11.

Change-Id: I392983438cc32b9455c53cc464626294f3ec72f9

Signed-off-by: Eric Williams <ericwill@redhat.com>---

 .../Eclipse SWT PI/gtk/library/os.c                | 25 ++++++++++++

 .../Eclipse SWT PI/gtk/library/os_stats.c          |  1 +

 .../Eclipse SWT PI/gtk/library/os_stats.h          |  1 +

 .../gtk/org/eclipse/swt/internal/gtk/OS.java      | 12 ++++++

 .../gtk/org/eclipse/swt/widgets/Menu.java          | 46 ++++++++++++++++++++++

 5 files changed, 85 insertions(+)

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 428d448..7cddab1 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	

@@ -16214,6 +16214,31 @@ JNIEXPORT jintLong JNICALL GTK_NATIVE(_1gtk_1widget_1get_1parent_1window)

 }

 #endif

+#ifndef NO__1gtk_1widget_1get_1preferred_1height

+JNIEXPORT void JNICALL OS_NATIVE(_1gtk_1widget_1get_1preferred_1height)

+	(JNIEnv *env, jclass that, jintLong arg0, jintArray arg1, jintArray arg2)

+{

+	jint *lparg1=NULL;

+	jint *lparg2=NULL;

+	OS_NATIVE_ENTER(env, that, _1gtk_1widget_1get_1preferred_1height_FUNC);

+	if (arg1) if ((lparg1 = (*env)->GetIntArrayElements(env, arg1, NULL)) == NULL) goto fail;

+	if (arg2) if ((lparg2 = (*env)->GetIntArrayElements(env, arg2, NULL)) == NULL) goto fail;

+/*

+	gtk_widget_get_preferred_height(arg0, lparg1, lparg2);

+*/

+	{

+		OS_LOAD_FUNCTION(fp, gtk_widget_get_preferred_height)

+		if (fp) {

+			((void (CALLING_CONVENTION*)(jintLong, jint *, jint *))fp)(arg0, lparg1, lparg2);

+		}

+	}

+fail:

+	if (arg2 && lparg2) (*env)->ReleaseIntArrayElements(env, arg2, lparg2, 0);

+	if (arg1 && lparg1) (*env)->ReleaseIntArrayElements(env, arg1, lparg1, 0);

+	OS_NATIVE_EXIT(env, that, _1gtk_1widget_1get_1preferred_1height_FUNC);

+}

+#endif

+

 #ifndef NO__1gtk_1widget_1get_1preferred_1height_1for_1width

 JNIEXPORT void JNICALL OS_NATIVE(_1gtk_1widget_1get_1preferred_1height_1for_1width)

 	(JNIEnv *env, jclass that, jintLong arg0, jint arg1, jintArray arg2, jintArray arg3)

diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h

index ddb326e..43acdb3 100644

--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h	

+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_custom.h	

@@ -296,6 +296,7 @@

 #define gtk_widget_override_color_LIB LIB_GTK

 #define gtk_widget_override_background_color_LIB LIB_GTK

 #define gtk_widget_override_font_LIB LIB_GTK

+#define gtk_widget_get_preferred_height_LIB LIB_GTK

 #define gtk_widget_get_preferred_height_for_width_LIB LIB_GTK

 #define gtk_widget_get_preferred_width_for_height_LIB LIB_GTK

 #define gtk_style_context_get_font_LIB LIB_GTK

diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c

index ddb326e..43acdb3 100644

--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c	

+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/library/os_stats.c	

@@ -1321,6 +1321,7 @@ char * GTK_nativeFunctionNames[] = {

 	"_1gtk_1widget_1get_1pango_1context",

 	"_1gtk_1widget_1get_1parent",

 	"_1gtk_1widget_1get_1parent_1window",

+	"_1gtk_1widget_1get_1preferred_1height",

 	"_1gtk_1widget_1get_1preferred_1height_1for_1width",

 	"_1gtk_1widget_1get_1preferred_1size",

 	"_1gtk_1widget_1get_1preferred_1width_1for_1height",

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 f8fd88f..2038eea 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	

@@ -1331,6 +1331,7 @@ typedef enum {

 	_1gtk_1widget_1get_1pango_1context_FUNC,

 	_1gtk_1widget_1get_1parent_FUNC,

 	_1gtk_1widget_1get_1parent_1window_FUNC,

+	_1gtk_1widget_1get_1preferred_1height_FUNC,

 	_1gtk_1widget_1get_1preferred_1height_1for_1width_FUNC,

 	_1gtk_1widget_1get_1preferred_1size_FUNC,

 	_1gtk_1widget_1get_1preferred_1width_1for_1height_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 a343cfe..a324248 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	

@@ -6388,6 +6388,18 @@ public class GTK extends OS {

 /**

  * @method flags=dynamic

  */

+public static final native void _gtk_widget_get_preferred_height(long /*int*/ widget, int[] minimum_size, int[] natural_size);

+public static final void gtk_widget_get_preferred_height(long /*int*/ widget, int[] minimum_size, int[] natural_size) {

+	lock.lock();

+	try {

+		_gtk_widget_get_preferred_height(widget, minimum_size, natural_size);

+	} finally {

+		lock.unlock();

+	}

+}

+/**

+ * @method flags=dynamic

+ */

 public static final native void _gtk_widget_get_preferred_width_for_height(long /*int*/ widget, int height, int[] minimum_size, int[] natural_size);

 public static final void gtk_widget_get_preferred_width_for_height(long /*int*/ widget, int height, int[] minimum_size, int[] natural_size) {

 	lock.lock();

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 d6ddbbc..6620fbc 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	

@@ -45,6 +45,7 @@ public class Menu extends Widget {

 	Decorations parent;

 	long /*int*/ imItem, imSeparator, imHandle;

 	ImageList imageList;

+	int poppedUpCount;

 /**

  * Constructs a new instance of this class given its parent,

@@ -227,7 +228,9 @@

 			* will fail.  The fix is to ensure that the timestamp of the last

 			* event processed is used.

 			*/

+			verifyMenuPosition(getItemCount());

 			OS.gtk_menu_popup (handle, 0, 0, address, data, 0, display.getLastEventTime ());

+			poppedUpCount = getItemCount();

 		} else {

 			sendEvent (SWT.Hide);

 		}

@@ -1069,6 +1072,49 @@ void setOrientation (boolean create) {

 }

 /**

+ * Feature in GTK3 on X11: context menus in SWT are populated

+ * dynamically, sometimes asynchronously outside of SWT

+ * (i.e. in Platform UI). This means that items are added and

+ * removed just before the menu is shown. This method of

+ * changing the menu content can sometimes cause sizing issues

+ * internally in GTK, specifically with the height of the

+ * toplevel GdkWindow. 

+ *

+ * The fix is to cache the number of items popped up previously,

+ * and if the number of items in the current menu (to be popped up)

+ * is different, then:

+ *     
get the preferred height of the menu

+ *     
set the toplevel GdkWindow to that height

+ *

+ * @param itemCount the current number of items in the menu, just

+ * before it's about to be shown/popped-up

+ */

+void verifyMenuPosition (int itemCount) {

+	if (OS.GTK3 && OS.isX11()) {

+		if (itemCount != poppedUpCount && poppedUpCount != 0) {

+			int [] naturalHeight = new int [1];

+			/*

+			 * We need to "show" the menu before fetching the preferred height.

+			 * Note, this does not actually pop-up the menu.

+			 */

+			OS.gtk_widget_show(handle);

+			/*

+			 * Menus are height-for-width only: use gtk_widget_get_preferred_height()

+			 * instead of gtk_widget_get_preferred_size().

+			 */

+			OS.gtk_widget_get_preferred_height(handle, null, naturalHeight);

+			if (naturalHeight[0] > 0) {

+				long /*int*/ topLevelWidget = OS.gtk_widget_get_toplevel(handle);

+				long /*int*/ topLevelWindow = OS.gtk_widget_get_window(topLevelWidget);

+				int width = OS.gdk_window_get_width(topLevelWindow);

+				OS.gdk_window_resize(topLevelWindow, width, naturalHeight[0]);

+			}

+		}

+	}

+	return;

+}

+

+/**

  * Marks the receiver as visible if the argument is true,

  * and marks it invisible otherwise.

  *