Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 371598

Summary: [FastView] Fast views do not always close on Escape
Product: [Eclipse Project] Platform Reporter: Ralf Sternberg <rsternberg>
Component: UIAssignee: Eric Moffatt <emoffatt>
Status: VERIFIED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: daniel_megert, eclipse, john.arthorne, markus.kell.r, pbenedict, pwebster, tomasz.zarna
Version: 4.2Flags: john.arthorne: review+
Target Milestone: 4.4 M1   
Hardware: PC   
OS: Linux   
Whiteboard: candidate43
Bug Depends on:    
Bug Blocks: 414050    

Description Ralf Sternberg CLA 2012-02-15 05:51:02 EST
Environment: Linux 64bit, GTK, Eclipse for RCP and RAP developers package 4.2M5

Steps to reproduce:
* Focus a code editor
* Invoke unit tests using Ctrl+X, T
-> Junit view opens as fast view (note that the fast view icon does not change to pressed state)
* Press ESC to close the fast view again
-> Fast view does not close
* Click into the code editor
-> Not even this closes the fast view

Other views show a similar behavior. For example, the Call Hierarchy fast view does also not close on ESC, but it does on focus out. In contrast to the JUnit view, its icon changes state when the view opens.

The Console fast view seems to always close on ESC and focus lost, as expected.
Comment 1 Eric Moffatt CLA 2012-04-04 15:41:01 EDT
I've seen this one and it really annoys me, I'll take a look
Comment 2 Eric Moffatt CLA 2012-04-17 13:31:50 EDT
this is a function of the page not being able to select some other part to activate (which would cause the FV to close).
Comment 3 Eric Moffatt CLA 2012-08-03 11:09:23 EDT
They should also open on 'enter' for accessibility...
Comment 4 Eric Moffatt CLA 2012-08-21 13:47:16 EDT
*** Bug 384814 has been marked as a duplicate of this bug. ***
Comment 5 Eric Moffatt CLA 2012-08-21 13:51:31 EDT
I've marked Bug 384814 as a DUP because they all share the underlying fault that we were basing our 'tear down' strategy on some other part becoming active. The current code searches for a part to activate in the current presentation (i.e. not in the trim). While usually not an issue if we have an *empty* shared area which is maximized then there are no candidates to activate.

We need to allow there to be *no* active part for cases like this, I'm just not sure what impact this will have on the rest of the environment (i.e. we can't send a 'partActivated' message because many current listeners would NPE...).
Comment 6 Eric Moffatt CLA 2012-08-23 14:18:13 EDT
*** Bug 386914 has been marked as a duplicate of this bug. ***
Comment 7 Eric Moffatt CLA 2012-08-27 10:29:58 EDT
John, could you please +1 this based on the review we just did ?

I still don't have an answer for clicking on the background of an empty editor stack but at least clicking on the icon or hitting ESC will tear down the current stack.
Comment 8 Eric Moffatt CLA 2012-08-27 11:29:21 EDT
commit 4310bfdf8f23e37eb27917b76b746d336b3ed2d9

Fixes at least the ESC and clicking on the open view's icon work...

The case of clicking on the background of the empty editor area is *not* fixed by this but this is a much time as we have in 4.2.1. I've re-opened Bug 384814 to track this for future work...
Comment 9 Eric Moffatt CLA 2012-08-30 13:23:54 EDT
Verified in M20120829-1200.
Comment 10 Dani Megert CLA 2012-11-05 04:20:44 EST
(In reply to comment #7)
> John, could you please +1 this based on the review we just did ?
> 
> I still don't have an answer for clicking on the background of an empty
> editor stack but at least clicking on the icon or hitting ESC will tear down
> the current stack.

No, hitting Esc to close the fast view still neither works in 4.3 M3 nor 4.2-M20121031-1200.
Comment 11 Dani Megert CLA 2012-11-05 04:29:00 EST
Note: the main reason why 'Esc' is not working is that the fast view does not get the keyboard focus (as compared to 3.x).
Comment 12 John Arthorne CLA 2012-11-20 11:41:09 EST
(In reply to comment #10)
> (In reply to comment #7)
> > John, could you please +1 this based on the review we just did ?
> > 
> > I still don't have an answer for clicking on the background of an empty
> > editor stack but at least clicking on the icon or hitting ESC will tear down
> > the current stack.
> 
> No, hitting Esc to close the fast view still neither works in 4.3 M3 nor
> 4.2-M20121031-1200.

That depends on how you open it. If I click a minimized view to reveal it, and then hit Esc, it minizes for me.
Comment 13 Paul Webster CLA 2013-01-11 14:02:52 EST
Defered to 4.3
Comment 14 Eric Moffatt CLA 2013-01-16 14:22:39 EST
This is indeed a pain...in debugging with the editor area maximized the Debug view keeps being 'broughtToTop' but not activating (so no Focus). In this case neither hitting the ESC (as Dani points out) nor clicking on the editor will close the stack.

I suspect that I'll have to arrange things so that both of these cases cause the stack to close. Perhaps a traversal listener for ESC and a mouse *filter* (a la 3.x) would work better...
Comment 15 Eric Moffatt CLA 2013-07-29 10:21:02 EDT
Committed:

http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=a23cbd01b2bb406e19fd38cf2fd6e3afcc82e957

This adds a mouse filter when a trim stack opens that will close it whenever a mouse click is detected in the *client area* of the shell but not over the floy out composite. This allows using the mail toolbar / menu but still lets you tear down the fly out by clicking elsewhere).

I'm tentatively going to mark this one as fixed...if anybody thinks we *need* ESC to work as well feel free to re-open.
Comment 16 Eric Moffatt CLA 2013-07-29 10:34:00 EDT
Modified the code to use MouseDown rather than MouseUp:

http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=f36ba3c99507d58dfa56979682733451bd2dc3d9
Comment 17 Paul Benedict CLA 2013-07-29 11:16:13 EDT
Could this be backported to 4.3.1? The annoyance is great enough and fix is small enough that should be safe. I don't want to wait a year to get this.
Comment 18 Markus Keller CLA 2013-07-29 12:09:16 EDT
(In reply to comment #15)
The mouseListener on the Display looks very scary, especially since the removeFilter() call is nested in many if-conditions.

And indeed, the filter was leaked in the first scenario I tested (open a second workbench window; execute comment 0; click active window's close box):

org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:4397)
	at org.eclipse.swt.SWT.error(SWT.java:4312)
	at org.eclipse.swt.SWT.error(SWT.java:4283)
	at org.eclipse.swt.widgets.Widget.error(Widget.java:472)
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:344)
	at org.eclipse.swt.widgets.Control.getBounds(Control.java:1233)
	at org.eclipse.e4.ui.workbench.addons.minmax.TrimStack$1.handleEvent(TrimStack.java:120)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84)
	at org.eclipse.swt.widgets.Display.filterEvent(Display.java:1262)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1057)
	at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:4170)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3759)
...


Moreover, I think you're not trying to fix the right bug, see comment #11:

> Note: the main reason why 'Esc' is not working is that the fast view does
> not get the keyboard focus (as compared to 3.x).

That's the scenario to fix. It should not be possible in the first place to get into the bad state where a fast view is shown but another part has focus in that window. A MouseDown filter should not be necessary for this.
Comment 19 Ralf Sternberg CLA 2013-07-29 13:18:51 EDT
+1 for comment 18

The real problem is that fast views don't get the keyboard focus under some conditions, e.g. when running tests opens the JUnit view. Not closing on click outside of the view is only one implication. This bug also makes it impossible to continue from that point using the keyboard. For me this is the most annoying problem in E4 and I'm sure I'm not the only IDE user who prefers using the keyboard.

However, not closing on ESC (the title of this bug) is obviously not related to the focus problem, because that also doesn't work when the fast view is focused. It worked in 3.x and it should also work in E4 again. Should we have another bug for the focus issue?
Comment 20 Eric Moffatt CLA 2013-07-29 15:07:01 EDT
Perhaps I'd be better off to perform the same hack we do in 3.x; *active* the view being shown in the minimized stack even if it's not being activated by the code (which is really calling BringToTop).

let me take a cur at this...
Comment 21 Paul Benedict CLA 2013-07-29 15:13:00 EDT
Personally, I like the idea that the FastView doesn't have the focus. For example, I want my Console Output and Error to popup on a message, but I don't want to take control away from my activity. All I want is to click somewhere or continue typing to dismiss the FastView. Right now, I have to click the FastView to give it focus just so I can click somewhere else to get rid of it.
Comment 22 Eric Moffatt CLA 2013-07-29 15:27:26 EDT
OK, thanks for the fast input. I've just committed:

http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=eed1302da726969deb2d6554f75f9ecfc0bcdb8f

This reverts the previous change and instead *activates* the view that's coming up. I was trying to avoid this because of the API issue but this fix is both simpler and also allows ESC to work...

Paul, you should be OK even with this change...it'll initially give the focus to the Console but either hitting ESC or clicking somewhere else should work to get back to your main task...
Comment 23 Paul Benedict CLA 2013-07-29 15:41:14 EDT
Thanks Eric. I think ESC is perfect escape hatch ;-) Back to my question (comment #17), if this solution is considered relatively low-impact, can it be back-ported to 4.3.1?
Comment 24 Eric Moffatt CLA 2013-07-29 16:05:48 EDT
Paul, I'm hoping so...

I'm going to re-mark this as FIXED barring further feedback...
Comment 25 Dani Megert CLA 2013-07-30 09:02:33 EDT
(In reply to comment #24)
> Paul, I'm hoping so...
> 
> I'm going to re-mark this as FIXED barring further feedback...

I did not review the fix but verified that it works as expected in  	N20130729-2000.
Comment 26 Dani Megert CLA 2013-07-31 05:41:17 EDT
(In reply to comment #25)
> (In reply to comment #24)
> > Paul, I'm hoping so...
> > 
> > I'm going to re-mark this as FIXED barring further feedback...
> 
> I did not review the fix but verified that it works as expected in  
> N20130729-2000.

While it works for views, it is still broken for shared/editor area as fast "view". Filed bug 414108 to track that.
Comment 27 Eric Moffatt CLA 2013-08-07 10:46:31 EDT
Verified in 4.4.0.I20130806-2000.
Comment 28 Marvin Fröhlich CLA 2016-01-28 10:30:11 EST
4.5.1:

I put Outline view into a FastView. When I click on it, it doesn't hide on Escape click.

So +1 from me.