Comment 1 Eduardo Pereira 2003-02-11 12:42:53 EST

*** This bug has been marked as a duplicate of 30971 ***

Comment 2 Randy Hudson 2004-06-08 16:27:21 EDT

This still occurs in RC1 of 3.0.

During startup:
getSite().getWorkbenchWindow().getActivePage() == null
BUT
getSite().getPage() != null

The only time getActivePage returns null is at startup, yet there are both 
parts and pages, so why isn't one active?

I couldn't really understand bug 30971 so I reopened this one instead. Hope 
that's ok.

Comment 3 Debbie Wilson 2004-06-09 09:05:01 EDT

Removed target milestone and changed Version to 3.0.

Comment 4 Nick Edgar 2004-06-09 09:59:02 EDT

The workbench restores the page(s) before setting the active page.
View and editors should get their page and window from their site, not use
getActive*.

Comment 5 Randy Hudson 2004-06-09 10:49:05 EDT

Nick, this behavior is prone to errors.  Also, parts and controls are *only* 
created for the active page. So once you start calling createControl(), there 
must be an active page already.

Comment 6 Nick Edgar 2005-05-02 11:51:27 EDT

This appears to have been fixed by the recent changes to workbench lifecycle.
I set a breakpoint in ResourceNavigator.createPartControl, and
getSite().getPage().getWorkbenchWindow().getActivePage() returns the site's page.

Comment 7 Nick Edgar 2005-05-02 11:54:14 EDT

This works too:
PlatformUI.getWorkbench().getActiveWorkbenchWindow().getActivePage() 

Note that it's still preferable to obtain the page directly from the part's
site, rather than using getActive*.

Comment 8 Stefan Xenos 2005-05-02 14:08:56 EDT

The recent changes were intended to make things more lazy... not to guarantee
any particular relationship between part lifecycle and the window's active page.
Parts need to check for null from getActivePage(), or access the page from their
site.

Comment 9 Nick Edgar 2005-05-13 12:11:37 EDT

Verified in I20050512-1200.

Comment 10 Pratik Shah 2005-05-17 12:17:18 EDT

I've noticed (in 3.1M7) that this doesn't affect getActivePerspective(), which 
still returns null during startup (and that problem is documented in Bug 
70701).

So, if these changes weren't intended to, and don't, guarantee an active page 
during startup, why has this bug been marked as fixed?  What are the scenarios 
when the active page is null?

Comment 11 Nick Edgar 2005-05-17 13:20:17 EDT

> this doesn't affect getActivePerspective()
Correct, that's covered by bug 70701.

> Why has this bug been marked as fixed?
Because there were changes in M7 that fix the problem here (although they were
not originally intended to specifically address this issue).

> What are the scenarios when the active page is null?
There are no known scenarios currently.  But in any case, the part should get
the containing page from its site, not use getActive*.

Comment 12 Randy Hudson 2005-05-17 14:31:04 EDT

I think that findViewReference is calling getActivePage, not us.

Comment 13 Stefan Xenos 2005-05-17 17:37:29 EDT

The page should be null if there are no pages in the window (happens if there
are no perspectives open yet) or if the only page hasn't been fully initialized yet.

IMHO, WorkbenchWindow should not be handing out pointers to
partially-initialized workbench pages. It is possible that one or more parts may
need to be created in order to fully initialize the page, in which case a part
would be created while the page pointer is null.

Comment 14 Randy Hudson 2005-05-17 18:08:11 EDT

By the time the workbench start to realize a page, all of the view *references* 
should be in existence and available. Views themselves may be in the process of 
being created.

How is this scenario different than the user opening a view?

Comment 15 Stefan Xenos 2005-05-17 18:58:48 EDT

Re comment 15:

That's why this is marked as fixed. Right now, there's no need to realize any
parts. However, this is just a sensible optimization - not an API guarantee. If
we ever discovered a good reason why a concrete part would be needed in order to
fully initialize the page, this would change.

For example, we might want to query the active part for an adapter or take
advantage of some batching optimization by pre-creating visible parts before the
page is activated... or some client code may call one of the APIs that forces
part creation.

Comment 16 Randy Hudson 2005-05-18 10:41:24 EDT

Stefan, You can't claim complete freedom to make changes in the future. Sure, 
the API is not documented very clearly, but the IViewReference interface exists 
for this startup scenario. Assuming that the interface is actually used for its 
intended purpose is valid.

If you did batch notification and existence of other parts, the effect would be 
that the GEF palette would display in a client's editor, and then disappear 
when the editor gets told that the view was created and get reconstructed in 
the view.

Comment 17 Stefan Xenos 2005-05-18 12:18:23 EDT

IViewReference is there to allow views to be lazily materialized, and it is
indeed used for this purpose. Views are materialized the first time they are
needed, not at a particular point in the lifecycle of the page. The reason it
does not specify any relationship with particular pointers from the workbench
page is because there is no relationship -- not because there is any ambiguity
in the doc.

If there is some code that is assuming a non-null active page (or even that the
active page is the same as the page containing the view), please file a bug
against that component so that it can be fixed.

Comment 18 Stefan Xenos 2005-05-18 12:21:36 EDT

Correctly closing as invalid. This is not a bug.

Comment 19 Jared Burns 2005-05-18 13:22:49 EDT

I haven't been following this recent discussion but the original bug report was
most certainly a bug. Just because an API says that something *can* return null,
doesn't mean that you always should. This bug was filed for a case where null
was being returned when it shouldn't.

As per Nick's remarks in comment #6, this bug was fixed.

Comment 20 Stefan Xenos 2005-05-18 13:46:23 EDT

It should be returning null until the page is initialized. That is the correct
behaviour. The fact that no parts have been created yet is coincidence.

Consider this: Right now, this PR is complaining that getActivePage() is null.
Next week, someone complains that getActivePage().getActivePart() is null.
However, it is not possible to satisfy both requirements at the same time (since
returning anything from getActivePart() would require creating a part).

The correct order of initialization in the workbench is bottom-up. Part refs are
initialized before being added to a perspective. Perspectives are initialized
before being added to a page, and pages are initialized before being added to a
window. If the page needs to create a part to honor one of its API contracts,
that part would be created before the page is added to the window.

getActivePart is a good example. It does not -- and should not -- have
restrictions like "you can only call this method on an active page", making it
possible to create a part in an inactive page.

Comment 21 Randy Hudson 2005-05-18 14:14:11 EDT

> It should be returning null until the page is initialized. That is the correct
> behaviour.

Maybe so, depending on what "initialized" means.  Initialized should not 
include creating parts in the page. You can't create a part's control without 
there being an active page and an active perspective.  This is true at other 
times and it is fixable during restore.

> Consider this: Right now, this PR is complaining that getActivePage() is null.
> Next week, someone complains that getActivePage().getActivePart() is null.

Umm, but that case is completely valid.  Close all of your views and editors, 
and go open a view or editor and put a breakpoint in init(...), and query the 
active part. The javadoc also indicates that null is possible, and this is a 
foreseeable case.

Comment 22 Stefan Xenos 2005-05-18 16:36:18 EDT

> Maybe so, depending on what "initialized" means.  Initialized 
> should not include creating parts in the page.

Initialized means that it is capable of respecting the API contract on all of
its methods, and all invariants are met. It is allowed to create any necessary
child objects to ensure that this is true. Parts are child objects, so this DOES
include parts.

> You can't create a part's control without there being an active page 
> and an active perspective. 

That's due to bugs which will eventually be fixed. There's no reason why you
shouldn't be able to ask for an IWorkbenchPartReference for a part in a hidden
perspective and then call getPart(true) on that reference. The fact that you
currently can't is a real bug (it violates an API contract)... and once that is
fixed, there will be cases where controls get created in inactive perspectives.

> Consider this: Right now, this PR is complaining that getActivePage() is null.
> Next week, someone complains that getActivePage().getActivePart() is null.

> Umm, but that case is completely valid.  Close all of your views and
> editors, and go open a view or editor and put a breakpoint in init(...), 
> and query the active part. The javadoc also indicates that null is 
> possible, and this is a foreseeable case.

So your argument is: if a method is javadoc'd to return null in a certain
situation, then returning null in that situation is not a bug. That sounds
reasonable to me.

Try this: close all perspectives, put a breakpoint in WorkbenchPage.init, open a
perspective, and query the active page. This is also valid, forseeable, and in
the javadoc, and therefore not a bug. Returning null from getActivePage is no
more or less a bug than returning null from getActivePart. 

You don't need an active part to create a new part, just like you don't need an
active page. You also don't need an active perspective, window, shell, editor,
etc. The part is guaranteed to get a parent page when it is created, but that
page does not need to be active (or even to belong to a window that has an
active page).

Comment 23 Jared Burns 2005-05-18 17:00:27 EDT

All theoretical arguments aside, this bug has been fixed. I just triple-checked
and the breakage I reported no longer exists.

1. getActivePage() used to return non-null
2. For a brief period, the workbench window was broken such that it was
returning null when it shouldn't.
3. As Nick observed and I've since verified, this bug was fixed and
getActivePage() now returns non-null again.

I'm not sure why we're still arguing about this or why the bug has been marked
INVALID after it was fixed. Are you saying that you want to force the method to
return null again, Stephan?

Comment 24 Randy Hudson 2005-05-18 17:38:42 EDT

> So your argument is: if a method is javadoc'd to return null in a certain
> situation, then returning null in that situation is not a bug. That sounds
> reasonable to me.

Probably because it's your arguement from comment 8:

"Parts need to check for null from getActivePage()"

Comment 25 Randy Hudson 2005-05-18 17:54:40 EDT

> You don't need an active part to create a new part, just like you don't need 
an
> active page.

That's not a very good analogy. The page owns the part so you need a page. 
Anyway, what GEF is specifically interested in is finding another part 
reference when one part is being created. So back over to bug 70701.

Comment 26 Stefan Xenos 2005-05-18 18:17:28 EDT

Re: comment 23

The window was never broken. It still returns null if there is no active page,
and that's exactly what it's supposed to do. There was never any guarantee that
parts won't be created in a window with no active page and there still isn't.

This will probably happen again in the future.

Re: comment 25

Yes, you definitely need a page. My point is that it does not need to be the
active page. It is possible to create parts in any page, active or not.

Comment 27 Jared Burns 2005-05-18 18:24:54 EDT

"It still returns null if there is no active page, and that's exactly what it's
supposed to do."

In my test case there is supposed to be an active page. That was the bug that
was fixed. I still don't understand why a fixed bug has been marked invalid, but
I'm not going to debate this anymore. The bug is fixed so I don't care. :-P

Comment 28 Randy Hudson 2006-02-01 11:51:53 EST

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