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