Bug 510803 (TreeEditingRegressionTabfolder) - [GTK3] regression in editing capabilities (seen in change method refactoring wizard)
Summary: [GTK3] regression in editing capabilities (seen in change method refactoring ...
Status: VERIFIED FIXED
Alias: TreeEditingRegressionTabfolder
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 4.8   Edit
Hardware: PC Linux
: P2 major (vote)
Target Milestone: 4.8 M6   Edit
Assignee: Leo Ufimtsev CLA
QA Contact:
URL:
Whiteboard:
Keywords: regression
Depends on:
Blocks: 531051 514484 515973
  Show dependency tree
 
Reported: 2017-01-21 09:01 EST by Stephan Herrmann CLA
Modified: 2018-03-07 16:23 EST (History)
7 users (show)

See Also:


Attachments
affected wizard (screenshot) (41.87 KB, image/png)
2017-01-21 09:58 EST, Stephan Herrmann CLA
no flags Details
Modified jFaceSnippet 40 that reproduces issue (2.83 KB, text/plain)
2017-03-27 16:20 EDT, Leo Ufimtsev CLA
no flags Details
SWT snippet to reproduce issue (2.01 KB, text/plain)
2017-03-28 15:48 EDT, Leo Ufimtsev CLA
no flags Details
SWT snippet to reproduce issue #2 (2.79 KB, text/plain)
2017-04-03 09:42 EDT, Leo Ufimtsev CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stephan Herrmann CLA 2017-01-21 09:01:42 EST
JDT's wizard of the change method signature refactoring has some problems when run on GTK3, which are not present under GTK2:

(1) it is no longer possible to edit table entries in-line, one *must* use the Edit button & dialog

(2) accelerators almost never work, exception: an initial Alt-A works, any other / subsequent hotkeys are without effect

Since Alt-E is necessary due to (1) and impossible due to (2) this dialog is now dead for keyboard-only use.
Comment 1 Andrey Loskutov CLA 2017-01-21 09:16:17 EST
Stephan, you are on which GTK 3 version? rpm -q gtk3 or attach the swt system property from about dialog. Screenshot would be good too.
Comment 2 Stephan Herrmann CLA 2017-01-21 09:55:18 EST
The box is Kubuntu 16.04.1 LTS (hence no rmp :) )

So what properties might be of interest? At least these:

org.eclipse.swt.internal.gtk.theme=Breeze
org.eclipse.swt.internal.gtk.version=3.18.9

More gtk3 related versions from dpkg:
rc  appmenu-gtk3:i386                           0.3.92-0ubuntu1.1
ii  gtk3-engines-breeze                         5.5.5-0ubuntu1
ii  libcanberra-gtk3-0:i386                     0.30-2.1ubuntu1
ii  libcanberra-gtk3-module:i386                0.30-2.1ubuntu1
ii  libdbusmenu-gtk3-4:i386                     16.04.1+16.04.20160927-0ubuntu1
ii  libwxgtk3.0-0v5:i386                        3.0.2+dfsg-1.3
Comment 3 Stephan Herrmann CLA 2017-01-21 09:58:38 EST
Created attachment 266393 [details]
affected wizard (screenshot)

This is when double click and Alt-E both are without any effect.
Comment 4 Andrey Loskutov CLA 2017-01-21 10:35:10 EST
just windering if you tried with adwaita theme? Breeze is from KDE.
Comment 5 Stephan Herrmann CLA 2017-01-21 11:07:41 EST
(In reply to Andrey Loskutov from comment #4)
> just windering if you tried with adwaita theme?

I don't have adwaita. 

> Breeze is from KDE.

Right, since I'm on Kubuntu, having a KDE theme shouldn't surprise, should it?

IMHO, Breeze is the first theme in a long time that is acceptable for the combo of KDE with GTK3 and Eclipse ;p
Comment 6 Andrey Loskutov CLA 2017-01-21 12:08:08 EST
OK, I meant: could you try a GTK theme coming from Gnome (like Adwaita), not from KDE? I personally use Clearlooks Phenix and that one seem to work fine (but I have not yet tested your case).
Comment 7 Andrey Loskutov CLA 2017-01-24 11:18:41 EST
OK, I see same issue with gtk3-3.14.13-16.el7.x86_64 on RHEL 7.2 and Clearlooks-Phenix theme, in 4.6.2 and 4.7.latest.
Comment 8 Stephan Herrmann CLA 2017-01-24 11:52:08 EST
(In reply to Andrey Loskutov from comment #7)
> OK, I see same issue with gtk3-3.14.13-16.el7.x86_64 on RHEL 7.2 and
> Clearlooks-Phenix theme, in 4.6.2 and 4.7.latest.

thanks
Comment 9 Ian Pun CLA 2017-02-23 15:35:47 EST
It seems to me that the issue might be caused somewhere inside the JDT codebase. If you run Snippet018TableViewerAddRemoveColumnsWithEditingNewAPI.java in org.eclipse.jface.snippets the editing works fine in both GTK3 and 2. It might be some odd call to the tableviewer from within JDT that may not be using the correct tableviewer?
Comment 10 Alexander Kurtakov CLA 2017-02-27 05:11:05 EST
(In reply to Ian Pun from comment #9)
> It seems to me that the issue might be caused somewhere inside the JDT
> codebase. If you run
> Snippet018TableViewerAddRemoveColumnsWithEditingNewAPI.java in
> org.eclipse.jface.snippets the editing works fine in both GTK3 and 2. It
> might be some odd call to the tableviewer from within JDT that may not be
> using the correct tableviewer?

Ian, this definetely points to issue in SWT but feel free to propose patch to JDT to make use of more reliable paths while working on the swt one.
Comment 11 Leo Ufimtsev CLA 2017-03-27 13:17:30 EDT
So far I found that the issue seems to be related to TabFolder. The if the editable Table is inside the TabFolder, then upon clicking:

"(Eclipse:4971): Gdk-WARNING **: gdk_window_new(): parent is destroyed"

is printed into the console and no editing action is performed. If you take the editable table out of the TabFolder, then editing works again.

I think this is because TabFolder internally reparents widgets and something doesn't get reparented properly. I think it's linked to:
Bug 454936 – [GTK3] DND does not work on TabFolder 
https://bugs.eclipse.org/bugs/show_bug.cgi?id=454936

I haven't been able to reproduce the issue on a local snippet yet, but I'm working on it.
Comment 12 Leo Ufimtsev CLA 2017-03-27 16:20:19 EDT
Created attachment 267485 [details]
Modified jFaceSnippet 40 that reproduces issue

Update:
I modified the original jFace Snippet such that it reporduces the issue on Gtk3, but not on Gtk2. I.e, on Gtk2 the cells are editable.
To note is that the Table is put into a TabFolder and item.setControl() is set after the table and it's editing capabilities have been enabled.
Comment 13 Leo Ufimtsev CLA 2017-03-28 15:48:25 EDT
Created attachment 267508 [details]
SWT snippet to reproduce issue

I narrowed it down to a pure, small SWT snippet.

If you run it:
- Click on an item and start typing.
Gtk2: Characters are appended to item.
Gtk3: Error spam:
(SWT:24353): Gdk-WARNING **: gdk_window_new(): parent is destroyed  <<<<< NOTE.
(SWT:24353): Gdk-CRITICAL **: gdk_window_set_user_data: assertion 'GDK_IS_WINDOW (window)' failed.
....

Here we have:
Shell
|- TabFolder
   |- TabItem
      |- Table
          |- Text (editable, not shown. Input taken to update table content)

On Gtk3, the tabItem.setControl() re-parents widgets, but it looks like "Text" is not reparented properly (or something isn't anyway), and so we get a lot of warning about parent being destroyed.

Gotta investigate further.
Comment 14 Leo Ufimtsev CLA 2017-04-03 09:42:20 EDT
Created attachment 267609 [details]
SWT snippet to reproduce issue #2

Update to original snippet
Comment 15 Ian Pun CLA 2017-06-20 14:57:56 EDT
Just an update, it looks like the fix done in Bug 454936 actually causes the spam of SWT errors when dealing with the position of setControl(). Removing it, however, still doesn't fix the full issue on Platform
Comment 16 Ian Pun CLA 2017-06-21 10:27:32 EDT
I've talked to Leo about this issue and it seems  related to how reparent was done in order to get DnD to work in TabFolder ( Bug 454936 ). He will be working on this once his queue clears up a bit.
Comment 17 Stephan Herrmann CLA 2017-11-20 16:14:15 EST
Would you mind adding this to bug 516839, so it doesn't get forgotten?
Comment 19 Leo Ufimtsev CLA 2018-02-12 09:45:13 EST
Status update:

Commenting out:

Tree.java
@Override
void setParentWindow (long /*int*/ widget) {
	long /*int*/ window = eventWindow ();
	GTK.gtk_widget_set_parent_window (widget, window);
}

Seems to fix the problem. Seems like TabItem re-parents and the above breaks part of the re-parenting.
Will continue to investigate.
Comment 20 Leo Ufimtsev CLA 2018-02-13 09:25:37 EST
Note:

> void setParentWindow (long /*int*/ widget) {
> 	long /*int*/ window = eventWindow ();
> 	GTK.gtk_widget_set_parent_window (widget, window);
> }

This was added because of these 2:
73811 – TableEditor overwrites table header and scrollbars (gtk)
https://bugs.eclipse.org/bugs/show_bug.cgi?id=73811

124481 – TreeEditor: ProgressBar does not fill cell correctly
https://bugs.eclipse.org/bugs/show_bug.cgi?id=124481