From 1a3294ba5966b01cc2953d4ad12ed02eb971b97f Mon Sep 17 00:00:00 2001

From: Leo Ufimtsev

Date: Wed, 14 Feb 2018 10:49:01 -0500

Subject: Bug 510803 [GTK3] Regression in table editing capabilities in

 tabfolder.

Breakage occurred because tabItem reparented an item that had a

non-standard gDk parent window.

Solution: fix gdk window parent for such special controls.

(I.e ControlEditors in tables).

Verification:

- Open child eclipse, change method signature.

  Now table cells can be edited via click.

- Open attached snippet, with patch cells can be edited properly.

- AllTests Gtk3.22

- X11/Wayland.

Patchset 3:

- Minor rename of methods to clarify that it's a *gdk* window,

Bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=5108033

Change-Id: I8dcaac950eb0847dd97016b2140c607012550d2f

Signed-off-by: Leo Ufimtsev <lufimtse@redhat.com>

---

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

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

 .../gtk/org/eclipse/swt/widgets/Composite.java     |  10 ++

 .../gtk/org/eclipse/swt/widgets/Control.java       |  51 ++++++++-

 .../gtk/org/eclipse/swt/widgets/ExpandItem.java    |   2 +-

 .../gtk/org/eclipse/swt/widgets/TabItem.java       |   4 +-

 .../gtk/org/eclipse/swt/widgets/Table.java         |   6 +-

 .../gtk/org/eclipse/swt/widgets/Tree.java          |   6 +-

 .../gtk/org/eclipse/swt/widgets/Widget.java        |  15 ---

 .../Bug510803_TabFolder_Table_inPlaceEditing.java  | 108 ------------------

 .../Bug510803_TabFolder_TreeEditor_Regression.java | 124 +++++++++++++++++++++

 11 files changed, 203 insertions(+), 135 deletions(-)

 delete mode 100644 tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug510803_TabFolder_Table_inPlaceEditing.java

 create mode 100644 tests/org.eclipse.swt.tests.gtk/ManualTests/org/eclipse/swt/tests/gtk/snippets/Bug510803_TabFolder_TreeEditor_Regression.java

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

@@ -16764,7 +16764,15 @@ JNIEXPORT void JNICALL OS_NATIVE(_1gtk_1widget_1reparent)

 	(JNIEnv *env, jclass that, jintLong arg0, jintLong arg1)

 {

 	OS_NATIVE_ENTER(env, that, _1gtk_1widget_1reparent_FUNC);

+/*

 	gtk_widget_reparent((GtkWidget *)arg0, (GtkWidget *)arg1);

+*/

+	{

+		OS_LOAD_FUNCTION(fp, gtk_widget_reparent)

+		if (fp) {

+			((void (CALLING_CONVENTION*)(GtkWidget *, GtkWidget *))fp)((GtkWidget *)arg0, (GtkWidget *)arg1);

+		}

+	}

 	OS_NATIVE_EXIT(env, that, _1gtk_1widget_1reparent_FUNC);

 }

 #endif

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 2619114..57bbf52 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	

@@ -506,6 +506,16 @@ void fixChildren (Shell newShell, Shell oldShell, Decorations newDecorations, De

 }

 @Override

+void fixParentGdkWindow() {

+	assert OS.GTK3;

+	// Changes to this method should be verified via

+	// org.eclipse.swt.tests.gtk/*/Bug510803_TabFolder_TreeEditor_Regression.java (part two)

+	for (Control child : _getChildren()) {

+		child.fixParentGdkWindow();

+	}

+}

+

+@Override

 void fixModal(long /*int*/ group, long /*int*/ modalGroup)  {

 	Control[] controls = _getChildren ();

 	for (int i = 0; i < controls.length; i++) {

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 e29d144..b0768d5 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	

@@ -2656,6 +2656,51 @@ void fixChildren (Shell newShell, Shell oldShell, Decorations newDecorations, De

 	oldDecorations.fixDecorations (newDecorations, this, menus);

 }

+/**

+ * In some situations, a control has a non-standard parent GdkWindow (Note gDk, not gTk).

+ * E.g, an TreeEditor who's parent is a Tree should have the Tree Viewer's inner bin as parent window.

+ *

+ * Note, composites should treat this differently and take child controls into consideration.

+ */

+void fixParentGdkWindow() {

+	assert OS.GTK3;

+	// Changes to this method should be verified via

+	// org.eclipse.swt.tests.gtk/*/Bug510803_TabFolder_TreeEditor_Regression.java (part one)

+	parent.setParentGdkWindow(this);

+}

+

+/**

+ * Native gtkwidget re-parenting in SWT on Gtk3 needs to be handled in a special way because

+ * some controls have non-standard GdkWindow as parents. (E.g ControlEditors), and other controls

+ * like TabItem and ExpandBar use reparenting to preserve proper hierarchy for correct event traversal (like dnd).

+ *

+ * Note, GdkWindows != GtkWindows.

+ *

+ * You should never call gtk_widget_reparent() directly or reparent widgets outside this method,

+ * otherwise you can break TabItem/TreeEditors.

+ *

+ * @param control that should be reparented.

+ * @param newParentHandle pointer/handle to the new GtkWidget parent.

+ */

+static void gtk_widget_reparent (Control control, long /*int*/ newParentHandle) {

+	if (OS.GTK3) {

+		// Changes to this method should be verified via both parts in:

+		// org.eclipse.swt.tests.gtk/*/Bug510803_TabFolder_TreeEditor_Regression.java

+		long /*int*/ widget = control.topHandle();

+		long /*int*/ parentContainer = OS.gtk_widget_get_parent (widget);

+		assert parentContainer != 0 : "Improper use of Control.gtk_widget_reparent. Widget currently has no parent.";

+		if (parentContainer != 0) {

+			OS.g_object_ref (widget); //so that it won't get destroyed due to lack of references.

+			OS.gtk_container_remove (parentContainer, widget);

+			OS.gtk_container_add (newParentHandle, widget);

+			OS.g_object_unref (widget);

+			control.fixParentGdkWindow();

+		}

+	} else { // Gtk2.

+		OS.gtk_widget_reparent(control.topHandle(), newParentHandle);

+	}

+}

+

 @Override

 long /*int*/ fixedMapProc (long /*int*/ widget) {

 	OS.gtk_widget_set_mapped (widget, true);

@@ -5254,7 +5299,7 @@ public boolean setParent (Composite parent) {

 		oldDecorations.fixAccelGroup ();

 	}

 	long /*int*/ newParent = parent.parentingHandle();

-	OS.gtk_widget_reparent(topHandle, newParent);

+	gtk_widget_reparent(this, newParent);

 	if (OS.GTK3) {

 		OS.swt_fixed_move (newParent, topHandle, x, y);

 	} else {

@@ -5287,7 +5332,7 @@ void setParentBackground () {

 	if (fixedHandle != 0) setBackgroundColor (fixedHandle, null);

 }

-void setParentWindow (Control child) {

+void setParentGdkWindow (Control child) {

 }

 boolean setRadioSelection (boolean value) {

@@ -5697,7 +5742,7 @@ void showWidget () {

 	state |= ZERO_WIDTH | ZERO_HEIGHT;

 	long /*int*/ topHandle = topHandle ();

 	long /*int*/ parentHandle = parent.parentingHandle ();

-	parent.setParentWindow (this);

+	parent.setParentGdkWindow (this);

 	OS.gtk_container_add (parentHandle, topHandle);

 	if (handle != 0 && handle != topHandle) OS.gtk_widget_show (handle);

 	if ((state & (ZERO_WIDTH | ZERO_HEIGHT)) == 0) {

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ExpandItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ExpandItem.java

index fcfbd33..6e48823 100644

--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ExpandItem.java	

+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ExpandItem.java	

@@ -525,7 +525,7 @@ public void setControl (Control control) {

 			//As ExpandItem's child can be created before the ExpandItem, our only

 			//option is to reparent the child upon the setControl(..) call.

 			//This is simmilar to TabFolder.

-			gtk_widget_reparent (control.topHandle (), clientHandle ());

+			Control.gtk_widget_reparent (control, clientHandle ());

 		}

 	}

 	parent.layoutItems (0, true);

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TabItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TabItem.java

index 208a373..ce9b4cc 100644

--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TabItem.java	

+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/TabItem.java	

@@ -293,8 +293,8 @@ public void setControl (Control control) {

 	}

 	if (control != null && OS.GTK3) {

-		// See implementation note about bug 454936 at the start of TabFolder.

-		OS.gtk_widget_reparent (control.topHandle (), pageHandle);

+		// To understand why we reparent, see implementation note about bug 454936 at the start of TabFolder.

+		Control.gtk_widget_reparent (control, pageHandle);

 	}

 	Control oldControl = this.control, newControl = control;

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java

index ccf01ec..c572e51 100644

--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java	

+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Table.java	

@@ -3649,9 +3649,9 @@ void setParentBackground () {

 }

 @Override

-void setParentWindow (Control child) {

-	long /*int*/ window = eventWindow ();

-	OS.gtk_widget_set_parent_window (child.topHandle(), window);

+void setParentGdkWindow (Control child) {

+	long /*int*/ parentGdkWindow = eventWindow ();

+	OS.gtk_widget_set_parent_window (child.topHandle(), parentGdkWindow);

 	/*

 	 * Feature in GTK3: all children of Table have their GdkWindows

 	 * re-parented so they are siblings of the parent Table

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java

index 86b72a0..50de17c 100644

--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java	

+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Tree.java	

@@ -3654,9 +3654,9 @@ void setParentBackground () {

 }

 @Override

-void setParentWindow (Control child) {

-	long /*int*/ window = eventWindow ();

-	OS.gtk_widget_set_parent_window (child.topHandle(), window);

+void setParentGdkWindow (Control child) {

+	long /*int*/ parentGdkWindow = eventWindow ();

+	OS.gtk_widget_set_parent_window (child.topHandle(), parentGdkWindow);

 	/*

 	 * Feature in GTK3: all children of Tree have their GdkWindows

 	 * re-parented so they are siblings of the parent Tree

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Widget.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Widget.java

index 40fe88c..2da1da5 100644

--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Widget.java	

+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/Widget.java	

@@ -936,21 +936,6 @@ long /*int*/ gtk_value_changed (long /*int*/ adjustment) {

 	return 0;

 }

-/**

- * Reparent on gtk side.

- * Note: Composites with a setControl() function should probably use this function

- * to correct hierarchy on gtk side.

- * @param widget  the handle to the widget. (usually topHandle())

- * @param newParent  handle to the new widget.

- */

-void gtk_widget_reparent (long /*int*/ widget, long /*int*/ newParent) {

-	//Note, we do not actually call  * 'gtk_widget_reparent(...) as it's deprecated as of gtk 3.14

-	OS.g_object_ref (widget); //so that it won't get destroyed due to lack of references.

-	OS.gtk_container_remove (OS.gtk_widget_get_parent (widget), widget);

-	OS.gtk_container_add (newParent, widget);

-	OS.g_object_unref (widget);

-}

-

 long /*int*/ gtk_window_state_event (long /*int*/ widget, long /*int*/ event) {

 	return 0;

 }