Comment 1 Nick Edgar 2002-06-04 13:56:27 EDT

Only occurs when Preview is a fast view.
Also occurs if you open the Update perspective and make Preview a fast view.

Vlad, do you know if the Preview view tracks activation of other parts and 
activates itself?  If not, this seems like a Workbench problem.  Possibly some 
interaction with the OLE control.

Comment 2 Nick Edgar 2002-06-04 13:59:04 EDT

After exiting, the workbench.xml has:

<perspectives activePart="org.eclipse.update.ui.DetailsView" 
activePerspective="org.eclipse.ui.resourcePerspective">

I believe this is the id for the Preview view (even though we tried to close 
it).

Comment 3 Nick Edgar 2002-06-04 14:07:21 EDT

The stack trace above seems secondary.
The initial one is as follows, but occurs in the same place:

!ENTRY org.eclipse.ui 4 4 Jun 04, 2002 14:13:44.793
!MESSAGE Unhandled exception caught in event loop.
!ENTRY org.eclipse.ui 4 0 Jun 04, 2002 14:13:44.808
!MESSAGE Widget is disposed
!STACK 0
org.eclipse.swt.SWTException: Widget is disposed
	at org.eclipse.swt.SWT.error(SWT.java:1887)
	at org.eclipse.swt.widgets.Control.getDisplay(Control.java(Compiled 
Code))
	at org.eclipse.swt.widgets.Control.getDisplay(Control.java(Compiled 
Code))
	at org.eclipse.swt.widgets.Control.getDisplay(Control.java(Compiled 
Code))
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java(Compiled 
Code))
	at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java(Compiled 
Code))
	at org.eclipse.swt.custom.CLabel.setBackground(CLabel.java:425)
	at org.eclipse.ui.internal.ViewPane.drawGradient(ViewPane.java:352)
	at org.eclipse.ui.internal.ViewPane.showFocus(ViewPane.java:456)
	at org.eclipse.ui.internal.WorkbenchPage.deactivatePart
(WorkbenchPage.java:897)
	at org.eclipse.ui.internal.WorkbenchPage.setActivePart
(WorkbenchPage.java:2015)
	at org.eclipse.ui.internal.WorkbenchPage.requestActivation
(WorkbenchPage.java:1819)
	at org.eclipse.ui.internal.PartPane.requestActivation
(PartPane.java:332)
	at org.eclipse.ui.internal.PartPane.handleEvent(PartPane.java:306)
	at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java
(Compiled Code))
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:841)
	at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:846)
	at org.eclipse.swt.widgets.Shell.setActiveControl(Shell.java:757)
	at org.eclipse.swt.widgets.Shell.WM_MOUSEACTIVATE(Shell.java:1117)
	at org.eclipse.swt.widgets.Control.windowProc(Control.java(Compiled 
Code))
	at org.eclipse.swt.widgets.Decorations.windowProc
(Decorations.java:1343)
	at org.eclipse.swt.widgets.Display.windowProc(Display.java(Compiled 
Code))
	at org.eclipse.swt.internal.win32.OS.DefWindowProcW(Native Method)
	at org.eclipse.swt.widgets.Scrollable.callWindowProc(Scrollable.java
(Compiled Code))
	at org.eclipse.swt.widgets.Scrollable.callWindowProc(Scrollable.java
(Compiled Code))
	at org.eclipse.swt.widgets.Control.windowProc(Control.java(Compiled 
Code))
	at org.eclipse.swt.widgets.Display.windowProc(Display.java(Compiled 
Code))
	at org.eclipse.swt.internal.win32.OS.DefWindowProcW(Native Method)
	at org.eclipse.swt.widgets.Scrollable.callWindowProc(Scrollable.java
(Compiled Code))
	at org.eclipse.swt.widgets.Scrollable.callWindowProc(Scrollable.java
(Compiled Code))
	at org.eclipse.swt.widgets.Control.windowProc(Control.java(Compiled 
Code))
	at org.eclipse.swt.widgets.Display.windowProc(Display.java(Compiled 
Code))
	at org.eclipse.swt.internal.win32.OS.DefWindowProcW(Native Method)
	at org.eclipse.swt.widgets.Scrollable.callWindowProc(Scrollable.java
(Compiled Code))
	at org.eclipse.swt.widgets.Scrollable.callWindowProc(Scrollable.java
(Compiled Code))
	at org.eclipse.swt.widgets.Control.windowProc(Control.java(Compiled 
Code))
	at org.eclipse.swt.widgets.Display.windowProc(Display.java(Compiled 
Code))
	at org.eclipse.swt.internal.win32.OS.DefWindowProcW(Native Method)
	at org.eclipse.swt.widgets.Scrollable.callWindowProc(Scrollable.java
(Compiled Code))
	at org.eclipse.swt.widgets.Scrollable.callWindowProc(Scrollable.java
(Compiled Code))
	at org.eclipse.swt.widgets.Control.windowProc(Control.java(Compiled 
Code))
	at org.eclipse.swt.widgets.Display.windowProc(Display.java(Compiled 
Code))
	at org.eclipse.swt.internal.win32.OS.PeekMessageW(Native Method)
	at org.eclipse.swt.internal.win32.OS.PeekMessage(OS.java:1600)
	at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:1284)
	at org.eclipse.ui.internal.Workbench.runEventLoop(Workbench.java:1177)
	at org.eclipse.ui.internal.Workbench.run(Workbench.java:1160)
	at org.eclipse.core.internal.boot.InternalBootLoader.run
(InternalBootLoader.java:739)
	at org.eclipse.core.boot.BootLoader.run(BootLoader.java:462)
	at java.lang.reflect.Method.invoke(Native Method)
	at org.eclipse.core.launcher.Main.basicRun(Main.java:244)
	at org.eclipse.core.launcher.Main.run(Main.java:693)
	at org.eclipse.core.launcher.Main.main(Main.java:526)

Comment 4 Nick Edgar 2002-06-05 09:23:26 EDT

*** Bug 19098 has been marked as a duplicate of this bug. ***

Comment 5 Nick Edgar 2002-06-05 09:23:41 EDT

*** Bug 19088 has been marked as a duplicate of this bug. ***

Comment 6 Nick Edgar 2002-06-05 09:24:15 EDT

Need to verify that PRs marked as dups are really dups.

Comment 7 Nick Edgar 2002-06-06 00:07:07 EDT

*** Bug 18918 has been marked as a duplicate of this bug. ***

Comment 8 Nick Edgar 2002-06-06 00:08:57 EDT

Need to establish whether this is caused by the Workbench or the Preview view.
Note that it does not use an OLE control after all.

Comment 9 Nick Edgar 2002-06-06 00:09:31 EDT

Even if it's caused by the Preview view, the Workbench should be more 
resilient to it.

Comment 10 Simon Arsenault 2002-06-07 13:59:48 EDT

In the update plugin, there is code that listens for selection changes in the 
workbench. When the empty Outline view is made active, the selection change 
event is fired (the selection will be empty). The update plugin has code to 
handle an empty selection. In that code, if the Preview view exist in the 
perspective, then bringToTop() is called. If the Preview view happens to be a 
fast view, then the workbench will opened and give focus to it in the bringToTop
() method. That is why clicking on the Outline view shows the Preview view. 

There is a problem here for the workbench API bringToTop because the java doc 
states it will make the view visible but will not change the active part. But 
for a fast view to be made visible, it needs to be the active part (i.e have 
focus), otherwise it will hide itself. This code needs to be updated to better 
handle fast view.

The next problem is related to the first. When the Preview view is closed, in 
the hideView method, it detects it is the active view, and therefore activates 
the last view to be active (in this case, the Outline view). But when the 
Outline view becomes active, the selection change happens, causing the update 
plugin to call bringToTop for the Preview view, causing the Preview fast view 
to become active again. Then the hideView() method continues to dispose of the 
Preview view. However, the page still has the Preview view as the active one. 
Once the other view is activated, the Preview view will bomb.

Changing the hideView method this close to release 2.0 is too risky. We will 
very likely introduce other problems or cause other plugins to behave 
differently. This fix is being delayed until after release 2.0 when more 
thought can be given to this and what the consequences will be.

Per Nick's request, I've opened another problem report (bug 19631) against the 
update plugin to stop calling bringToTop during a selection change event.

Comment 11 Randy Giffen 2002-08-01 16:51:44 EDT

Reopened for investigation

Comment 12 Douglas Pollock 2004-07-08 15:31:42 EDT

My recommendation is to change the documentation for IWorkbenchPage.bringToTop 
to say that it will set focus if the view happens to be a fast view.  To 
accomodate those people that might not want that effect, we could added a 
bringToTop(IWorkbenchPart,boolean).  The boolean flag would indicate whether 
fast views should be affected by this call. 
 
I don't believe it is a realistic expectation to have a fast view open that 
does not have focus.  This seems wrong somehow.... 

Comment 13 Douglas Pollock 2004-09-28 11:42:06 EDT

Moving to Nick Edgar, as he owns ViewMgmt.  Adding MVM and Billy to the CC as 
they own Workbench. 

Comment 14 Nick Edgar 2005-04-27 15:10:34 EDT

In the latest from head, bringToTop for a view will give it focus if it's in the
same stack as the active view, but will have no effect if it's a fast view.

Comment 15 Stefan Xenos 2005-04-29 15:11:37 EDT

Re: comment 14

While this change fixes the exception, I'm not sure if it makes sense. After
talking with Doug and Kim, we've identified 4 operations you might want to do on
a part.

1. Activate (Always make visible and give focus)
2. Make visible (Always make visible. Unzoom and change focus only if necessary)
3. Draw attention (Make visible if possible, but not if doing so affects zoom or
focus).
4. Create (Add to the layout but do not change zoom, focus, or the visibity of
any other parts.).

We believe that editors and views should both have open methods that accept all
four flags, and that the existing workbench APIs should all be classified into
one of these behaviors.

openEditor(..., true) = activate
openEditor(..., false) = make visible

showView(String) = activate

showView(..., VIEW_ACTIVATE) = activate
showView(..., VIEW_VISIBLE) = draw attention
showView(..., VIEW_CREATE) = create

bringToTop() = make visible?

If we decide to give bringToTop the "make visible" behavior, then the code
should be updated to unzoom if necessary and to permit opening of fast views (it
WILL set focus).

If we decide to give bringToTop the "draw attention" behavior, then we should
update the code to turn it into a no-op if the page is zoomed or the part is in
the active stack.

Thoughts?

Comment 16 Nick Edgar 2005-04-29 15:17:37 EDT

This makes sense to me.  bringToTop should be a "make visible" operation.

Shouldn't VIEW_VISIBLE really be "make visible" instead of "draw attention"?
I think this was discussed in another PR, but I don't recall what the upshot was.

In lieu of API changes for 3.1, I suggest we just fix the bringToTop behaviour,
and clarify the Javadoc.

Comment 17 Stefan Xenos 2005-04-29 19:06:01 EDT

Re: comment 16

Yes, I filed bug 91775 because I believed this to be the case. However, after
talking this over with Kim I now believe otherwise. I will try to paraphrase
(Kim, please correct me if I'm wrong):

1. People use the VIEW_VISIBLE flag in situations where they want to draw
attention to a view if possible, but want to guarantee that they never steal focus.
2. This was the original intention of the VIEW_VISIBLE flag, even if the
constant name was poorly chosen.
3. Stealing focus from the user is really annoying. No API should ever do this
unless the JavaDoc is really clear that that's what will happen.


I believe that these arguments apply to bringToTop and openEditor(..., false) as
well, and that all three should have the current "draw attention" behavior of
VIEW_VISIBLE.

That is:

bringToTop() = draw attention
openEditor(..., false) = draw attention


Note that this is not a breaking change: the treatment of openEditor(..., false)
and bringToTop() is undocumented when used on a part in the active stack... and
up until the recent workbenchpage refactoring this scenario would put the
workbench in an invalid state. (In other words, it is not possible for a client
to be relying on the difference between "make visible" and "draw attention"
because this scenario never worked before).

Comment 18 Michael Van Meekeren 2005-06-14 15:54:49 EDT

We should invistigate what happened in 3.0 and it seems that bringtotop should
have an effect on fastviews.  If the fix is not risky we need to get another
component lead to approve.

Comment 19 Nick Edgar 2005-06-15 13:37:51 EDT

Stefan, I'm not sure what the right answer is here.
Would you be able to help?

Comment 20 Michael Van Meekeren 2005-06-16 14:14:27 EDT

adding Mike Wilson to approve.   


Just talked with Stefan and we decided that we should put back the 3.0 behaviour
of bringToTop() for fast views.  The reason is that you could say we have a
regression where users would have previously noticed a view (although activation
in fastviews might be overkill) changing or being brought to front and now the
call to bringToTop will have no effect and introduce mysterious scenarious from
3.0.  

In future we need to revisit this area in the context of bringToTop, showView
and also the view changed (Tab bolding and italisizing) to provide consistent
behaviour.  Fastviews should not take focus when bringToTop is called, this is a
bug I can live with for 3.1 however.

Comment 21 Stefan Xenos 2005-06-16 17:01:49 EDT

Re: comment 20

Just to clarify, we agreed to temporarily bring back the 3.0 behavior of
revealing and setting focus in fast views - not to reveal the fast view without
setting focus (which would be an invalid state).

Comment 22 Douglas Pollock 2005-06-17 14:45:06 EDT

Moving to 3.1 RC4.

Comment 23 Michael Van Meekeren 2005-06-21 09:43:27 EDT

to be reviewed June 21, need to investigate this today, I think the thing to do
will be to push this to the 3.1.1 release.

Comment 24 Kim Horne 2005-06-21 13:06:29 EDT

bringToTop currently works with fast views providing there is an editor open.  It's the case when there are 
no editors open that it fails.  I have a very simple patch that will cause it to work in the no-editor case but 
I am not at all confident in it.  The activation stuff is far to subtle and I'm not convinced that the patch is 
otherwise harmless.  Stefan is the only one who understands this stuff enough to know if this is safe - 
without his approval I would hold off putting this in for 3.1

Comment 25 Kim Horne 2005-06-21 13:09:02 EDT

Created attachment 23644 [details]
Patch against org.eclipse.ui.workbench

The problem in the no editor case is that we were checking the fast view part
container (null) to see if it was the same as the currently active view
container (non null) and the currently active editor container (null).	Because
they were equal we acted as if we were bringing to top the editor area and the
fast view was ignored.	This patch adds null checks against the part container.

Comment 26 Kim Horne 2005-06-21 13:46:40 EDT

I've gone and opened bug 101095 to specifically address the bringToTop() regression with regard to fast 
views in 3.1.  I am removing the target for this bug as there was never any intention to fix it per se, just 
restore 3.0 behavior.  Because we've discovered the regression is fairly minor and only happens when 
there is no editor open we're going to hold off on fixing it for 3.1.

Comment 27 Douglas Pollock 2005-09-22 10:23:27 EDT

Removing milestone. 

Comment 28 Boris Bokowski 2007-06-19 15:08:12 EDT

Reading all this makes my head hurt - does anyone on the CC list remember if there is a remaining issue to be addressed? Fast views are being phased out in favour of the new min/max support, bug 101095 has been closed, and the update manager views don't exist anymore. I'll close this bug as worksforme since I cannot reproduce it with 3.3, but feel free to reopen with a description of the issue that needs addressing.