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

Bug 409633

Summary: Lock the toolbars command not working
Product: [Eclipse Project] Platform Reporter: uls oz <ulasoz2006>
Component: UIAssignee: Patrik Suzzi <psuzzi>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: andreix, bartoszpop, benjarobin+eclipse, benken, bsd, dale.asberry, emoffatt, fernando.torres, gautier.desaintmartinlacaze, jonathan.dumont, larry.singer, psuzzi, shashwat.work, torresdyl
Version: 4.6Keywords: helpwanted
Target Milestone: 4.7   
Hardware: PC   
OS: Windows 7   
See Also: https://git.eclipse.org/r/65040
https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=8f2f668a3ca69828368913a9b5308be677e6981e
Whiteboard:
Bug Depends on:    
Bug Blocks: 514972    
Attachments:
Description Flags
anim gif: solution in action none

Description uls oz CLA 2013-05-31 16:30:26 EDT
To reproduce;

1. Download: http://www.eclipse.org/downloads/download.php?file=/eclipse/downloads/drops4/R-4.2.2-201302041200/eclipse-SDK-4.2.2-win32.zip
2. Open and type "Lock the t.." in Quick Access.
3. Choose "Lock the toolbars"
4. Nothing happens.
Comment 1 Paul Webster CLA 2013-06-17 10:41:02 EDT
*** Bug 410503 has been marked as a duplicate of this bug. ***
Comment 2 Dale Asberry CLA 2014-11-29 18:24:14 EST
Same functionality missing from the toolbar context menu (right-click).
Comment 3 Larry Singer CLA 2015-02-04 00:26:26 EST
The same error is in Luna (4.4.1). With a fresh workspace the tool bars are locked and cannot be unlocked. With a workspace migrated from a previous version the toolbars are not locked.
Comment 4 Larry Singer CLA 2015-02-04 00:35:20 EST
I tried some of these and it now partially works.

There was no theme selected. I selected Windows 7 then selected Help/Welcome. I was then able to move the toolbar for a collapsed view. I was still not able to move the perspective bar or unlock other toolbars.

Also see Bug 421539
Comment 5 Joseph Benken CLA 2015-09-04 14:50:45 EDT
I want to clarify that this is a regression.  Shouldn't the priority be higher?  The functionality as described in the Mars docs is broken: http://help.eclipse.org/mars/index.jsp?topic=%2Forg.eclipse.platform.doc.user%2Ftasks%2Fttoolbars.htm
Comment 6 Patrik Suzzi CLA 2015-09-22 20:09:36 EDT
In our case we have the LockToolBarHandler that calls the CoolBarToTrimManager#setLockLayout(.) , that is an empty body:

	@Override
	public void setLockLayout(boolean value) {
	}

The class implements the interface ICoolBarManager2, extending ICoolBarManager. According the API documents[1] for the latter,  we have:

void	setLockLayout(boolean value)
	Locks or unlocks the layout of the underlying cool bar widget.

So the API is not respected, as the method body is not implementing the functionality. 

Note: The method body is empty since the beginning, on 2011-05-04, when the class was firstly created/moved in the current package. So, it was never implemented.

[1] http://help.eclipse.org/mars/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Freference%2Fapi%2Forg%2Feclipse%2Fjface%2Faction%2FICoolBarManager.html
Comment 7 Fernando Torres CLA 2015-11-29 05:14:52 EST
Please, correct this bug in Mars.SR1 for Windows. It´s not aceptable and a waste of space have a toolbar with 2 lines and half of space empty and can´t do nothing to rearrange it.
Comment 8 Fernando Torres CLA 2015-11-29 05:22:48 EST
It appears to resolve changing theme and restarting eclipse... It´s magic ;-)
Comment 9 Patrik Suzzi CLA 2016-01-23 05:42:40 EST
I'm solving this by adding the IPresentationEngine.NO_MOVE tag to all the MToolBar of the window, that are not tagged as TOOLBAR_SEPARATOR.
Comment 10 Eclipse Genie CLA 2016-01-23 05:50:17 EST
New Gerrit change created: https://git.eclipse.org/r/65040
Comment 11 Patrik Suzzi CLA 2016-04-25 14:16:32 EDT
Please, see attached change.
Comment 12 Patrik Suzzi CLA 2016-04-26 05:34:03 EDT
Moving to 4.7, as it plausibly does not fit earlier.
Comment 13 Patrik Suzzi CLA 2017-04-08 06:25:38 EDT
This still happens in Eclipse SDK
Version: Oxygen (4.7)
Build id: I20170406-2000
OS: Windows 10, v.10.0, x86_64 / win32
Comment 14 Patrik Suzzi CLA 2017-04-08 11:23:27 EDT
As suggested by Brian in the comments, I looked at the callers of CSSRenderingUtils#frameMeIfPossible(), which installs the drag handles, to figure out how their logic should change for determining if the drag handles should be installed.

As frameMeIfPossible() is called in ToolBarManagerRenderer#createWidget() and ToolControlRenderer#createWidget(), with the current code we can add/remove drag handles only when the ToolControls/ToolBars are created. 

I suggest adding a new bug to fix the add/remove of the drag handle image after fixing this bug.
Comment 15 Patrik Suzzi CLA 2017-04-10 12:27:07 EDT
Created attachment 267733 [details]
anim gif: solution in action

Please see the attached animated gif. 

@Brian, thanks for pointing me to the #frameMeIfPossible. Indeed it was pivotal for the solution. 

The proposed solution forces the call of CSSRenderingUtils#frameMeIfPossible with a trick: 

 el.setToBeRendered(false);
 el.setToBeRendered(true);

Which forces the call of ToolBarManagerRenderer#createWidgeg(), followed by the call of CSSRenderingUtils#frameMeIfPossible.