Community
Participate
Working Groups
Build Identifier: Version: 4.2.0 Build id: I20120503-1800 I will add an attachment project that will reproduce this issue. The attached project adds three toolbar items to the part toolbar and then you can press a button to remove them one by one. When you remove a toolbar button, the toolbar button is removed from the MMenu's children list but the visuals are not updated. If you click on a removed toolbar button an exception is thrown. I mentioned this issue on bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=368466#c1. Which deals with adding toolbar items and the toolbar not being refreshed. The comment was most likely overlooked and it was my bad for not updating the test project. Reproducible: Always Steps to Reproduce: 1. Add a button to a part's toolbar 2. Remove the button from the part's toolbar and the issue will occur
Created attachment 215061 [details] Project that can be used to recreate bug
See also 376442 PW
Created attachment 215507 [details] Patch for this bug Possible fix for this bug. The idea behind this fix is to make sure the MToolBar model is in sync with the jface ToolBar. Previously new items added to the model were in sync with the ToolBar, but items that were removed from the model were NOT removed from ToolBar. The new code will compare what is in the ToolBar to what is in the model in the processContents(...) method. If there is an item on the ToolBar that is not in the model then the item will be removed from the ToolBar.
(In reply to comment #3) > Created attachment 215507 [details] > Patch for this bug Hi Josh, Thanks for this patch. Could you please update it similar to bug 374568 review #2, use OLD_VALUE in a REMOVE event to determine which model item to remove? PW
Created attachment 223415 [details] Updated ToolbarManagerRenderer Patch I updated the patch with the suggestions from Comment 4. The only concern I have with this patch is that I was not able to test it with tool items that have a dropdown. I wasn't able to get the drop down to work. The button would show and have a dropdown arrow but when I clicked on it the menu was empty. I am going to create a test project this issue and log a separate bug. I only tried adding the toolitem that has a dropdown programmatically.
New Gerrit change created: https://git.eclipse.org/r/141615
Thanks, Rolf.
Gerrit change https://git.eclipse.org/r/141615 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=16d6917f294c68a2e69d4c3d07282671b401ec95
Rolf, I suggest to add this to the N&N for 4.12. I think this is an interesting feature for RCP customers.
@all: please install API tools if you change API and make sure you have right baseline. org.eclipse.e4.ui.workbench bundle has now API error => please fix.
(In reply to Andrey Loskutov from comment #10) > @all: please install API tools if you change API and make sure you have > right baseline. org.eclipse.e4.ui.workbench bundle has now API error => > please fix. How many times do we have to tell this? While I don't expect this from contributors, every committer must do this and also check API Tools when approving a Gerrit change.
Rolf, tonight I don't have laptop access. Please have a look, if you are not available I can have a look tomorrow morning.
(In reply to Dani Megert from comment #11) > (In reply to Andrey Loskutov from comment #10) > > @all: please install API tools if you change API and make sure you have > How many times do we have to tell this? A technical solution would be to include the API tools check into the Gerrit build.
(In reply to Lars Vogel from comment #13) > A technical solution would be to include the API tools check into the Gerrit > build. Right. As long as this is not implemented you *have to* install and use API tools, as I do. I can't always do the dirty work for others.
(In reply to Andrey Loskutov from comment #14) > (In reply to Lars Vogel from comment #13) > > A technical solution would be to include the API tools check into the Gerrit > > build. > > Right. As long as this is not implemented you *have to* install and use API > tools, as I do. I can't always do the dirty work for others. It is installed but it work unreliable for me. In this particular case I did not check the change via API tools as you were as involved in the code review and I incorrectly assumed you did the check. Definitely my fault here.
New Gerrit change created: https://git.eclipse.org/r/141807
(In reply to Andrey Loskutov from comment #10) > @all: please install API tools if you change API and make sure you have > right baseline. org.eclipse.e4.ui.workbench bundle has now API error => > please fix. Sorry, missed this error. Go the API tools installed, as of the Oomph configuration. However, the API tooling got me confused here. When adding the method it suggested a '@since 1.9' annotation. In turn, that caused a (overlooked) error in the Manifest. Fixing that one again, raises an error expected '@since 1.10'. Shouldn't the API tools suggest to increase the version of the Manifest and adding '@since 1.10' instead of the '@since 1.9' suggestion?
Gerrit change https://git.eclipse.org/r/141807 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=943a1d97e447359db6fcf34bf7e8af3bec93ef80
This change broke the outline toolbar. The first version with this problem is I20190507-1800 and reverting this change fix it. How to reproduce: - close all editor tabs - open multiple java files - select/activate the first opened java file and then close it - the outline shows duplicated toolbar icons The problem is not limited to Java outline.
Created attachment 278574 [details] Duplicated Outline Toolbar Buttons
(In reply to Paul Pazderski from comment #19) > This change broke the outline toolbar. The first version with this problem > is I20190507-1800 and reverting this change fix it. > > How to reproduce: > - close all editor tabs > - open multiple java files > - select/activate the first opened java file and then close it > - the outline shows duplicated toolbar icons > > The problem is not limited to Java outline. Yes, a bad one, I see broken console buttons etc. I see that the regression is due the new code introduced in org.eclipse.e4.ui.workbench.renderers.swt.ToolBarManagerRenderer.disposeToolBarElement(ToolBarManager, MToolBarElement). If I comment out entire content in disposeToolBarElement() regression disappears. It can be the change unhides some existing bug, but unfortunately I can't manage to run MToolItemTest in the IDE - it opens few windows and freezes, and so I'm not even able to check which tests are affected. This toolbars rendering code is a real mess. I'm going to revert the change today, so that tomorrow build is usable again - or anyone fixes that soon.
New Gerrit change created: https://git.eclipse.org/r/142004
Gerrit change https://git.eclipse.org/r/142004 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=9d0760eedabb98eef6103ca2e55c48c653296c8c
I found the root cause of the regression. To get legacy items in the E4 model, the ToolbarManager is reconciled into the model. While the model is manipulated in this step, the ToolbarManager is updated due to update model. Cause of the regression: 1. reconcileManagerToModel does not update the visibility of opaque items that are already in the model. 2. When legacy items get another position in the model, they are moved by removing and re-inserting them. By adding support for removing items, (2) will remove the opaque item from the model because of the new code. After that the item will be re-inserted into the model, and addition code is run, but because of (1) the visibility of the opaque item is incorrect causing the items to become visible. I think it will be to risky to fix this for the 4.12 cycle.
New Gerrit change created: https://git.eclipse.org/r/145577
Gerrit change https://git.eclipse.org/r/145577 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=d367093508dbaab34123d19f555ed5c94f289178
(In reply to Eclipse Genie from comment #26) > Gerrit change https://git.eclipse.org/r/145577 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/ > ?id=d367093508dbaab34123d19f555ed5c94f289178 This caused bug 549898.
(In reply to Andrey Loskutov from comment #27) > (In reply to Eclipse Genie from comment #26) > > Gerrit change https://git.eclipse.org/r/145577 was merged to [master]. > > Commit: > > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/ > > ?id=d367093508dbaab34123d19f555ed5c94f289178 > > This caused bug 549898. and bug 551067.
*** Bug 444472 has been marked as a duplicate of this bug. ***
