Comment 1 Remy Suen 2010-07-22 13:51:15 EDT

This is also reproducible on I20100721-2056. Resetting does _not_ help.

Comment 2 Susan McCourt 2010-07-22 14:10:08 EDT

Confirmed on Win7, I20100721-2056.
It seems as if it doesn't matter how many perspectives you have open, etc.
If you close the last one and it has an editor in it, you are hosed.

Comment 3 Susan McCourt 2010-07-22 14:22:11 EDT

Remy, recall that in bug 319870 comment 7, we had a case where closing all perspectives left the editor area hanging around.  Could our fix for that bug have introduced this?

Comment 4 Remy Suen 2010-07-22 14:29:33 EDT

(In reply to comment #3)
> Could our fix for that bug have introduced this?

The old code just closed parts without ever removing the editor area (which is why it stuck around) so it is technically possible that might've changed something.

Comment 5 Susan McCourt 2010-07-22 14:41:11 EDT

(In reply to comment #4)
> (In reply to comment #3)
> > Could our fix for that bug have introduced this?
> 
> The old code just closed parts without ever removing the editor area (which is
> why it stuck around) so it is technically possible that might've changed
> something.

This is definitely a regression.  It didn't completely work before, but it was better.  I just tried I20100718-2237 (because I happen to have the zip lying around) and here's what happens.

1. Wipe your deltas.
2. Launch your SDK.
3. Open a file
4. Window > Close Perspective
5. Window > Open Perspective > Other... > Java
6. The perspective returns *with an empty editor area*

(you can still open the file again, etc.  not too hosed.)

Comment 6 Eric Moffatt 2010-07-22 15:59:52 EDT

Created attachment 175017 [details]
Don't hide the last editor stack on a perspective close


This is a corner case where the last perspective has already unhooked its shared elements (in this case including the EA). This causes a deep search starting from the window to fail to find the shared EA (and consequently any of its stacks).

In the interests of the least code change I've left the original search in place and handle this corner in new code specifically invoked under this condition.

Note that if you split the editor area before the close all stacks but one will be closed / removed.

Comment 7 Remy Suen 2010-07-22 16:22:54 EDT

(In reply to comment #6)
> Created an attachment (id=175017) [details]
> Don't hide the last editor stack on a perspective close

The inlined comment doesn't seem to match what's happening in the code.

// So we'll try to walk up to the top of the editor stack's containment
// model and search there
if (editorStacks.size() == 0) {
  MUIElement container = element;
  while (container.getParent() != null) {
    container = container.getParent();
  }

In the code above, we don't actually "walk up to the top" of the containment model because if container.getParent() isn't null, the while loop will just run once and it'll be done.

Could you fix the inlined comment, Eric? Thanks.

Comment 8 Susan McCourt 2010-07-22 16:47:05 EDT

+1 for RC3.
I agree with the approach to just search again (surgical fix), but I'd like to see the bug number included in the comment that explains why we are doing this.

(In reply to comment #7)

> The inlined comment doesn't seem to match what's happening in the code.
> 
> // So we'll try to walk up to the top of the editor stack's containment
> // model and search there
> if (editorStacks.size() == 0) {
>   MUIElement container = element;
>   while (container.getParent() != null) {
>     container = container.getParent();
>   }
> 
> In the code above, we don't actually "walk up to the top" of the containment
> model because if container.getParent() isn't null, the while loop will just run
> once and it'll be done.
> 
> Could you fix the inlined comment, Eric? Thanks.

???
I read this as doing just what it says.  It will walk up the parent refs until it reaches a null parent.  So it will search from the top of the stack's containment model.  If container.getParent() isn't null, container will be reassigned to the parent until the walk continues.  

(Or do I need more caffeine?)

Comment 9 Eric Moffatt 2010-07-22 16:49:41 EDT

Remy, we have to walk all the way up to the 'shared element' (whose 'getParent' == null because it's not in the children list) because we could be dealing with an editor stack some number of sashes deep in a split editor area...

I'll change it to something like:

Walk up the containment hierachy, getParent == null indicates that this is the MpartSashContainer representing the EA.

Comment 10 Eric Moffatt 2010-07-22 16:51:33 EDT

Committed in >20100722. Applied the patch.

Comment 11 Eric Moffatt 2010-07-22 16:52:09 EDT

Marking as fixed.

Comment 12 Remy Suen 2010-07-22 16:53:09 EDT

(In reply to comment #8)
> I read this as doing just what it says.

I think when I was debugging it only "looped" once which was causing my confusion. But yes, after looking at the code again, the code matches the comment.

Comment 13 Eric Moffatt 2010-07-22 16:53:44 EDT

Susan, I've added the bug number to the comment...

Comment 14 Eric Moffatt 2010-07-27 13:41:57 EDT

Verified on XP in I20100726-2152.