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;
}