Bug 500703: [GTK3.20+] Combo with SWT.READ_ONLY is garbled upon re-size

In GTK3.20+ two things inside GTK3 changed. First, certain widgets have
their clips unioned with the clips of their parent widget. This can
cause overdrawing in widgets like Combo. Second, the internal widget
hierarchy of Combo changed, and most of the resizing is done by a GtkBox
instead of GtkComboBox itself.

These two changes combined caused this bug. The overdrawing causes the
Combo to draw over its neighbouring widgets (like a label). The resize
issue means that resizing the widget doesn't cause it get smaller: the x
coordinates of the allocation and clip just become negative and the
whole widget shifts to the left.

The fix is to adjust the clip and allocation of the Combo and its
children from the parent Composite's draw handler. This ensures that the
Combo is drawn with correct coordinates and that it actually shrinks
when resized, instead of just shifting.

Tested on GTK3.22, X11 and Wayland. Window -> Preferences -> General ->
Keys behaves as expected, and no AllNonBrowser JUnit tests fail.

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

6 files changed, 180 insertions, 3 deletions

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 85b5361401..674d32688d 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
@@ -13340,6 +13340,28 @@ JNIEXPORT jboolean JNICALL GTK_NATIVE(_1gtk_1widget_1get_1child_1visible)
 }
 #endif
 
+#ifndef NO__1gtk_1widget_1get_1clip
+JNIEXPORT void JNICALL GTK_NATIVE(_1gtk_1widget_1get_1clip)
+	(JNIEnv *env, jclass that, jintLong arg0, jobject arg1)
+{
+	GtkAllocation _arg1, *lparg1=NULL;
+	GTK_NATIVE_ENTER(env, that, _1gtk_1widget_1get_1clip_FUNC);
+	if (arg1) if ((lparg1 = getGtkAllocationFields(env, arg1, &_arg1)) == NULL) goto fail;
+/*
+	gtk_widget_get_clip((GtkWidget *)arg0, lparg1);
+*/
+	{
+		GTK_LOAD_FUNCTION(fp, gtk_widget_get_clip)
+		if (fp) {
+			((void (CALLING_CONVENTION*)(GtkWidget *, GtkAllocation *))fp)((GtkWidget *)arg0, lparg1);
+		}
+	}
+fail:
+	if (arg1 && lparg1) setGtkAllocationFields(env, arg1, lparg1);
+	GTK_NATIVE_EXIT(env, that, _1gtk_1widget_1get_1clip_FUNC);
+}
+#endif
+
 #ifndef NO__1gtk_1widget_1get_1default_1style
 JNIEXPORT jintLong JNICALL GTK_NATIVE(_1gtk_1widget_1get_1default_1style)
 	(JNIEnv *env, jclass that)
@@ -14110,6 +14132,28 @@ JNIEXPORT void JNICALL GTK_NATIVE(_1gtk_1widget_1set_1can_1focus)
 }
 #endif
 
+#ifndef NO__1gtk_1widget_1set_1clip
+JNIEXPORT void JNICALL GTK_NATIVE(_1gtk_1widget_1set_1clip)
+	(JNIEnv *env, jclass that, jintLong arg0, jobject arg1)
+{
+	GtkAllocation _arg1, *lparg1=NULL;
+	GTK_NATIVE_ENTER(env, that, _1gtk_1widget_1set_1clip_FUNC);
+	if (arg1) if ((lparg1 = getGtkAllocationFields(env, arg1, &_arg1)) == NULL) goto fail;
+/*
+	gtk_widget_set_clip((GtkWidget *)arg0, lparg1);
+*/
+	{
+		GTK_LOAD_FUNCTION(fp, gtk_widget_set_clip)
+		if (fp) {
+			((void (CALLING_CONVENTION*)(GtkWidget *, GtkAllocation *))fp)((GtkWidget *)arg0, lparg1);
+		}
+	}
+fail:
+	if (arg1 && lparg1) setGtkAllocationFields(env, arg1, lparg1);
+	GTK_NATIVE_EXIT(env, that, _1gtk_1widget_1set_1clip_FUNC);
+}
+#endif
+
 #ifndef NO__1gtk_1widget_1set_1default_1direction
 JNIEXPORT void JNICALL GTK_NATIVE(_1gtk_1widget_1set_1default_1direction)
 	(JNIEnv *env, jclass that, jint arg0)
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 cf583fd184..1bd6a98e1f 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
@@ -1066,6 +1066,7 @@ char * GTK_nativeFunctionNames[] = {
 	"_1gtk_1widget_1get_1allocation",
 	"_1gtk_1widget_1get_1can_1default",
 	"_1gtk_1widget_1get_1child_1visible",
+	"_1gtk_1widget_1get_1clip",
 	"_1gtk_1widget_1get_1default_1style",
 	"_1gtk_1widget_1get_1events",
 	"_1gtk_1widget_1get_1has_1window",
@@ -1116,6 +1117,7 @@ char * GTK_nativeFunctionNames[] = {
 	"_1gtk_1widget_1set_1app_1paintable",
 	"_1gtk_1widget_1set_1can_1default",
 	"_1gtk_1widget_1set_1can_1focus",
+	"_1gtk_1widget_1set_1clip",
 	"_1gtk_1widget_1set_1default_1direction",
 	"_1gtk_1widget_1set_1direction",
 	"_1gtk_1widget_1set_1double_1buffered",
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 d006679824..c999193244 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
@@ -1064,6 +1064,7 @@ typedef enum {
 	_1gtk_1widget_1get_1allocation_FUNC,
 	_1gtk_1widget_1get_1can_1default_FUNC,
 	_1gtk_1widget_1get_1child_1visible_FUNC,
+	_1gtk_1widget_1get_1clip_FUNC,
 	_1gtk_1widget_1get_1default_1style_FUNC,
 	_1gtk_1widget_1get_1events_FUNC,
 	_1gtk_1widget_1get_1has_1window_FUNC,
@@ -1114,6 +1115,7 @@ typedef enum {
 	_1gtk_1widget_1set_1app_1paintable_FUNC,
 	_1gtk_1widget_1set_1can_1default_FUNC,
 	_1gtk_1widget_1set_1can_1focus_FUNC,
+	_1gtk_1widget_1set_1clip_FUNC,
 	_1gtk_1widget_1set_1default_1direction_FUNC,
 	_1gtk_1widget_1set_1direction_FUNC,
 	_1gtk_1widget_1set_1double_1buffered_FUNC,
diff --git a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/GTK.java b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/GTK.java
index ff3427fa12..f361fc1325 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/GTK.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT PI/gtk/org/eclipse/swt/internal/gtk/GTK.java
@@ -2896,6 +2896,32 @@ public class GTK extends OS {
 	        }
 	}
 	/**
+	 * @method flags=dynamic
+	 * @param widget cast=(GtkWidget *)
+	 */
+	public static final native void _gtk_widget_set_clip(long /*int*/ widget, GtkAllocation allocation);
+	public static final void gtk_widget_set_clip(long /*int*/ widget, GtkAllocation allocation) {
+		lock.lock();
+		try {
+			_gtk_widget_set_clip(widget, allocation);
+		} finally {
+			lock.unlock();
+		}
+	}
+	/**
+	 * @method flags=dynamic
+	 * @param widget cast=(GtkWidget *)
+	 */
+	public static final native void _gtk_widget_get_clip(long /*int*/ widget, GtkAllocation allocation);
+	public static final void gtk_widget_get_clip(long /*int*/ widget, GtkAllocation allocation) {
+		lock.lock();
+		try {
+			_gtk_widget_get_clip(widget, allocation);
+		} finally {
+			lock.unlock();
+		}
+	}
+	/**
 	 * @param widget cast=(GtkWidget *)
 	 * @param has_window cast=(gboolean)
 	 */
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Combo.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Combo.java
index b2174e230c..d9f95a3ad8 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Combo.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Combo.java
@@ -58,13 +58,12 @@ import org.eclipse.swt.internal.gtk.*;
  * @noextend This class is not intended to be subclassed by clients.
  */
 public class Combo extends Composite {
-	long /*int*/ buttonHandle, entryHandle, textRenderer, cellHandle, popupHandle, menuHandle;
+	long /*int*/ buttonHandle, entryHandle, textRenderer, cellHandle, popupHandle, menuHandle, buttonBoxHandle, cellBoxHandle;
 	int lastEventTime, visibleCount = 10;
 	long /*int*/ imContext;
 	long /*int*/ gdkEventKey = 0;
 	int fixStart = -1, fixEnd = -1;
 	String [] items = new String [0];
-	boolean selectionAdded;
 	int indexSelected;
 	GdkRGBA background;
 	/**
@@ -502,6 +501,9 @@ void createHandle (int index) {
 	if (menuHandle != 0) OS.g_object_ref (menuHandle);
 	buttonHandle = findButtonHandle ();
 	if (buttonHandle != 0) OS.g_object_ref (buttonHandle);
+	if (buttonBoxHandle != 0) OS.g_object_ref (buttonBoxHandle);
+	if (cellHandle != 0) cellBoxHandle = GTK.gtk_widget_get_parent(cellHandle);
+	if (cellBoxHandle != 0) OS.g_object_ref(cellBoxHandle);
 	/*
 	* Feature in GTK. By default, read only combo boxes
 	* process the RETURN key rather than allowing the
@@ -628,6 +630,7 @@ long /*int*/ findButtonHandle() {
 			OS.g_list_free(display.allChildren);
 			display.allChildren = 0;
 		}
+		buttonBoxHandle = childHandle;
 	}
 
 	GTK.gtk_container_forall (childHandle, display.allChildrenProc, 0);
@@ -1354,6 +1357,34 @@ long /*int*/ gtk_delete_text (long /*int*/ widget, long /*int*/ start_pos, long
 }
 
 @Override
+long /*int*/ gtk_draw (long /*int*/ widget, long /*int*/ cairo) {
+	/*
+	 * Feature in GTK3.20+: READ_ONLY Combos have their clip unioned
+	 * with the parent container (Composite in this case). This causes
+	 * overdrawing into neighbouring widgets and breaks resizing.
+	 *
+	 * The fix is to adjust the clip and allocation of the Combo and its children
+	 * in the parent Composite's draw handler, as it ensures that the draw events
+	 * going to the Combo have the correct geometry specifications. This has to
+	 * happen in the parent Composite since neighouring widgets that share the
+	 * same parent are affected. See bug 500703.
+	 */
+	if (GTK.GTK_VERSION >= OS.VERSION(3, 20, 0) && (style & SWT.READ_ONLY) != 0) {
+		if (parent != null && parent.fixClipHandle == 0) {
+			long /*int*/ parentHandle = GTK.gtk_widget_get_parent(fixedHandle);
+			if (parentHandle != 0) {
+				parent.fixClipHandle = parentHandle;
+				GTK.gtk_widget_queue_draw(parentHandle);
+				long [] /*int*/ array = {fixedHandle, handle, buttonBoxHandle, buttonHandle, cellBoxHandle, cellHandle};
+				parent.fixClipHandleChildren = new long /*int*/ [array.length];
+				System.arraycopy (array, 0, parent.fixClipHandleChildren, 0, array.length);
+			}
+		}
+	}
+	return super.gtk_draw(widget, cairo);
+}
+
+@Override
 long /*int*/ gtk_event_after (long /*int*/ widget, long /*int*/ gdkEvent)  {
 	/*
 	* Feature in GTK. Depending on where the user clicks, GTK prevents
@@ -1662,7 +1693,13 @@ void releaseHandle () {
 	if (buttonHandle != 0) {
 		OS.g_object_unref (buttonHandle);
 	}
-	menuHandle = buttonHandle = entryHandle = 0;
+	if (buttonBoxHandle != 0) {
+		OS.g_object_unref (buttonBoxHandle);
+	}
+	if (cellBoxHandle != 0) {
+		OS.g_object_unref (cellBoxHandle);
+	}
+	cellBoxHandle = buttonBoxHandle = menuHandle = buttonHandle = entryHandle = 0;
 }
 
 @Override
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Composite.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Composite.java
index bd253c43d7..ed35e96db8 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Composite.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Composite.java
@@ -65,6 +65,20 @@ public class Composite extends Scrollable {
 	Layout layout;
 	Control[] tabList;
 	int layoutCount, backgroundMode;
+	/**
+	 * When this field is set, it indicates that a child widget of this Composite
+	 * needs to have its clip set to its allocation. This is because on GTK3.20+
+	 * some widgets (like Combo) have their clips merged with that of their parent.
+	 */
+	long /*int*/ fixClipHandle;
+	/**
+	 * If fixClipHandle is set, fixClipArray can contain additional child widgets
+	 * that also need to have their clips adjusted.
+	 *
+	 * <p>The array <b>must</b> be ordered by widget hierarchy, as this array will be
+	 * traversed in-order to adjust the clipping of each element. See bug 500703.</p>
+	 */
+	long /*int*/ [] fixClipHandleChildren = {};
 
 	static final String NO_INPUT_METHOD = "org.eclipse.swt.internal.gtk.noInputMethod"; //$NON-NLS-1$
 
@@ -334,6 +348,53 @@ void createHandle (int index, boolean fixed, boolean scrolled) {
 	}
 }
 
+/**
+ * Iterates though the array of child widgets that need to have their clips
+ * adjusted: if a child has a negative clip, adjust it. Also check if the child's
+ * allocation is negative and adjust it as necessary.
+ *
+ * <p>If the array is empty this method just returns. See bug 500703.</p>
+ */
+void fixChildClippings () {
+	if (fixClipHandleChildren == null) {
+		return;
+	} else {
+		GtkRequisition minimumSize = new GtkRequisition ();
+		GtkRequisition naturalSize = new GtkRequisition ();
+		GtkAllocation clip = new GtkAllocation ();
+		GtkAllocation allocation = new GtkAllocation ();
+		for (long /*int*/ widget : fixClipHandleChildren) {
+			GTK.gtk_widget_get_allocation(widget, allocation);
+			GTK.gtk_widget_get_clip(widget, clip);
+			/*
+			 * If the clip is negative, add the x coordinate to the width
+			 * and set the x coordinate to 0.
+			 */
+			if (clip.x < 0) {
+				clip.width = clip.width + clip.x;
+				clip.x = 0;
+				/*
+				 * Some "transient" widgets like menus get allocations of
+				 * {-1, -1, 1, 1}. Check to make sure this isn't the case
+				 * before proceeding.
+				 */
+				if (allocation.x < -1 && (allocation.width > 1 || allocation.height > 1)) {
+					// Adjust the allocation just like the clip, if it's negative
+					allocation.width = allocation.width + allocation.x;
+					allocation.x = 0;
+					// Call gtk_widget_get_preferred_size() to prevent warnings
+					GTK.gtk_widget_get_preferred_size(widget, minimumSize, naturalSize);
+					// Allocate and queue a resize event
+					GTK.gtk_widget_size_allocate(widget, allocation);
+					GTK.gtk_widget_queue_resize(widget);
+				}
+			}
+			// Adjust the clip
+			GTK.gtk_widget_set_clip(widget, allocation);
+		}
+	}
+}
+
 @Override
 long /*int*/ gtk_draw (long /*int*/ widget, long /*int*/ cairo) {
 	if (GTK.GTK_VERSION >= OS.VERSION(3, 14, 0)) {
@@ -345,6 +406,11 @@ long /*int*/ gtk_draw (long /*int*/ widget, long /*int*/ cairo) {
 		// We specify a 0 value for x & y as we want the whole widget to be
 		// colored, not some portion of it.
 		GTK.gtk_render_background(context, cairo, 0, 0, width, height);
+		if (GTK.GTK_VERSION >= OS.VERSION(3, 20, 0)) {
+			// If fixClipHandle is set: iterate through the children of widget
+			// and set their clips to be that of their allocation
+			if (widget == fixClipHandle) fixChildClippings();
+		}
 	}
 	return super.gtk_draw(widget, cairo);
 }