Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 317591

Summary: RemoveGui is not recursive
Product: [Eclipse Project] e4 Reporter: Remy Suen <remy.suen>
Component: UIAssignee: Project Inbox <e4.ui-inbox>
Status: CLOSED DUPLICATE QA Contact: Eric Moffatt <emoffatt>
Severity: critical    
Priority: P3    
Version: 1.0   
Target Milestone: 4.1 M1   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
PartRenderingEngineTests patch v1
none
Ensure that the TBR cascading gets done after the initial element has been handled
none
Patch to explicitly un-inject elements in removeGui none

Description Remy Suen CLA 2010-06-22 10:43:36 EDT 
1. Put a breakpoint in CompatibilityPart's destroy() method.
2. Launch your inner.
3. Close your 'Package Explorer'.
4. Nothing happens.
5. Close your 'Outline' view.
6. Nothing happens.
7. Close your 'Problems' view.
8. The breakpoint is hit. Resume.
9. Close your 'Javadoc' view.
10. The breakpoint is hit. Resume.
11. Close your 'Declaration' view.
12. Nothing happens.
Comment 1 Remy Suen CLA 2010-06-22 19:14:36 EDT 
Created attachment 172472 [details]
PartRenderingEngineTests patch v1

Tests patch to reproduce the problem.

When a part stack is unrendered, its child parts are not disposed. Please note that this happens _regardless_ of whether it is a shared part or not.
Comment 2 Eric Moffatt CLA 2010-06-29 10:23:44 EDT 
The first element to get 'removeGui' called on it is the stack (rather than the part that was initially closed.

Perhaps the CleanupAddon should perform *all* its operations in an asynch??
Comment 3 Eric Moffatt CLA 2010-06-29 10:48:19 EDT 
Created attachment 173011 [details]
Ensure that the TBR cascading gets done after the initial element has been handled
Comment 4 Eric Moffatt CLA 2010-06-29 10:51:47 EDT 
Committed in >20100629. Applied the patch.

Remy raises the point that for true consistency that setting the TBR to false for a stack should call 'removeGui' on any TBR(true) elements it owns (but *not* set their TBR to false). We should discuss if there's a better way to manage this (perhaps by adding a callback into the renderer ?)
Comment 5 Eric Moffatt CLA 2010-06-29 11:10:19 EDT 
Renaming this defect to reflect that the current removeGui code does not explicitly recurse through its children. 

This is likely to interfere with some of the existing logic however (i.e. we call the parent's 'hideChild' which, for stacks, will force another tab to become the 'selectedElement', leading to badness...;-).

For this reason I've moved this post-release for a proper examination.
Comment 6 Eric Moffatt CLA 2010-06-29 15:22:12 EDT 
Created attachment 173039 [details]
Patch to explicitly un-inject elements in removeGui
Comment 7 Eric Moffatt CLA 2010-06-29 15:23:08 EDT 
Committed in >20100629. Applied the patch.
Comment 8 Remy Suen CLA 2010-09-09 10:51:10 EDT 
(In reply to comment #1)
> Created an attachment (id=172472) [details]
> PartRenderingEngineTests patch v1

Merged into HEAD.

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