Bug 539367: [GTK3] Long text in Combo drawn over drop-down button and

outside of combo

Combos have a GtkCellView, which renders the text, and other icons. by
default, GtkCellView has a "fit-model" property which is set to true.
This means the content in the cell view will always expand to the
maximum size. Setting it to false allows us to resize the cell view as
desired.

The fix for this bug is to set the clipping of the cell view to a
smaller size: we find the x position of the drop down icon, minus its
width. This stops the text from being drawn over the drop down icon, and
sometimes over other widgets.

Tested on GTK3.22 and 3.20, all other versions of GTK3 are unaffected.
The snippets in this patch do not show the bug, nor does the one for bug
500703. Additionally, the keys preferences page does not exhibit any
issues either. No AllNonBrowser JUnit tests fail.

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

9 files changed, 256 insertions, 37 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 727f91fcd8..d01c9eea69 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
@@ -3850,6 +3850,16 @@ JNIEXPORT jintLong JNICALL GTK_NATIVE(_1gtk_1cell_1renderer_1toggle_1new)
 }
 #endif
 
+#ifndef NO__1gtk_1cell_1view_1set_1fit_1model
+JNIEXPORT void JNICALL GTK_NATIVE(_1gtk_1cell_1view_1set_1fit_1model)
+	(JNIEnv *env, jclass that, jintLong arg0, jboolean arg1)
+{
+	GTK_NATIVE_ENTER(env, that, _1gtk_1cell_1view_1set_1fit_1model_FUNC);
+	gtk_cell_view_set_fit_model((GtkCellView *)arg0, (gboolean)arg1);
+	GTK_NATIVE_EXIT(env, that, _1gtk_1cell_1view_1set_1fit_1model_FUNC);
+}
+#endif
+
 #ifndef NO__1gtk_1check_1button_1new
 JNIEXPORT jintLong JNICALL GTK_NATIVE(_1gtk_1check_1button_1new)
 	(JNIEnv *env, jclass that)
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 9edd81f826..5113c8cacb 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
@@ -312,6 +312,7 @@ char * GTK_nativeFunctionNames[] = {
 	"_1gtk_1cell_1renderer_1set_1fixed_1size",
 	"_1gtk_1cell_1renderer_1text_1new",
 	"_1gtk_1cell_1renderer_1toggle_1new",
+	"_1gtk_1cell_1view_1set_1fit_1model",
 	"_1gtk_1check_1button_1new",
 	"_1gtk_1check_1menu_1item_1get_1active",
 	"_1gtk_1check_1menu_1item_1new",
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 1dadd780f8..c32d15e0ef 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
@@ -310,6 +310,7 @@ typedef enum {
 	_1gtk_1cell_1renderer_1set_1fixed_1size_FUNC,
 	_1gtk_1cell_1renderer_1text_1new_FUNC,
 	_1gtk_1cell_1renderer_1toggle_1new_FUNC,
+	_1gtk_1cell_1view_1set_1fit_1model_FUNC,
 	_1gtk_1check_1button_1new_FUNC,
 	_1gtk_1check_1menu_1item_1get_1active_FUNC,
 	_1gtk_1check_1menu_1item_1new_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 cbf5b0bd34..7fbb11324d 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
@@ -1082,6 +1082,19 @@ public class GTK extends OS {
 			lock.unlock();
 		}
 	}
+	/**
+	 * @param cell_view cast=(GtkCellView *)
+	 * @param fit_model cast=(gboolean)
+	 */
+	public static final native void _gtk_cell_view_set_fit_model(long /*int*/ cell_view, boolean fit_model);
+	public static final void gtk_cell_view_set_fit_model(long /*int*/ cell_view, boolean fit_model) {
+		lock.lock();
+		try {
+			_gtk_cell_view_set_fit_model(cell_view, fit_model);
+		} finally {
+			lock.unlock();
+		}
+	}
 	public static final native long /*int*/ _gtk_check_button_new();
 	public static final long /*int*/ gtk_check_button_new() {
 		lock.lock();
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 fffaa4331c..4f8d2302ae 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
@@ -61,7 +61,8 @@ 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, buttonBoxHandle, cellBoxHandle;
+	long /*int*/ buttonHandle, entryHandle, textRenderer, cellHandle, popupHandle, menuHandle,
+		buttonBoxHandle, cellBoxHandle, arrowHandle;
 	int lastEventTime, visibleCount = 10;
 	long /*int*/ imContext;
 	long /*int*/ gdkEventKey = 0;
@@ -72,6 +73,7 @@ public class Combo extends Composite {
 	String cssButtonBackground, cssButtonForeground = " ";
 	long /*int*/ buttonProvider;
 	boolean firstDraw = true;
+	boolean unselected = true, fitModelToggled = false;
 	/**
 	 * the operating system limit for the number of characters
 	 * that the text field in an instance of this class can hold
@@ -539,6 +541,13 @@ void createHandle (int index) {
 	if ((style & SWT.READ_ONLY) != 0 && buttonHandle != 0) {
 		GTK.gtk_widget_set_receives_default (buttonHandle, false);
 	}
+	/*
+	 * Find the arrowHandle, which is the handle belonging to the GtkIcon
+	 * drop down arrow. See bug 539367.
+	 */
+	if ((style & SWT.READ_ONLY) != 0 && GTK.GTK_VERSION >= OS.VERSION(3, 20, 0)) {
+		if (cellBoxHandle != 0) arrowHandle = findArrowHandle();
+	}
 	// In GTK 3 font description is inherited from parent widget which is not how SWT has always worked,
 	// reset to default font to get the usual behavior
 	setFontDescription(defaultFont().handle);
@@ -689,6 +698,31 @@ long /*int*/ findButtonHandle() {
 	return result;
 }
 
+long /*int*/ findArrowHandle() {
+	long /*int*/ result = 0;
+	if (GTK.GTK_VERSION >= OS.VERSION(3, 20, 0) && cellBoxHandle != 0) {
+		GTK.gtk_container_forall (cellBoxHandle, display.allChildrenProc, 0);
+		if (display.allChildren != 0) {
+			long /*int*/ list = display.allChildren;
+			while (list != 0) {
+				long /*int*/ widget = OS.g_list_data (list);
+				/*
+				 * Feature in GTK: GtkIcon isn't public, so we have to do
+				 * type lookups using gtk_widget_get_name(). See bug 539367.
+				 */
+				String name = display.gtk_widget_get_name(widget);
+				if (name != null && name.contains("GtkIcon")) {
+					result = widget;
+				}
+				list = OS.g_list_next (list);
+			}
+			OS.g_list_free (display.allChildren);
+			display.allChildren = 0;
+		}
+	}
+	return result;
+}
+
 long /*int*/ findMenuHandle() {
 	if (popupHandle == 0) return 0;
 	long /*int*/ result = 0;
@@ -1267,6 +1301,7 @@ long /*int*/ gtk_button_press_event (long /*int*/ widget, long /*int*/ event) {
 @Override
 long /*int*/ gtk_changed (long /*int*/ widget) {
 	if (widget == handle) {
+		unselected = false;
 		if (entryHandle == 0) {
 			sendEvent(SWT.Modify);
 			if (isDisposed ()) return 0;
@@ -1380,6 +1415,39 @@ long /*int*/ gtk_delete_text (long /*int*/ widget, long /*int*/ start_pos, long
 }
 
 @Override
+void adjustChildClipping (long /*int*/ widget) {
+	/*
+	 * When adjusting the GtkCellView's clip, take into account
+	 * the position of the "arrow" icon. We set the clip of the
+	 * GtkCellView to the icon's position, minus the icon's width.
+	 *
+	 * This ensures the text never draws longer than the Combo itself.
+	 * See bug 539367.
+	 */
+	if (widget == cellHandle && (style & SWT.READ_ONLY) != 0 && GTK.GTK_VERSION >= OS.VERSION(3, 20, 0) && !unselected) {
+		/*
+		 * Set "fit-model" mode for READ_ONLY Combos on GTK3.20+ to false.
+		 * This means the GtkCellView rendering the text can be set to
+		 * a size other than the maximum. See bug 539367.
+		 */
+		if (!fitModelToggled) {
+			GTK.gtk_cell_view_set_fit_model(cellHandle, false);
+			fitModelToggled = true;
+		}
+		GtkAllocation iconAllocation = new GtkAllocation ();
+		GTK.gtk_widget_get_allocation(arrowHandle, iconAllocation);
+		GtkAllocation cellViewAllocation = new GtkAllocation ();
+		GTK.gtk_widget_get_allocation(cellHandle, cellViewAllocation);
+
+		cellViewAllocation.width = (iconAllocation.x - iconAllocation.width);
+		GTK.gtk_widget_set_clip(widget, cellViewAllocation);
+		return;
+	} else {
+		super.adjustChildClipping(widget);
+	}
+}
+
+@Override
 long /*int*/ gtk_draw (long /*int*/ widget, long /*int*/ cairo) {
 	/*
 	 * Feature in GTK3.20+: Combos have their clip unioned
@@ -1581,6 +1649,7 @@ long /*int*/ gtk_populate_popup (long /*int*/ widget, long /*int*/ menu) {
 @Override
 long /*int*/ gtk_selection_done(long /*int*/ menushell) {
 	int index = GTK.gtk_combo_box_get_active (handle);
+	unselected = false;
 	if (indexSelected == -1){
 		indexSelected = index;
 	}
@@ -1973,6 +2042,7 @@ public void select (int index) {
 		*/
 		sendEvent (SWT.Modify);
 	}
+	unselected = false;
 }
 
 void setButtonBackgroundGdkRGBA (GdkRGBA rgba) {
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 fe9bd61d73..77255563f1 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
@@ -358,51 +358,23 @@ 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.
+ * adjusted, and calls Control.adjustChildClipping() on it.
  *
- * <p>If the array is empty this method just returns. See bug 500703.</p>
+ * The default implementation in Composite is: 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, and 539367.</p>
  */
-void fixChildClippings () {
+void fixClippings () {
 	if (fixClipHandle == 0 || fixClipMap.isEmpty()) {
 		return;
 	} else {
-		GtkRequisition minimumSize = new GtkRequisition ();
-		GtkRequisition naturalSize = new GtkRequisition ();
-		GtkAllocation clip = new GtkAllocation ();
-		GtkAllocation allocation = new GtkAllocation ();
 		Control [] children = _getChildren();
 		for (Control child : children) {
 			if (fixClipMap.containsKey(child)) {
 				long /*int*/ [] childHandles = fixClipMap.get(child);
 				for (long /*int*/ widget : childHandles) {
-					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);
+					child.adjustChildClipping(widget);
 				}
 			}
 		}
@@ -410,6 +382,39 @@ void fixChildClippings () {
 }
 
 @Override
+void adjustChildClipping (long /*int*/ widget) {
+	GtkRequisition minimumSize = new GtkRequisition ();
+	GtkRequisition naturalSize = new GtkRequisition ();
+	GtkAllocation clip = new GtkAllocation ();
+	GtkAllocation allocation = new GtkAllocation ();
+	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) {
+		/*
+		 * 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)) {
 		long /*int*/ context = GTK.gtk_widget_get_style_context(widget);
@@ -423,7 +428,7 @@ long /*int*/ gtk_draw (long /*int*/ widget, long /*int*/ cairo) {
 		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();
+			if (widget == fixClipHandle) fixClippings();
 		}
 	}
 	return super.gtk_draw(widget, cairo);
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Control.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Control.java
index d4937c0c40..37798b73c6 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Control.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Control.java
@@ -1922,6 +1922,15 @@ public void addPaintListener(PaintListener listener) {
 	addListener(SWT.Paint,typedListener);
 }
 
+/**
+ * Allows Controls to adjust the clipping of themselves or
+ * their children.
+ *
+ * @param widget the handle to the widget
+ */
+void adjustChildClipping (long /*int*/ widget) {
+}
+
 void addRelation (Control control) {
 }
 
diff --git a/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug539367_ComboLongText.java b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug539367_ComboLongText.java
new file mode 100644
index 0000000000..eb4b44fe41
--- /dev/null
+++ b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug539367_ComboLongText.java
@@ -0,0 +1,53 @@
+/*******************************************************************************
+ * Copyright (c) 2018 Simeon Andreev and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     Simeon Andreev - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.swt.tests.gtk.snippets;
+
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.layout.FillLayout;
+import org.eclipse.swt.layout.GridData;
+import org.eclipse.swt.layout.GridLayout;
+import org.eclipse.swt.widgets.Combo;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Shell;
+
+public class Bug539367_ComboLongText {
+
+	public static void main(String[] args) {
+		Display display = new Display();
+		Shell shell = new Shell(display);
+		shell.setSize(400, 150);
+		shell.setText("Bug combo text");
+		shell.setLayout(new FillLayout());
+
+		Composite parent = new Composite(shell, SWT.BORDER);
+		parent.setLayout(new GridLayout());
+
+		Combo combo = new Combo(parent, SWT.READ_ONLY | SWT.DROP_DOWN);
+		GridData comboGridData = new GridData(SWT.FILL, SWT.CENTER, true, false, 1, 1);
+		combo.setLayoutData(comboGridData);
+		combo.add("some long long long long combo text");
+		combo.select(0);
+
+		new Composite(shell, SWT.BORDER);
+
+		shell.open();
+
+		while (!shell.isDisposed()) {
+			if (!display.readAndDispatch())
+				display.sleep();
+		}
+		display.dispose();
+	}
+}
\ No newline at end of file
diff --git a/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug539367_ComboLongText2.java b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug539367_ComboLongText2.java
new file mode 100644
index 0000000000..bbfdf9183e
--- /dev/null
+++ b/tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug539367_ComboLongText2.java
@@ -0,0 +1,57 @@
+/*******************************************************************************
+ * Copyright (c) 2018 Simeon Andreev and others.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *     Simeon Andreev - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.swt.tests.gtk.snippets;
+
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.layout.FillLayout;
+import org.eclipse.swt.layout.GridData;
+import org.eclipse.swt.layout.GridLayout;
+import org.eclipse.swt.widgets.Combo;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.swt.widgets.Text;
+
+public class Bug539367_ComboLongText2 {
+
+	public static void main(String[] args) {
+		Display display = new Display();
+		Shell shell = new Shell(display);
+		shell.setSize(250, 150);
+		shell.setText("Bug combo text");
+		shell.setLayout(new FillLayout());
+
+		Composite parent = new Composite(shell, SWT.BORDER);
+		parent.setLayout(new GridLayout());
+
+		Combo combo = new Combo(shell, SWT.READ_ONLY | SWT.DROP_DOWN);
+		GridData comboGridData = new GridData(SWT.FILL, SWT.CENTER, true, false, 1, 1);
+		combo.setLayoutData(comboGridData);
+		combo.add("some long long long long combo text");
+		combo.select(0);
+
+		Composite lastComposite = new Composite(shell, SWT.BORDER);
+		lastComposite.setLayout(new FillLayout());
+		Text text = new Text(lastComposite, SWT.NONE);
+		text.setText("my text");
+
+		shell.open();
+
+		while (!shell.isDisposed()) {
+			if (!display.readAndDispatch())
+				display.sleep();
+		}
+		display.dispose();
+	}
+}
\ No newline at end of file