Bugzilla – Full Text Bug Listing
| Summary: | Closing last perspective hoses editor area if it has an editor | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] e4 | Reporter: | Remy Suen <remy.suen> | ||||
| Component: | UI | Assignee: | Eric Moffatt <emoffatt> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | critical | ||||||
| Priority: | P3 | CC: | emoffatt, susan | ||||
| Version: | 1.0 | Flags: | remy.suen:
review+
susan: review+ |
||||
| Target Milestone: | 1.0 RC3 | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Remy Suen
This is also reproducible on I20100721-2056. Resetting does _not_ help. 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. 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? (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. (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.) 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.
(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. +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?) 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. Committed in >20100722. Applied the patch. Marking as fixed. (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. Susan, I've added the bug number to the comment... Verified on XP in I20100726-2152. |