Community
Participate
Working Groups
The API introduced in bug 213645 is not working in conjunction with save and restore. I've tried debugging in the workbench a bit but I must admit I have no clue what needs to be changed. Any pointers how we can achieve this would appreciateds so that I can prepare a patch for 3.8
Created attachment 202102 [details] Patch This fixes the problem for me - can someone review that and tell me if I'm on a completely wrong track?
Created attachment 202103 [details] example project
Created attachment 202248 [details] Revised patch to never create a ContainerPlaceholder for a 'durable' stack Tom, the original patch worked but was overkill since the 'replacePlaceholderWithPart(LayoutPart)' is called for every part in the presentation. This patch just augments your change to the ViewSashContainer's restoreState method to not create a ContainerPlaceholder if the actual ViewStack is to be 'durable'. I'll commit this into 3.8 and I'll open a new defect to see if we can get it into 3.7.2 (no promises but we can show that the change can only affect 'durable' ViewStacks and is otherwise without side-effects).
Committed in >20110826. Applied the revised patch.
Tom, get back to me if there are any issues with this. I tested it using your example project (thanks!) and it appears to work fine.
> I'll commit this into 3.8 and I'll open a new defect to see if we can get it > into 3.7.2 (no promises but we can show that the change can only affect > 'durable' ViewStacks and is otherwise without side-effects). Did you opened a bug for this Eric? I have no problem rolling my own workbench-plugin but getting it from upstream would certainly make my life easier. Tom
The current fix for this introduces a regression into the existing work flows (see bug 358119 for details). We need the perspective's id in order to make the 'isDurable' API call to the advisor. Not sure what the correct fix is yet...
Well first of all isDurable is used mainly RCP applications who normally not hit by the problem we are seeing here because this happens only if a perspective is restored in a New Window, so we could simply add a check for null-check and omit the call isDurable() - in the other parts where the call is done there's also a check if the descriptor is not null BTW. I think we need to document the fact that we are not calling isDurable() in the above mentionned case. Another interesting question naturally is also why the perspective is not set when we restore a view in a new window.
I start to ask myself why Boris introduce this by setting a boolean field and not simply invoking the check on the advisor when getDurable() is called. Then this would not be different whether the folder is restored or created from scratch
Eric, I think we don't need to store those in the ViewStack. I've taken a look and the getDurable() is only used in the PerspectiveHelper. Instead of storing/restoring the durable information we can simply test in the PerspectiveHelper by calling the WindowAdvisor. The problem with we are seeing when a perspective is restored in a new window the perspective is set after the restore happened. I'll prepare a patch.
and btw your fix didn't worked out fully because when one restored a ViewStack which is empty it was hidden. I think we should remove the durable-attribute and implement the logic inside the PerspectiveHelper where we are guaranteed to have the perspective.
Tom, any chance you could make a patch ? I think I understand but I read code better...;-).
It's at the top of my work queue for Monday
Beauty, mine too :)
Created attachment 204162 [details] Patch This patch completely removes the durable-attribute and simply calls out to the advisor when needed. If I read the code correct this perspective is guaranteed to be set when when helper is invoked and so we don't run in the NPE like the one in bug 358119. Now the only question I'm still uncertain about is why Boris introduced the durable attribute, am I missing a race condition here e.g. with the window-advisor?
Boris - do you by chance remember why you added this durable attribute?
Tom, this patch looks good. I'm just being a bit more diligent about testing this time. I think you are correct though in that by placing the code directly into the PerspectiveHelper is should be *impossible* to not be able to get its id as needed... The only concern I have is that the ViewStack is still using the 'durable' field in 'setMinimized' but the field no longer reflects the actual 'durable' state. This ends up leaving the window in an incorrect state if you minimize a 'durable' folder (screen shot coming). Let me see if I can work around that since the code that's in 'setMinimized' is a hack to get around the issue and there's already added logic in the PerspectiveHelper regarding min/max (so adding a special check to 'isDurable' to return false under this condition isn't really too bad I think).
Created attachment 204212 [details] Screen shot of the sample app if you minimize the 'messages' stack Since the field is no longer used by the PerspectiveHelper the code no longer works.
Tom, I'm still working on this but my first three tries...#fail. Sheesh, the 3.x codebase is complicated (it's been a while since I've had to . I've gone back to your original patch since is still has the field in use (changing the min/max implementation is hairier than getting this working I suspect).
Created attachment 204324 [details] Patch This patch is an improved version from the very first one: a) durable replacement happens in PerspectiveHelper b) It takes care of minimized stacks by consulting the perspectives memento
Thanks Tom, I'm going to do both this patch as well as the fix for bug 355763 at the same time since the NO_CLOSE tag is the 'e4' version of 'durable'...
OK, as far as I can tell this one doesn't introduce any issues in the IDE itself and looks safe. There is still one corner case that's not perfect. If you minimize the durable stack and close its last view while its minimized the stack disappears from the trim but does *not* show back up in the UI. If you are OK with that then I'll commit this patch. It may be possible to detect when a 'durable' but minimized stack goes empty and force a restore on it from the FastViewManager... Let me know what you think.
*** Bug 345050 has been marked as a duplicate of this bug. ***
We can fix this now easily in 4.x by using the NO_AUTO_CLOSE tag introduced in bug 355763
What's the current state of this defect ? I'm considering moving if out of Luna...
In Eclipse3 compatibility mode the method WorkbenchWindowAdvisor#isDurableFolder() is never called and therefore the tag "NoAutoCollapse" is not defined in the corresponding MStack model elements with the result that the Folder will be always collapsed when the last part is closed
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
