Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 320437 - [UI] PageBook toolbars lose the view menu drop down on a page change
Summary: [UI] PageBook toolbars lose the view menu drop down on a page change
Status: RESOLVED FIXED
Alias: None
Product: e4
Classification: Eclipse Project
Component: UI (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 1.0 RC3   Edit
Assignee: Eric Moffatt CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-20 15:33 EDT by Eric Moffatt CLA
Modified: 2010-07-27 13:53 EDT (History)
2 users (show)

See Also:
pwebster: review+
emoffatt: review? (bokowski)

Attachments
force the pagebooks to re-render, adding the view menu (2.39 KB, patch)
2010-07-20 20:11 EDT, Eric Moffatt CLA 		no flags Details | Diff
Revised version of previous patch (2.46 KB, patch)
2010-07-21 08:57 EDT, Eric Moffatt CLA 		no flags Details | Diff
Revised patch that also forces a layout after a perspective switch (3.76 KB, patch)
2010-07-21 09:23 EDT, Eric Moffatt CLA 		no flags Details | Diff
Revised patch nased on Boris' comments (3.43 KB, patch)
2010-07-21 11:14 EDT, Eric Moffatt CLA 		no flags Details | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Moffatt CLA 2010-07-20 15:33:15 EDT 
If you start clean then the Outline view will have a view menu drop down but once you open an editor it doesn't...

Switching perspectives will bring it back (likely because we re-render the toolbar, causing the code in the RenderedToolbarRenderer to put it back...perhaps we can change the pagebook's update logic to do the same ?
Comment 1 Eric Moffatt CLA 2010-07-20 20:11:26 EDT 
Created attachment 174808 [details]
force the pagebooks to re-render, adding the view menu


This is a patch in progress, not ready for review...
Comment 2 Eric Moffatt CLA 2010-07-21 08:57:02 EDT 
Created attachment 174844 [details]
Revised version of previous patch


This changes ActionBars to use the same logic as the LSR to refresh the TB, causing the view dropdown to get re-added...
Comment 3 Eric Moffatt CLA 2010-07-21 09:23:53 EDT 
Created attachment 174847 [details]
Revised patch that also forces a layout after a perspective switch


The CTF wasn't doing a layout on perspective switches, sometimes causing the TB to 'disappear' if it was 'wrapped' onto the second line...
Comment 4 Paul Webster CLA 2010-07-21 09:28:33 EDT 
I think that's what is necessary to bridge a 3.x update to an e4 renderer.

Just to be safe, after 
Control tbCtrl = tbm.getControl();
you should add a null check.

PW
Comment 5 Boris Bokowski CLA 2010-07-21 10:58:43 EDT 
- remove parentContext null check
- after null check in ActionBars, use field directly instead of getter
- check control for null and disposed
- tbModel != null should be tbModel instanceof MRenderedToolBar
- do we need null checks like part.getContext()!=null and renderer != null?
Comment 6 Eric Moffatt CLA 2010-07-21 11:14:07 EDT 
Created attachment 174867 [details]
Revised patch nased on Boris' comments
Comment 7 Eric Moffatt CLA 2010-07-21 11:16:55 EDT 
Committed in >20100721. Applied the patch.
Comment 8 Boris Bokowski CLA 2010-07-21 11:53:58 EDT 
The latest patch still has calls to getToolbarManager() instead of using the field directly. Also, there should be a check that "renderer" is not null.
Comment 9 Boris Bokowski CLA 2010-07-21 11:54:38 EDT 
Eric, can you make the changes and then set the bug to fixed again? Thanks.
Comment 10 Boris Bokowski CLA 2010-07-22 17:40:53 EDT 
(In reply to comment #9)
> Eric, can you make the changes and then set the bug to fixed again? Thanks.

Made these changes and checked them in.
Comment 11 Eric Moffatt CLA 2010-07-27 13:53:35 EDT 
Verified on XP in I20100726-2152.