Community
Participate
Working Groups
The bug has been detected on Sirius: during the opening of a diagram editor, GMF provide several contributions to the action bars (org.eclipse.ui.internal.EditorReference.createEditorActionBars(WorkbenchPage, EditorDescriptor, IEditorSite)). Several items are MenuManager with menus items, these contributions are then represented by DropDown buttons in toolbar. The WorkbenchWindow coolbar manager is a org.eclipse.ui.internal.CoolBarToTrimManager. During its update it adds all menu items to the toolbar. It should add only the MenuManager. I will attach later a minimal reproduction case. In Sirius, we let GMF initalize its actions bars and then we remove some items from those bars. During the editor opening we can see each add operation in the action bars. We found a temporary workaround (Bug 430092) to defer the update of the action bar. I think org.eclipse.ui.internal.EditorReference.initialize(IWorkbenchPart) could use WorkbenchWindow.largeUpdateStart() and largeUpdateEnd() around the action bar initialization to improve the user perception. Detected during analysis of Bug 430092 after Bug 410426 and Bug 428322 corrections.
Detected on Sirius+Luna M6
Created attachment 240854 [details] Issues in Logic diagam action bar Step to reproduce: . Take an Eclipse Luna M6 . Install GMF runtime, sdk and samples. . Create a project . Create a Logic Diagram (New > Example ... GMF (Graphical Modeling Framework) Diagrams > Logic Diagram) -> DropDown menu items are empty, their expected children are in the action bar (see Select, Align, Arrange actions) -> Several items are duplicated (Font) The action bar is populated by org.eclipse.gmf.examples.runtime.diagram.logic.internal.ui.parts.LogicDiagramActionBarContributor. The sample plugins are available in New > Example ... GMF (Graphical Modeling Framework) Plug-ins> Logic.
Item duplication sample: FontNameContributionItem.createControl() is called twice. First createControl() call -> the added item is a DirectToolItemImpl created from the toolbarFontGroup contribution item. Thread [main] (Suspended (breakpoint at line 58 in FontNameContributionItem)) FontNameContributionItem.createControl(Composite) line: 58 FontNameContributionItem(AbstractContributionItem).createToolItem(ToolBar, int) line: 355 FontNameContributionItem(AbstractContributionItem).fill(ToolBar, int) line: 334 ToolBarManager2(ToolBarManager).update(boolean) line: 371 ToolBarManagerRenderer.processContents(MElementContainer<MUIElement>) line: 548 ToolBarManagerRenderer$6.handleEvent(Event) line: 245 UIEventHandler$1.run() line: 41 UISynchronizer(Synchronizer).syncExec(Runnable) line: 187 UISynchronizer.syncExec(Runnable) line: 150 Display.syncExec(Runnable) line: 4731 E4Application$1.syncExec(Runnable) line: 210 UIEventHandler.handleEvent(Event) line: 38 EventHandlerWrapper.handleEvent(Event, Permission) line: 197 EventHandlerTracker.dispatchEvent(EventHandlerWrapper, Permission, int, Event) line: 197 EventHandlerTracker.dispatchEvent(Object, Object, int, Object) line: 1 EventManager.dispatchEvent(Set<Entry<K,V>>, EventDispatcher<K,V,E>, int, E) line: 230 ListenerQueue<K,V,E>.dispatchEventSynchronous(int, E) line: 148 EventAdminImpl.dispatchEvent(Event, boolean) line: 135 EventAdminImpl.sendEvent(Event) line: 78 EventComponent.sendEvent(Event) line: 39 EventBroker.send(String, Object) line: 80 UIEventPublisher.notifyChanged(Notification) line: 58 ToolBarImpl(BasicNotifierImpl).eNotify(Notification) line: 374 ToolBarImpl$1(EcoreEList<E>).dispatchNotification(Notification) line: 249 ToolBarImpl$1(NotifyingListImpl<E>).addUnique(E) line: 294 ToolBarImpl$1(AbstractEList<E>).add(E) line: 303 CoolBarToTrimManager.fill(MToolBar, IContributionManager) line: 746 CoolBarToTrimManager.update(boolean) line: 666 WorkbenchWindow.updateActionBars() line: 2202 WWinActionBars.updateActionBars() line: 113 EditorActionBars(SubActionBars).updateActionBars() line: 611 LogicDiagramActionBarContributor(DiagramActionBarContributor).init(IActionBars) line: 85 LogicDiagramActionBarContributor(EditorActionBarContributor).init(IActionBars, IWorkbenchPage) line: 146 EditorReference.createEditorActionBars(WorkbenchPage, EditorDescriptor, IEditorSite) line: 469 EditorReference.initialize(IWorkbenchPart) line: 381 CompatibilityEditor(CompatibilityPart).create() line: 301 ... Second createControl() call: same stack but the added item is a DirectToolItemImpl created from the toolbarFontName contribution item. The font name combo is created twice. During one update, ToolBarManager2(ToolBarManager).update(boolean) is not able to detect that the item is already in the toolbar.
In facts, org.eclipse.jface.action.ContributionManager.contributions contains doublons. I am looking for the element which adds several items twice.
The duplication issue comes from org.eclipse.e4.ui.workbench.renderers.swt.ToolBarManagerRenderer.addToManager(ToolBarManager, MToolBarElement, IContributionItem) which adds the item to the current manager whereas the items is already in its contributions. I see several possibilities: * in org.eclipse.jface.action.ToolBarManager.update(boolean), the 'clean' list could be replaced by a LinkedHashSet to avoid duplication -> I'm not sure we should change this. * use a specific ToolbarManager which does not allow duplication (see org.eclipse.jface.action.ContributionManager.allowItem(IContributionItem) which could be overriden). * in ToolBarManagerRenderer.addToManager(): check that the current contribution is not already present in the contribution lists. GMF init the contributions in org.eclipse.gmf.runtime.diagram.ui.parts.DiagramActionBarContributor.init(IActionBars)
My last comment removed the 4.4 M7 target milestone set by Paul Webster.
See https://git.eclipse.org/r/23347 for a correction proposal. It corrects the menu manager problem and the doublons issue.
*** Bug 429900 has been marked as a duplicate of this bug. ***
I have updated the patch. Instead of checking if a contribution is already in a ToolBarManager, I changed the order of adding contributions to model and linking model elements with contributions in the ToolBarManagerRenderer. Because of missing permissions I was not able to push the patch to gerrit with the author/commiter/signed-off fields filled properly. Please correct them before pushing to master.
I applied the patch from Maxime and Wojciech for the duplicate separator issues with https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=16955c8ac94ed8fbad83c8e4c188be1d04a9824a
*** Bug 430593 has been marked as a duplicate of this bug. ***
I have checked the fix from Wojciech for the duplication issue on Sirius and GMF Logic sample, it works fine with it (and so the check I added in ToolbarManagerRenderer in my first patch is not more needed).
(In reply to Maxime Porhel from comment #12) > I have checked the fix from Wojciech for the duplication issue on Sirius and > GMF Logic sample, it works fine with it (and so the check I added in > ToolbarManagerRenderer in my first patch is not more needed). Maxime, What does skipping MenuManagers in the CoolBarToTrimManager do? Are we really incorrectly feeding MenuManagers into it somewhere? PW
(In reply to Paul Webster from comment #13) > (In reply to Maxime Porhel from comment #12) > > I have checked the fix from Wojciech for the duplication issue on Sirius and > > GMF Logic sample, it works fine with it (and so the check I added in > > ToolbarManagerRenderer in my first patch is not more needed). > > Maxime, > > What does skipping MenuManagers in the CoolBarToTrimManager do? Are we > really incorrectly feeding MenuManagers into it somewhere? > > PW Without skipping MenuManagers in the CoolBarToTrimManager, all GMF org.eclipse.gmf.runtime.diagram.ui.actions.internal.ActionMenuManager (Align, Arrange, Select, ...) contributed to the action bars will have their items directly added to the toolbar whereas they are expected to be displayed as in drop down menu when the user clics on the drop down button created from the ActionMenuManager contribution. See the second attached screenshot.
Created attachment 241469 [details] MenuManagers children are directly added in the toolbar.
(In reply to Maxime Porhel from comment #12) > I have checked the fix from Wojciech for the duplication issue on Sirius and > GMF Logic sample, it works fine with it (and so the check I added in > ToolbarManagerRenderer in my first patch is not more needed). The patch set 5 (https://git.eclipse.org/r/#/c/23347/) now contains a fix which corrects the menu manager children issue. (Works on Sirius and GMF Logic sample)
(In reply to Maxime Porhel from comment #16) > > The patch set 5 (https://git.eclipse.org/r/#/c/23347/) now contains a fix > which corrects the menu manager children issue. (Works on Sirius and GMF > Logic sample) Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=85bd30aaaf10bdc4caa47f449eacb635aa6264e5 Thanks Maxime. PW
(In reply to Lars Vogel from comment #10) > I applied the patch from Maxime and Wojciech for the duplicate separator > issues with > https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/ > ?id=16955c8ac94ed8fbad83c8e4c188be1d04a9824a This fixes the duplicate separators issue (bug 430593) for me, using eclipse-SDK-N20140331-2000-win32-x86_64.
Verified in I20140429-2000.
