Bug 112692 - PaletteStack.setChildren should refresh active entry
Summary: PaletteStack.setChildren should refresh active entry
Status: RESOLVED FIXED
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy GEF (MVC) (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows 2000
: P3 minor (vote)
Target Milestone: 3.4.0 (Ganymede) M5   Edit
Assignee: Cherie Revells CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2005-10-14 14:59 EDT by Cherie Revells CLA
Modified: 2008-09-18 13:32 EDT (History)
2 users (show)

See Also:


Attachments
Patch to fix problem (6.46 KB, patch)
2007-12-21 15:00 EST, Cherie Revells CLA
ahunter.eclipse: iplog+
Details | Diff
patch to fix NPE (945 bytes, text/plain)
2008-01-02 11:35 EST, Cherie Revells CLA
ahunter.eclipse: iplog+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Cherie Revells CLA 2005-10-14 14:59:32 EDT
From bugzilla 110616:

There is an issue in the application 
when the active entry of a palette stack is one that was removed in a call to 
setChildren().  This could be fixed if setChildren() was overridden in 
PaletteStack to call checkActiveEntry() after the new children were set.

------- Additional Comment #3 From Randy Hudson 2005-10-13 11:29 [reply] -------
 
1) The "on top" entry should get updated whenever remove or setChildren gets 
called, or when the on-top entry becomes hidden. If setChildren is called, I 
think it is valid to just reset the on top entry to be the first child. To 
avoid this behavior clients should just remove the items individually.

2) In general, we should verify that removing or hiding the active entry 
results in the default entry being selected (or null if no default).
Comment 1 Randy Hudson CLA 2005-10-14 15:28:37 EDT
So you hijack bug 110616 and then reopened a new bug reporting the same 
problem?? I guess we'll this one now :-)
Comment 2 Cherie Revells CLA 2007-12-21 15:00:53 EST
Created attachment 85737 [details]
Patch to fix problem

I have made the change discussed in the comments so that PaletteStack.setChildren() also calls checkActiveEntry().

Randy's second comment also indicates that there is a problem where the active entry is not updated if it becomes hidden.  There is a use case to reproduce this problem...  

The Customize Palette dialog does not allow the user to hide individual stack items.  I think it should.  When I changed this in the Customize Palette dialog, I realized that the problem is that if the active entry is hidden, then the entire stack is hidden.  The rest of the changes in this patch address this issue.

To summarize the changes:
- PaletteStack now listens to all its children and when it receives notification that the active entry has been hidden it calls checkActiveEntry() to update the active entry.  
- checkActiveEntry() has been updated to also check the visibility of the entries.
- The changes in PaletteStackEditPart are required in the scenario where all the stack's children were hidden and then one becomes visible.
Comment 3 Cherie Revells CLA 2007-12-21 15:04:03 EST
Anthony, this one is ready for a code review.  It affects the Customize Palette capability that I added in GMF.
Comment 4 Anthony Hunter CLA 2007-12-21 16:01:35 EST
Looks ok, committed to HEAD
Comment 5 Cherie Revells CLA 2008-01-02 11:35:33 EST
Created attachment 85994 [details]
patch to fix NPE

I should have added a null check on the active entry in the PaletteStack.checkActiveEntry() method.
Comment 6 Cherie Revells CLA 2008-01-02 11:38:26 EST
Anthony, can you commit the attached patch?
Comment 7 Anthony Hunter CLA 2008-01-02 13:17:38 EST
Committed the fix to HEAD.
Comment 8 Anthony Hunter CLA 2008-01-02 13:17:58 EST
Committed the fix to HEAD.