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

Bug 320437

Summary:

[UI] PageBook toolbars lose the view menu drop down on a page change

Product:

[Eclipse Project] e4

Reporter:

Eric Moffatt <emoffatt>

Component:

Assignee:

Eric Moffatt <emoffatt>

Status:

RESOLVED FIXED

QA Contact:

Severity:

major

Priority:

CC:

bokowski, remy.suen

Version:

unspecified

Flags:

pwebster: review+
emoffatt: review? (bokowski)

Target Milestone:

1.0 RC3

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
force the pagebooks to re-render, adding the view menu	none
Revised version of previous patch	none
Revised patch that also forces a layout after a perspective switch	none
Revised patch nased on Boris' comments	none

Description Eric Moffatt

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

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

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

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

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

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

2010-07-21 11:14:07 EDT

Created attachment 174867 [details]
Revised patch nased on Boris' comments

Comment 7 Eric Moffatt

2010-07-21 11:16:55 EDT

Committed in >20100721. Applied the patch.

Comment 8 Boris Bokowski

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

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

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

2010-07-27 13:53:35 EDT

Verified on XP in I20100726-2152.