Community
Participate
Working Groups
Created attachment 284677 [details] Video showing the problem in a recent 4.18 build. To reproduce, refactor extract code to a method. Observe the Java editor can be typed in, but doesn't respond to commands such as save, undo and so on (as its no active. See attached video "extract_method_refactor_inacive_editor.mp4". The video was made in a new workspace, but with theming disabled (as the inactive editor tab still *appears* active with theming enabled, while not actually being active). This makes extracting methods with refactor very clunky. I see this in: Eclipse SDK Version: 2020-12 (4.18) Build id: I20201103-0030 Eclipse SDK Version: 2020-09 (4.17) Build id: I20200823-1800 But not in: Eclipse SDK Version: 2020-06 (4.16) Build id: I20200505-1800
Created attachment 284678 [details] Video showing expected behaviour with Eclipse 4.16.
Looks really bad. I can confirm I see that if I do exact steps in the video. However, I can't reproduce it *if* I don't select proposal with the mouse, but use keyboard for selection / navigation in the shell. So I assume that is about shell activation/deactivation events or mouse listener that interfere here and steal focus out from the active editor shell. Could also be SWT issue (I've added SWT to the CC). Could you also see this on Windows?
I also sometimes can't type after the refactor method extract, I'm not sure under which conditions this occurs.
Earliest 4.17 I have is: Eclipse SDK Version: 2020-09 (4.17) Build id: I20200622-1800 The problem is reproducible in it. I don't have a 4.16 later than the build from 20200505. So this is roughly the period in which the problem was introduced.
(In reply to Simeon Andreev from comment #4) > Earliest 4.17 I have is: > > Eclipse SDK > Version: 2020-09 (4.17) > Build id: I20200622-1800 > > The problem is reproducible in it. I don't have a 4.16 later than the build > from 20200505. So this is roughly the period in which the problem was > introduced. You can find some older 4.16 / 4.17 I builds on my socbm599 workstation in /data/eclipse4.16/ & /data/eclipse4.17/. Feel free to bisect.
Problem seen in: Eclipse SDK Version: 2020-09 (4.17) Build id: I20200609-1800 Problem not seen in: Eclipse SDK Version: 2020-09 (4.17) Build id: I20200609-0150
(In reply to Simeon Andreev from comment #6) > Problem seen in: > > Eclipse SDK > Version: 2020-09 (4.17) > Build id: I20200609-1800 > > Problem not seen in: > > Eclipse SDK > Version: 2020-09 (4.17) > Build id: I20200609-0150 Cool. That leads me to bug 563555.
I tend to revert https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/163633 for M3 as this has more severe regression as the bug it was supposed to fix.
There has also been a regression in Bug 565674, which turned out to be an Eclipse bug. It's rather unfortunate how many bugs got tied together and fixing one bug caused other to start causing problems. Still, in my understanding, it would be better to fix all bugs rather then restoring a bug so that other bugs stay low.
(In reply to Alexandr Miloslavskiy from comment #9) > Still, in my > understanding, it would be better to fix all bugs rather then restoring a > bug so that other bugs stay low. Sure, but it is just matter of available resources & regression severity balance. I've verified that revert fixes this issue. Now we know where the regression is, could you please check if you can provide another patch for bug 563555 that doesn't cause this one? If you could provide that before M3, we don't need a full revert. If not, we should revert & re-open bug 563555. Revert gerrit is here: https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/171810
Most likely, patch for bug 563555 is just fine and some other bug in Eclipse needs to be fixed instead. I'm pretty busy with all sorts of other things here, I will be grateful if someone figured which code in Eclipse is responsible for the problem. It's likely related to a wrong .getActiveShell() somewhere.
Only this call to Display.getActiveShell() seems somewhat related: at org.eclipse.swt.widgets.Display.getActiveShell(Display.java:1684) at org.eclipse.jface.text.contentassist.CompletionProposalPopup.hasFocus(CompletionProposalPopup.java:1049) at org.eclipse.jface.text.contentassist.ContentAssistant$Closer.lambda$0(ContentAssistant.java:219) The refactoring change itself is performed on this stack trace: at org.eclipse.jdt.ui.text.java.correction.ChangeCorrectionProposal.performChange(ChangeCorrectionProposal.java:180) at org.eclipse.jdt.internal.ui.text.correction.proposals.LinkedCorrectionProposal.performChange(LinkedCorrectionProposal.java:152) at org.eclipse.jdt.ui.text.java.correction.CUCorrectionProposal.apply(CUCorrectionProposal.java:167) at org.eclipse.jface.text.contentassist.CompletionProposalPopup.insertProposal(CompletionProposalPopup.java:1002) at org.eclipse.jface.text.contentassist.CompletionProposalPopup.insertSelectedProposalWithMask(CompletionProposalPopup.java:946) at org.eclipse.jface.text.contentassist.CompletionProposalPopup.access$7(CompletionProposalPopup.java:942) at org.eclipse.jface.text.contentassist.CompletionProposalPopup$3.widgetDefaultSelected(CompletionProposalPopup.java:695) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:123) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5785) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1427) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:5048) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4526) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1157) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1046) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155) at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:644) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:551) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:156) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:401) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:653) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:590) at org.eclipse.equinox.launcher.Main.run(Main.java:1461) at org.eclipse.equinox.launcher.Main.main(Main.java:1434) I'm not seeing anything odd on first glance.
Thanks, the stack is already helpful. Glancing at code (don't have access to debugger atm), I suspect that 'CompletionProposalPopup#fProposalShell' is parented to incorrect Shell. Later, in 'CompletionProposalPopup#hide()', 'fProposalShell' is disposed, which returns focus to wrong Shell. Watching the video, I noticed that after popup is clicked, editor tab becomes gray (inactive).
CompletionProposalPopup.fProposalShell is set in CompletionProposalPopup.createProposalSelector(), from what I can tell to the Eclipse window shell itself: Shell {contributor_runtime_eclipse_jre11_clean_ws - Test/src/test/Test.java - Eclipse SDK} at org.eclipse.jface.text.contentassist.CompletionProposalPopup.createProposalSelector(CompletionProposalPopup.java:605) at org.eclipse.jface.text.contentassist.CompletionProposalPopup.lambda$0(CompletionProposalPopup.java:515) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:74) at org.eclipse.jface.text.contentassist.CompletionProposalPopup.showProposals(CompletionProposalPopup.java:500) at org.eclipse.jface.text.contentassist.ContentAssistant.showPossibleCompletions(ContentAssistant.java:1827) at org.eclipse.jface.text.quickassist.QuickAssistAssistant.showPossibleQuickAssists(QuickAssistAssistant.java:113) at org.eclipse.jdt.internal.ui.text.correction.JavaCorrectionAssistant.showPossibleQuickAssists(JavaCorrectionAssistant.java:194) at org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitEditor$AdaptedSourceViewer.doOperation(CompilationUnitEditor.java:200) at org.eclipse.ui.texteditor.TextOperationAction.lambda$0(TextOperationAction.java:130) at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:74) at org.eclipse.ui.texteditor.TextOperationAction.run(TextOperationAction.java:130) at org.eclipse.jface.action.Action.runWithEvent(Action.java:474) at org.eclipse.jface.commands.ActionHandler.execute(ActionHandler.java:121) at org.eclipse.ui.internal.handlers.E4HandlerProxy.execute(E4HandlerProxy.java:97) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:58) at org.eclipse.e4.core.internal.di.InjectorImpl.invokeUsingClass(InjectorImpl.java:319) at org.eclipse.e4.core.internal.di.InjectorImpl.invoke(InjectorImpl.java:253) at org.eclipse.e4.core.contexts.ContextInjectionFactory.invoke(ContextInjectionFactory.java:173) at org.eclipse.e4.core.commands.internal.HandlerServiceHandler.execute(HandlerServiceHandler.java:156) at org.eclipse.core.commands.Command.executeWithChecks(Command.java:488) at org.eclipse.core.commands.ParameterizedCommand.executeWithChecks(ParameterizedCommand.java:487) at org.eclipse.e4.core.commands.internal.HandlerServiceImpl.executeHandler(HandlerServiceImpl.java:213) at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.executeCommand(KeyBindingDispatcher.java:308) at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.press(KeyBindingDispatcher.java:584) at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.processKeyEvent(KeyBindingDispatcher.java:653) at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.filterKeySequenceBindings(KeyBindingDispatcher.java:443) at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher.access$2(KeyBindingDispatcher.java:386) at org.eclipse.e4.ui.bindings.keys.KeyBindingDispatcher$KeyDownFilter.handleEvent(KeyBindingDispatcher.java:96) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89) at org.eclipse.swt.widgets.Display.filterEvent(Display.java:1890) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1426) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1453) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1436) at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:1475) at org.eclipse.swt.widgets.Widget.gtk_key_press_event(Widget.java:838) at org.eclipse.swt.widgets.Control.gtk_key_press_event(Control.java:3943) at org.eclipse.swt.widgets.Composite.gtk_key_press_event(Composite.java:889) at org.eclipse.swt.widgets.Widget.windowProc(Widget.java:2293) at org.eclipse.swt.widgets.Control.windowProc(Control.java:6757) at org.eclipse.swt.widgets.Display.windowProc(Display.java:6031) at org.eclipse.swt.internal.gtk.GTK.gtk_main_do_event(Native Method) at org.eclipse.swt.widgets.Display.eventProc(Display.java:1527) at org.eclipse.swt.internal.gtk.OS.g_main_context_iteration(Native Method) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4524) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1157) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1046) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:155) at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:644) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:551) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:156) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:401) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:566) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:653) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:590) at org.eclipse.equinox.launcher.Main.run(Main.java:1461) at org.eclipse.equinox.launcher.Main.main(Main.java:1434)
Maybe the code depended on the now removed "fixActiveShell()" call (that probably was added as a workaround for something), to re-activate the editor. This change seems to "fix" the problem: diff --git a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/CompletionProposalPopup.java b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/CompletionProposalPopup.java index 128c97ae5..64038d07a 100644 --- a/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/CompletionProposalPopup.java +++ b/org.eclipse.jface.text/src/org/eclipse/jface/text/contentassist/CompletionProposalPopup.java @@ -1088,8 +1088,10 @@ class CompletionProposalPopup implements IContentAssistListener { fPopupCloser.uninstall(); fProposalShell.setVisible(false); + Shell shell= (Shell) fProposalShell.getParent(); fProposalShell.dispose(); fProposalShell= null; + shell.forceActive(); } if (fMessageTextFont != null) { I'm guessing we want something better though.
Thanks for testing! > Maybe the code depended on the now removed "fixActiveShell()" call (that probably was added as a workaround for something), to re-activate the editor. That's what I am thinking myself, 'fixActiveShell()' has been there for a while and some code incorrectly depends on it. > This change seems to "fix" the problem: The patch is in fact contrary to what I expected, or I'm misunderstanding something. In Bug 563555, I added a test snippet that shows that GTK already sets focus to parent shell when child shell has focus and is disposed/hidden. I tried all sorts of combinations there and it always works as expected. In your fix, however, you merely do what GTK is already expected to do - remember the parent shell and force focus it after disposing child Shell. Which makes me wonder why GTK failed to do the same thing. The only thing that comes to my mind is that child shell unexpectedly lost focus. To my understanding, this could happen between '.setVisible(false)' and next 'Display.readAndDispatch()'. Since '.dispose()' follows '.setVisible(false)', it probably mean that something happened in listeners.
Commenting out the "setVisible(false)" call doesn't help, though maybe that is not the only such call; I've not checked.
'setVisible(false)' shouldn't be a problem, in fact, this is where GTK is expected to transfer focus to parent shell. If you're willing to do more debugging (which I'm really grateful for), could you please check 1) If 'fProposalShell' has focus just before 'fProposalShell.setVisible(false)' 2) If 'fProposalShell' has focus just before its 'gtk_widget_destroy()' 3) Which Shell has focus after 'fProposalShell.dispose()' - if null, it might also be necessary to do a 'display.readAndDispatch()'.
> 1) If 'fProposalShell' has focus just before 'fProposalShell.setVisible(false)' No, or at least Shell.isFocusControl() returns false. Some table has the focus (I assume the table with content assist suggestions): display.getFocusControl()=Table {} I guess you mean to check if the shell is active? > 2) If 'fProposalShell' has focus just before its 'gtk_widget_destroy()' Also no, its a table again with the focus. > 3) Which Shell has focus after 'fProposalShell.dispose()' - if null, it No shell listed by Display.getShells() has focus (at least according to Shell.isFocusControl()), even after calling while (display.readAndDispatch()). Display.getFocusControl() returns null before and after looping display.readAndDispatch().
Alright, no quick victory here :( Simeon, thanks for taking your time! I will put it on my TODO list, which is already in a pretty bad shape :( Hopefully I will be able to find time for this next week.
(In reply to Alexandr Miloslavskiy from comment #20) > I will put it on my TODO list, which is already in a pretty bad shape :( > Hopefully I will be able to find time for this next week. Thanks. Don't want to add extra pressure, but Sarika just announced the M3 deadline for the next Monday :-)
I'm actually looking into this right now. Problem is, it is not reproducible for me and works as intended. My steps are: 1) Ubuntu 20.04 2) Eclipse 2020-09 3) SWT from 'master' (instead of bundled jars) 4) Disabled theming in 'Eclipse | Window | Preferences | General | Appearance' 5) Have this code in editor: public class SomeClass { public static void main(String[] args) { System.out.println("aaa"); } } 6) Select 'System.out.println("aaa")' 7) Press Ctrl+1 8) Select 'Extract to method' with mouse 9) Type 'aaa' to change method name 10) Press Ctrl+S Contrary to video, in step 8 I have to click (to activate popup) and then double-click (to select menu item). After I click popup, editor's tab becomes gray (inactive). After I select menu item, it becomes active again.
Which particular Eclipse SDK build is this and what GTK3 version do you have?
Created attachment 284712 [details] My KDE settings Could be window manager related. KDE implements various focus stealing prevention things that could be configured, see screenshot. Could also be VNC related (I'm working in VNC, Simeon too). And of course could be distribution dependent (RHEL 7.4 vs Ubuntu). In that sense Windows environment looks like a paradise.
(In reply to Simeon Andreev from comment #23) > Which particular Eclipse SDK build is this and what GTK3 version do you have? Reproducible with both 2020-09 (downloaded from website) and latest IBuild I20201108-1800. $ dpkg -l libgtk-3-0 libgtk-3-0:amd64 3.24.20-0ubuntu1 amd64 (In reply to Andrey Loskutov from comment #24) > Could be window manager related. KDE implements various focus stealing > prevention things that could be configured, see screenshot. Could also be > VNC related (I'm working in VNC, Simeon too). And of course could be > distribution dependent (RHEL 7.4 vs Ubuntu). In that sense Windows > environment looks like a paradise. I suspect that the key thing is that for whatever reason I have to double-click to select popup item. Is it correct that you only need ONE mouse click after pressing Ctrl+1 ?
(In reply to Alexandr Miloslavskiy from comment #25) > Reproducible with both 2020-09 (downloaded from website) and latest IBuild > I20201108-1800. I wanted to say "regression not reproducible"
(In reply to Alexandr Miloslavskiy from comment #25) > I suspect that the key thing is that for whatever reason I have to > double-click to select popup item. Is it correct that you only need ONE > mouse click after pressing Ctrl+1 ? No, I also need double click here.
Do you also need a click to activate popup, or is it already active? Is it correct that you press Ctrl+1 and then immediately double-click?
(In reply to Alexandr Miloslavskiy from comment #28) > Do you also need a click to activate popup, or is it already active? Is it > correct that you press Ctrl+1 and then immediately double-click? Popup that opens on Ctrl+1 *seem* to not take a focus from the active window (the title shows it is as active, tab is still highlighted, but I haven't verified this with debugger). Only after clicking into the popup (or double clicking into it) I see the main window title bar and also the active editor tab to be grayed out/loosing the "active" visual hint.
(In reply to Alexandr Miloslavskiy from comment #28) > Is it correct that you press Ctrl+1 and then immediately double-click? I can wait a minute between - nothing changes, still same behavior.
I can both double-click on the entry in the content assist pop-up, or I can click on the pop-up (which will deactivate the editor) and then double-click on the entry. Both result in the same behaviour. RHEL has no GTK 3.24 build, nor does it have required RPMs to build GTK 3.24 from source. Are you able to check with GTK 3.22.30? If not can try to compile everything GTK 3.24 needs, but that can be very lengthy.
Thanks for clarification. Ubuntu 18.04 has GTK 3.22.30; hang on while I get Eclipse there and test it...
Also no regression reproducible on Ubuntu 18.04 with Eclipse 2020-09.
It looks like GTK version is not the difference here, now I'll have to try KDE instead of Ubuntu's Gnome... I guess I'm going to install a VM with Manjaro KDE. Sigh. Meanwhile, if you have access to some other Linux, can you please test there?
I can reproduce with CentOS 8, GTK 3.22.30, Eclipse SDK 4.18 I20200928-1800. Though for some reason there its even worse, as switching between editor tabs doesn't help. I have to close and open the editor for hotkeys to work again. I'm running CentOS 8 with Gnome, not KDE, in a VM (Oracle VM VirtualBox).
Created attachment 284713 [details] No regression on Manjaro 20.1.2 KDE Also no regression on Manjaro 20.1.2 KDE. I have recorded video with pauses between steps so that it's easier to understand. Note how Eclipse's title becomes inactive when I click popup and re-activates when popup is double-clicked. Note how * in editor tab disappears when I press Ctrl+S in the end.
Created attachment 284714 [details] Manjaro KDE settings KDE settings in Manjaro. The only difference I see is 'Delay Focus by:'. I tried changing it to 0ms, still no regression.
Alexandr, could you try CentOS 8? That seem to be broken without KDE.
Installing a VM now. I don't really have good expectations about it, though. I suspect that you have some evil 3rd party software that causes the problem.
My CentOS 8 installation is mostly defaults and Eclipse is as downloaded (no extra plug-ins).
The regression, as described, is reproducible on CentOS 8 with Eclipse 2020-09. However, it is also reproducible with 2020-06! Can you confirm? This would mean that this is again not caused by fix for Bug 563555. In case of 2020-06, it is sufficient to click editor tab to make hotkeys work again. It is unclear if this is a different problem or not.
(In reply to Alexandr Miloslavskiy from comment #41) > However, it is also reproducible with 2020-06! Can you confirm? This would > mean that this is again not caused by fix for Bug 563555. However, reverting this fix fixes the problem on RH7/KDE. We might have multiple issues here, thanks to creativity of Linux distros trying to reinvent the wheel again and again.
The problem is also reproducible on CentOS 8 with Eclipse 2020-03. I also tested with Eclipse 2020-09 and Bug 563555 reverted: the problem is still there, but clicking editor enables hotkeys again.
I can confirm, my CentOS 8 installation shows the problem with 4.16 also. The bug got "worse" with 4.17. I assume the difference is Gnome instead of KDE, as I have the GTK 3.22.30 on it. Might also be related to WebKit2 versions in some way, though I doubt that.
Created attachment 284737 [details] Reproducing snippet I was able to reproduce the problem of CentOS 8 in a small SWT snippet. The consequence of the problem is that shell is no longer active after closing child shell. It all begins with SWT.ON_TOP style of child shell. Can you please test the snippet on RHEL? What happens if Bug 563555 is reverted? What happens if 'SWT.ON_TOP' is removed? On CentOS8, reverting Bug 563555 doesn't fix anything in the snippet.
It seems that the problem is caused by code in 'Shell.bringToTop()' under 'if ((xFocus || (style & SWT.ON_TOP) != 0))'. At least on CentOS, removing it helps. This also correlates with my previous suspicions that Shell can't be activated due to a grab. For testing purposes, can you delete entire block and see if it helps on RHEL?
I wonder if the difference between my CentOS 8 VM and our work environment is not Gnome/KDE but between Wayland (in my VM) and X11 (our work environment). The code you mentioned in Shell.bringToTop() does other operations for X11. Possibly the behaviour on Wayland has been broken for quite some time, but X11 worked. Anyway, commenting out everything in the "true" case of "if ((xFocus || (style & SWT.ON_TOP) != 0)) {", on RHEL 7.4 (X11), I see the same behaviour as on CentOS 8. I cannot use hotkeys and clicking on the editor doesn't help to restore hotkeys. I have to close and open the editor for them to work. Again on RHEL 7.4, X11: In your snippet from comment 45 I do see inactive shell as soon as the pop-up opens and after I select a tree item it remains inactive. With the Shell.bringToTop() code commented out, I always see "shell: active", even after the pop-up opens. If I revert commit 6371f380811160e02b570e6441e42938de3db6da, behaviour in the snippet is as expected; active until the pop-up shell opens, then active again after the pop-up shell closes.
Created attachment 284746 [details] Ubuntu 19.10 Wayland weirdness Tested on Ubuntu 19.10 (wayland): * Shell is always active, even after popup is opened * There are some pretty bad visual glitches (see video) * The glitches are NOT solved by removing 'Shell.bringToTop()' or Bug 563555 * Regression is not reproducible in Eclipse 2020-09
I have now tested a lot: [1] Visual glitches [2] Parent Shell remains active after opening child Shell [3] Bad due to 'Shell.bringToTop()', clicking doesn't help Ubuntu 18.10 (X11): eclipse:good, snippet:good Ubuntu 18.10 (Wayland): eclipse:good, snippet:good [1][2] Ubuntu 19.10 (X11): eclipse:good, snippet:good Ubuntu 19.10 (Wayland): eclipse:good, snippet:good [1][2] Ubuntu 20.04 (X11): eclipse:good, snippet:good Ubuntu 20.04 (Wayland): eclipse:good, snippet:good [1][2] Manjaro 20.1 (X11): eclipse:good, snippet:good Fedora 31 (Wayland): eclipse:good Fedora 31 (X11): eclipse:good CentOS8 (Wayland): eclipse:bad [3], snippet:bad [3] CentOS8 (X11): eclipse:good, snippet:good CentOS7 (X11): bad, as described in Comment 1 To summarize: * There are a few bugs [1][2][3] that are unrelated to Bug 563555. * CentOS is the only Linux from a large number of others that has the problem at all. * CentOS8 no longer has the original problem as per Comment 1.
(In reply to Alexandr Miloslavskiy from comment #49) > I have now tested a lot: Many thanks! > To summarize: > * There are a few bugs [1][2][3] that are unrelated to Bug 563555. > * CentOS is the only Linux from a large number of others that has the > problem at all. > * CentOS8 no longer has the original problem as per Comment 1. Yes, CentOS 8 has even bigger problem :-) CentOS 7 problem root cause still remains unclear, but the resulted broken behavior is still directly related to the bug 563555. I've tried to debug this, the events I see (set trace points on places where org.eclipse.swt.widgets.Display.activeShell is changed) Original code with fixActiveShell() Shell.gtk_focus_in_event() Shell.bringToTop() Shell.gtk_focus_out_event() Control.sendFocusEvent() Shell.gtk_focus_in_event() <--- closed popup here ---> Shell.bringToTop() Shell.gtk_focus_in_event() Code with fixActiveShell() that does NOT call Shell.bringToTop() Shell.gtk_focus_in_event() Shell.bringToTop() Shell.gtk_focus_out_event() Control.sendFocusEvent() Shell.gtk_focus_in_event() <--- closed popup here ---> Shell.releaseWidget() The last call to Shell.releaseWidget() above is the one that sets the Display.activeShell to null and so breaks RHEL 7. So obviously, on other systems some event is sent to SWT that restores Display.activeShell back to the parent shell. Alexander, could you please check who changes Display.activeShell back to non-null value after closing popup on Ubuntu/Fedora? I assume Shell.bringToTop(boolean) can be related, I see there a lot of workarounds and dances around the shell/focus.
This is how it happens: <SWT.Activate> at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:89) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5785) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1427) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1453) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1432) at org.eclipse.swt.widgets.Shell.gtk_focus_in_event(Shell.java:1533) at org.eclipse.swt.widgets.Widget.windowProc(Widget.java:2321) at org.eclipse.swt.widgets.Control.windowProc(Control.java:6757) at org.eclipse.swt.widgets.Display.windowProc(Display.java:6031) at org.eclipse.swt.internal.gtk.GTK.gtk_main_do_event(GTK.java:-1) at org.eclipse.swt.widgets.Display.eventProc(Display.java:1527) at org.eclipse.swt.internal.gtk.OS.g_main_context_iteration(OS.java:-1) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4524) at Bug568550_ShellActivateOnClose.main(Bug568550_ShellActivateOnClose.java:78)
You can just remove 'SWT.ON_TOP' in snippet to get the expected behavior.
(In reply to Alexandr Miloslavskiy from comment #51) > This is how it happens: > at org.eclipse.swt.widgets.Shell.gtk_focus_in_event(Shell.java:1533) This is what we don't see after closing popup without shell.bringToTop() call from fixActiveShell() :-( So the workaround with fixActiveShell() is still needed... Do I understand it right, this shell.bringToTop() was responsible for bug 563555 on Ubuntu? Any explanation *why*?
(In reply to Alexandr Miloslavskiy from comment #52) > You can just remove 'SWT.ON_TOP' in snippet to get the expected behavior. Which behavior do you mean? Sorry, I didn't understand that.
> So the workaround with fixActiveShell() is still needed... Is still think that there's some other bug that causes the problem, and instead of workaround, it's best to figure what happens. Now that I *finally* can reproduce it, I'm trying to get down to the root cause. Spent a terrible amount of time hunting a repro across all possible Linux. > Do I understand it right, this shell.bringToTop() was responsible for bug > 563555 on Ubuntu? Any explanation *why*? Probably not; Bug 563555 was because SWT explicitly activates a shell when another shell is closed, and it seems that window manager understands it as trying to activate something while something else is active, hence the annoying "is ready" popup. Without this explicit activation, things are good. Just CentOS is acting weird for some reason. > Which behavior do you mean? Sorry, I didn't understand that. Use your RHEL and test snippet from Comment 45; remove 'SWT.ON_TOP' in snippet; snippet will behave perfectly.
Created attachment 284750 [details] Reproducing snippet Slightly polished snippet.
Alrighty, I debugged SWT, GTK, X11 and KDE! 'gdk_window_focus()' translates to '_NET_ACTIVE_WINDOW' request that goes through X11 to Window Manager, KDE in the case of CentOS7. Then, KDE evaluates '_NET_ACTIVE_WINDOW' request and... finds no such window! That raised my eyebrow for sure. Okay, let's start from a different end. SWT uses 'GTK_WINDOW_POPUP' for 'SWT.ON_TOP' windows. Looking in GTK sources, there are multiple warnings to not do that, notably: * A #GtkWindow can be one of these types. Most things you’d consider a * “window” should have type #GTK_WINDOW_TOPLEVEL; windows with this type * are managed by the window manager and have a frame by default (call * gtk_window_set_decorated() to toggle the frame). Windows with type * #GTK_WINDOW_POPUP are ignored by the window manager; window manager * keybindings won’t work on them, the window manager won’t decorate the * window with a frame, many GTK+ features that rely on the window * manager will not work (e.g. resize grips and * maximization/minimization). #GTK_WINDOW_POPUP is used to implement * widgets such as #GtkMenu or tooltips that you normally don’t think of * as windows per se. Nearly all windows should be #GTK_WINDOW_TOPLEVEL. * In particular, do not use #GTK_WINDOW_POPUP just to turn off * the window borders; use gtk_window_set_decorated() for that. The most important pieces here are "ignored by the window manager" and "don't". So SWT for whatever reason requests 'GTK_WINDOW_TOPLEVEL' and invites all sorts of trouble, for which there is a lot of dancing, which still fails.
To summarize, I suggest to * Leave Bug 563555 alone, because its intent in not at all related to this problem and it actually fixes other bugs and removes unwanted window manipulation from SWT * Stop using 'GTK_WINDOW_TOPLEVEL' * Drop most of workarounds related to 'SWT.ON_TOP' * Maybe fix a few things so that Shell still looks and feels like the old 'SWT.ON_TOP', including 'gtk_window_set_decorated()'
(In reply to Alexandr Miloslavskiy from comment #58) > To summarize, I suggest to > > * Leave Bug 563555 alone, because its intent in not at all related to this > problem > and it actually fixes other bugs and removes unwanted window manipulation > from > SWT > * Stop using 'GTK_WINDOW_TOPLEVEL' > * Drop most of workarounds related to 'SWT.ON_TOP' > * Maybe fix a few things so that Shell still looks and feels like the old > 'SWT.ON_TOP', including 'gtk_window_set_decorated()' Sounds like a big task. Do you plan to provide a patch?
I have already strayed too far from my prioritized tasks. Really need to clean my TODO list first. I'm not sure if it's actually a big task. Depends on what are the differences of current SWT.ON_TOP to regular shells. If there aren't many then it shouldn't be much harder than just removing all offending code. Could be a bit of trial&error. I don't see a reason to hurry: Bug 563555 has been there since 4.17 and there weren't a lot of complaints, also, only CentOS seems to be negatively affected. Still, if you really want it, then I would suggest to restore code from Bug 563555, but *only* for 'SWT.ON_TOP' windows, so that both bugs will no longer happen instead of switching between the two.
(In reply to Alexandr Miloslavskiy from comment #60) > I don't see a reason to hurry: Bug 563555 has been there since 4.17 and > there weren't a lot of complaints, also, only CentOS seems to be negatively > affected. I see a reason because I have a lab of ~200 developers and also numerous customers that would get the bug together with our update from 4.15 to 4.18+ SDK. And we will stay on RH 7.x / 8.x for longer time. > Still, if you really want it, then I would suggest to restore code > from Bug 563555, but *only* for 'SWT.ON_TOP' windows, so that both bugs will > no longer happen instead of switching between the two. I've pushed https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/172195, please check that. I really would like to see the fix (in some way) in M3.
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.swt/+/172195 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=2f7f7a8c8b0838357c0975aa340b0c39d6d83d6d Thanks Alexandr!
