Community
Participate
Working Groups
This method is currently unimplemented. That means it cannot be used, and it is causing test failures. PW
At least one of the failing tests: org.eclipse.ui.tests.menus.MenuPopulationTest.testViewPopulation()
fails because of this bug and bug 365903 : org.eclipse.ui.tests.menus.MenuVisibilityTest.testBasicContribution() org.eclipse.ui.tests.menus.MenuVisibilityTest.testExtensionContributionExpression() org.eclipse.ui.tests.menus.MenuVisibilityTest.testVisibilityTracksEnablement() org.eclipse.ui.tests.menus.MenuPopulationTest.testMenuServiceContribution() org.eclipse.ui.tests.menus.MenuPopulationTest.testFactoryAddition() org.eclipse.ui.tests.menus.MenuPopulationTest.testDynamicFactoryAddition() org.eclipse.ui.tests.menus.MenuPopulationTest.testDynamicFactoryRemove() org.eclipse.ui.tests.menus.MenuPopulationTest.testFactoryScopePopulation() Fail because of this bug: org.eclipse.ui.tests.menus.MenuBaseTests.testBasicPopulation() org.eclipse.ui.tests.menus.MenuBaseTests.testBasicMenuPopulation() org.eclipse.ui.tests.menus.MenuPopulationTest.testMenuIcons() org.eclipse.ui.tests.menus.MenuPopulationTest.testToolBarItems() org.eclipse.ui.tests.menus.MenuPopulationTest.testAfterQueryOneGroup() org.eclipse.ui.tests.menus.MenuPopulationTest.testAfterQueryTwoGroups() org.eclipse.ui.tests.menus.MenuPopulationTest.testBeforeQueryOneGroup() org.eclipse.ui.tests.menus.MenuPopulationTest.testBeforeQueryTwoGroups() org.eclipse.ui.tests.menus.MenuPopulationTest.testBeforeQueryTwoGroups2() org.eclipse.ui.tests.menus.MenuPopulationTest.testEndofQueryOneGroup() org.eclipse.ui.tests.menus.MenuPopulationTest.testEndofQueryTwoGroups() org.eclipse.ui.tests.menus.MenuPopulationTest.testEndofQueryTwoGroups2() org.eclipse.ui.tests.menus.MenuPopulationTest.testEndofQueryTwoGroups3() org.eclipse.ui.tests.menus.MenuPopulationTest.testEndofQueryTwoGroups4() org.eclipse.ui.tests.menus.DynamicMenuTest.testDynamicMenu() org.eclipse.ui.tests.menus.DynamicMenuTest.testDynamicMenuMultiOpen() org.eclipse.ui.tests.menus.Bug231304Test.testToolTip()
org.eclipse.ui.tests.commands.CommandEnablementTest.testRestoreContributedUI() as well.
I've pushed a work-in-progress to pwebster/bug366528 PW
This is still an issue for Juno, which effectively means all clients using populateContributionManager are broken by now. In ShowInMenu this also must have been an issue, and interestingly it was rewritten with a lot of calls to the internal class ContributionsAnalyzer and menu model. Is there any workaround that can be offered to all the broken clients? IMHO this is a must-fix for Juno SR1. The importance of P3 is not justified.
(In reply to comment #5) > Is there any workaround that can be offered to all the broken clients? No, there's no workaround at this time. We're resource constrained and there's nobody to fix this for SR1 (patches welcome). There might be the possiblity to get this in for 4.2.2. PW
*** Bug 384997 has been marked as a duplicate of this bug. ***
This is critical for us as well.
The bug prevents us in the tools.cdt.tcf project to move to the Eclipse 4.x platform. We can support Eclipse 3.8 only until this is fixed.
I'd love to help the effort out by sending in a patch, but I'm not really sure where to begin on this. I can port over the 3.x series code related to action factories into the WorkbenchService and then do some minor work searching the menu contributions on the MApplication for menus provided by commands in populateContributionManager (and converting them into IContributionItems) to get the MenuPopulationTest tests (or most of them) to pass, but I'm pretty sure that wouldn't properly integrate this into the 4.x model. Does anyone familiar with the 4,x model and the compatibility layer/bridge have any pointers on where to get started, or examples? I see some related bridge code in MenuManagerRenderer. Should I look to PopupMenuExtender#addMenuContributions as an example of what to do here?
(In reply to comment #10) > Does anyone familiar with the 4,x model and the compatibility layer/bridge have > any pointers on where to get started, or examples? I see some related bridge > code in MenuManagerRenderer. Should I look to > PopupMenuExtender#addMenuContributions as an example of what to do here? You're correct, some of the work that was previously done by WorkbenchMenuManager#populateContributionManager(*) is now done by the MenuManagerRenderer. The similar populating of contributions can be seen org.eclipse.ui.internal.PopupMenuExtender.addMenuContributions(IMenuManager) The trick would be somehow creating or finding the matching MMenu (model object) to be used with that MenuManager passed in to populateContributionManager(*). If you'd like to go ahead, I'll try to answer any questions you might have. PW
Created attachment 219171 [details] Partial patch This seems to fix most of the issues for our own usage. MenuPopulationTest is down to 2 errors and 4 failures. #testMenuServiceContribution seems to fail when I run it, but if I step through in the debugger it passes. I stumbled my way through this, so who knows how well this fits into the new E4 model.
(In reply to comment #12) > Created attachment 219171 [details] > Partial patch Hi Chris. Thank you for the patch. Some comments: 1) By keeping track of the MMenu to MenuManager, the MenuManagerRenderer is providing a link between locationURI (the MMenu elementId) and the MenuManager. That information should be looked up/managed by the MenuManagerRenderer. 2) When it's time to add or remove an MMenuContribution or MToolBarContribution from the MApplication, MenuManagerRenderer (and ToolBarManagerRenderer) has to be able to add/remove ContributionRecord for the various managers. 3) in populateContributionManager, you need to support a similar pattern for ToolBarManagerRenderer. 4) in populateContributionManager, when you are looking for a place to put your MMenu, I think we have a problem. If the MenuManager (or ToolBarManager) has already created a control, then arguably we would walk up the parent chain, looking at getData(org.eclipse.e4.ui.internal.workbench.swt.AbstractPartRenderer.OWNING_ME) until we find a model element. That provides the context for us to add the MMenu to. Sometimes the views using this method for their own internal toolbars are not the active view. However, if the contribution manager hasn't created a control yet, we aren't in a position to guess where we should hang the MMenu off of. I'm not sure yet what to do in this scenario. PW
CQ:WINDE4BLOCKING This issue is blocking Wind River's adoption of Eclipse 4.x - we're going to stay on 3.x until this is fixed.
Hi, This is also blocking Obeo's adoption of Eclipse 4.2. Chris, are you still working on the patch ? Can we help ?
Paul, Chris, I'm trying to do some work on this issue. First, I'd like to inform you that I'm pretty rookie with e4 application model/framework, so excuse my comments if they seem dumb ;) I've a few questions about Paul's comments: > 1) By keeping track of the MMenu to MenuManager, the MenuManagerRenderer is > providing a link between locationURI (the MMenu elementId) and the > MenuManager. That information should be looked up/managed by the > MenuManagerRenderer. Do you suggest that this information should not be tracked down (through linkModelToManager/clearModelToManager) in the WorbenchMenuService? If so, where do you think it should be? > 2) When it's time to add or remove an MMenuContribution or > MToolBarContribution from the MApplication, MenuManagerRenderer (and > ToolBarManagerRenderer) has to be able to add/remove ContributionRecord for > the various managers. Could explain a little bit more, I don't get the point here. > 3) in populateContributionManager, you need to support a similar pattern for > ToolBarManagerRenderer. I added a similar pattern for ToolBarManagerRenderer. In fact, this is my primary use case. To do this, I needed to relax visibility of ToolBarManagerRenderer#processContribution(). What do you think about this change? > 4) in populateContributionManager, when you are looking for a place to put > your MMenu, I think we have a problem. If the MenuManager (or > ToolBarManager) has already created a control, then arguably we would walk > up the parent chain, looking at > getData(org.eclipse.e4.ui.internal.workbench.swt.AbstractPartRenderer. > OWNING_ME) until we find a model element. That provides the context for us > to add the MMenu to. Sometimes the views using this method for their own > internal toolbars are not the active view. I implemented the look up method through the parent chain like this: private Object findModelElementInParentChain(Control control) { if (control == null) { return null; } Control currentControl = control; Object owner = getOwingMe(currentControl); while (currentControl != null && owner == null) { currentControl = currentControl.getParent(); owner = getOwingMe(currentControl); } return owner; } with getOwningMe() looking for the OWNING_ME control's data. This works pretty well for ToolBarManager#getControl() but fails for all MenuManager#getMenu()#getParent() (at least in the tests of /org.eclipse.e4.ui.menu.tests/src/org/eclipse/e4/ui/menu/tests/MenuTestSuite.java). > However, if the contribution manager hasn't created a control yet, we aren't > in a position to guess where we should hang the MMenu off of. I'm not sure > yet what to do in this scenario. No idea neither :( I can provide a patch with the toolbarmanager pattern added, but it's not yet very usable.. Regards, Mikael
Created attachment 224911 [details] Partial patch 2 Improved patch of Chris' one with support for ToolbarManager. Still don't get item contributed with extension point.
Created attachment 224912 [details] Partial patch 3 (previous one was not taking all my changes into account, sorry...)
Hi, I don't manage to get my toolbar to be displayed properly. With the previous patch, the populateContributionManager manage to create and fill the MToolBar and the ToolBarManager properly, but my toolbar displays only one contribution (the first one contributed through the menuContribution extension point). Does anyone have an idea why? So far I'm stuck. If someone could give me some clue how to continue this work, I would be glad to continue to investigate. Regards,
(In reply to comment #18) > Created attachment 224912 [details] > Partial patch 3 (previous one was not taking all my changes into account, > sorry...) Hi Mikael, thank you for the contributions. I think that you're heading in the right direction. Some comments: 1) org.eclipse.ui.internal.menus.WorkbenchMenuService.addContributionFactory(AbstractContributionFactory) I don't think you can go back over the existing list of MenuManagers to re-call populateContributionManagers(*). That will re-apply all MMenuContributions, and we don't want that (only to apply the new one if relevant). We should also just rely on the MenuManagerRenderer if we need a list of MenuManager nominally "under our control" 2) in populateContributionManager(*) renderer.processContributions(*) fills in MMenuContributions but it only deals with the elementId of that MMenu or MToolBar. But populateContributionManager(*) can be called with an ID that is not the elementId. Maybe we need to update renderer.processContributions(*) so it can have its ID passed in, and use the elementId in the current calls in the renderer itself. 3) in getMenuModel/getToolBarModel. The first thing I would try is to get the MenuManagerRenderer (or TBMR) and call org.eclipse.e4.ui.workbench.renderers.swt.MenuManagerRenderer.getMenuModel(MenuManager). If it's already been linked it will be looked up. If we can't find one already registered with the renderer, then yes we'll have to create a model element for it. We could (in theory) just create the MMenu and then link it to that MenuManager for the renderer, but I guess we need to live somewhere in the lookup hierarchy for the model and the IEclipseContexts. Maybe we could try the MPart and then the MWindow, whichever one we find first. You can check out your progress by running the org.eclipse.ui.tests.menus.MenusTestSuite tests as JUnit plug-in tests. You need these plugins to run it: org.eclipse.core.tests.harness org.eclipse.test.performance org.eclipse.ui.tests org.eclipse.ui.tests.harness Once again, thank you for your contribution. Later, PW
(In reply to comment #18) > Created attachment 224912 [details] > Partial patch 3 (previous one was not taking all my changes into account, > sorry...) Mikael, I've pushed your patch to a topic branch, http://git.eclipse.org/c/platform/eclipse.platform.ui.git/log/?h=pwebster/bug366528 Then I've tried to implement some of my the changes I suggested in comment #20 The MMenu (and presumably the MToolbar which I didn't look at) both need to live in the hierarchy of either in a part or at the window level. I've added a place for toolbars to go in the MPart, and for both menus and toolbars (not related to trim) to go in the MWindow. Once this is working, we'll need a final resting place for these things. Also, because of changes in Eclipse4 most menu items are not evaluated until an SWT.Show event, that's why you'll see some of the tests changed to allow the menu model to be updated. When running the MenuTestSuite, we're down to 12 failures and no errors. If you're still interested in pursuing this, please have a look at what I've done and start running the MenuTestSUite. Thanks again, Paul
Hi Paul, Sorry for the late answer. I was on vacation the past two weeks :) I'm back and I will continue to work on this issue. Thanks for your answers and your work. I will now have a look at all of this and come back to you once I have questions and/or some results. Regards, Mikael
Created attachment 225216 [details] Partial patch 4, test failures down to 10 Hi Paul, Please find attached a patch that cut down to 10 failures the MenuTestSuite by handling popup contributions. For the other tests, here are my observations / questions: - MenuVisibilityTest#testExtensionContributionExpression() seems to fail because of the expression of visibleWhen (uses the activeContexts variable). The visibleWhen expressions seem to be are translated into application model through ContributionRecord#updateVisibility. This MExpression is then evaluated through ContributionsAnalyzer.isVisible(MCoreExpression, ExpressionContext), but I don't know if Eclipse4 is handling Eclipse3's context correctly. Any hint? - MenuPopulationTest#testMenuServiceContribution(). This one tests the contribution to a menu through an AbstractContributionFactory. It first asserts that items contributed by the factory are not in the proper menu manager, then it adds the factory to the menu service, and finally asserts that the contribution is done. This last assert fail (even with an SWT.Show event sent.). I think it was the purpose of Chris when he re-called the populateContributionManager() at the end of the addContributionFactory method. Do we need to retrieve a previously created menu manager (by relying on the MenuManagerRenderer) and do something on it? Or maybe just do a processContributions on the MMenu associated to the MenuManager? In MenuPopulationTest#testDynamicFactoryAddition(), I think this is the same issue as the previous one, but the menu is local instead of being the app menu bar. To sum up, everything is fine when doing menuService#addContributionFactory and then menuService#populateContributionManager, but not in the reverse order. Do you have any idea to suggest? Best Regards, Mikael
I've applied your patch and pushed it up to branch pwebster/bug366528. I've also fixed 2 tests. testMenuIcons and testToolBarItems to deal with HandledContributionItems. testToolBarItems is still failing, but now it's because HandledContributionItems is not retrieving the correct image from ICommandImageService (or the model element is not being correctly fed the image URI from MenuPersistence). Mikael, to pick this up you might have to fetch from origin, and then do a reset to origin/pwebster/bug366528 ... since I rebased on R4_2_maintenance it's changed all the commits and you probably don't want to do a pull. PW
I've provided an alternate implementation of what we've got so far in http://git.eclipse.org/c/platform/eclipse.platform.ui.git/log/?h=pwebster/bug366528-2 It doesn't store the toolbars or menus in the model, but treats them as transient data. Other than that, it's the same as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/log/?h=pwebster/bug366528 The implication is no model change, but the context lookup code has to be modified everywhere. I think I'll go back to working from http://git.eclipse.org/c/platform/eclipse.platform.ui.git/log/?h=pwebster/bug366528 PW
Created attachment 225598 [details] thoughts on adding a MMenuContribution A sketch of adding an MMenuContribution and trying to use it on existing MMenu models. It's not using the correct IEclipseContext to try and merge in the contribution record. PW
Thanks for the hard work on this so far. This bug is still a key blocker for us why we cannot adopt Eclipse 4. It would be great to see this in 4.2.2 , is that realistically possible ?
(In reply to comment #27) > Thanks for the hard work on this so far. > This bug is still a key blocker for us why we cannot adopt Eclipse 4. > > It would be great to see this in 4.2.2 , is that realistically possible ? Not at this time. The 3 biggest things missing from the 2 experimental branches: 1) dynamically adding or removing contributions when the contribution factory goes away 2) Correctly cleaning up on a releaseContributions(*) 3) fixing all calls to these methods in the workbench In its current state, #2 is the killer because it leaks contributions and all associated links. Unless plans change, I'm planning to push this into 4.2.2+, but it's not ready for 4.2.2 (risk is high). PW
I lack time to work on this bug but the bug is still a blocker too. Paul, do you have any plan for m6 or m7? Thank you.
(In reply to comment #29) > I lack time to work on this bug but the bug is still a blocker too. Paul, do > you have any plan for m6 or m7? Thank you. This is one of the big 3 that I still hope to get into M7. PW
Sorry, seems I accidentally set the security bit. PW
OK, work in progress at https://git.eclipse.org/r/#/c/12027/ and https://git.eclipse.org/r/#/c/12028/ I'll be doing my work in https://git.eclipse.org/r/#/c/12028/ going forward PW
Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=73688b8c91e945b9589f95facdabbfff64216025 and http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=6695c13c77fe66765f49962f0dae9fc560de09ef PW
In 4.3.0.I20130516-2200 The rest of the test failures are tracked in bug 366520 PW
Created attachment 232176 [details] anyVisible draft patch to handle Factory case in toolbar. Hi, I discovered some issues on Kepler RC2 for toolbar contributions. First, I observe two different behaviors: only one contribution is added to our toolbar in a bundle context whereas all contributions are visible in my development environment with a Kepler RC2 target platform. The behavior is the same (only one contribution) for a runtime launched from the bundle. I tried to investigate without result: in debug using the Eclipse Remote Java Application, I did not see any differences for the moment. All our menu contributions (org.eclipse.ui.menus extension point) inherit from org.eclipse.ui.menus.ExtensionContributionFactory. Their createContributionItems methods are called when we use IMenuService#populateContributionManager(*). In both context, the corresponding org.eclipse.e4.ui.model.application.ui.menu.impl.OpaqueMenuItemImpl are created. But in the bundle context, only the item correspondign to the first found extension is visible in the toolbar. The second issue deals with the visibility of the contribution. Our contribution factories create IContributionItem with visible when expressions. These visibleWhen expressions are never evaluated. Our visibleWhen programmatic expression seems to be well added to the application model as MCoreExpression of the MUIElement(MOpaqueToolITem) created from our IContributionItem. ToolBarContributionRecord.mergeIntoModel() detects the use of a factory and call mergeFactoryIntoModel(), otherwise it direclty call MToolBarContribution.getChildren(). However, during the next phase, ToolbarContributionRecord does not handle the factory case and always return false for our contributions. I discovered a linked issue when I tried to handle the factory case, a ClassCastException occurs in ToolBarManagerRenderer.toBeRenderedUpdater EventHandler (l 162): the ici IContributionItem is directly casted into a ContributionItem to get its parent, but there are other possible IContributionItems with or without getParent() method. In our case, we have some org.eclipse.jface.action.MenuManager. Other types like IToolbarContributionManager (with the use of getToolbarManager instead of getParent) might require to be added too. The third issue occurs only in the target platform context: I get an NPE in the ToolBarManager.update(boolean) method of the toolbar manager on which the IMenuService#populateContributionManager(*) is called. In our context, we create a toolbar in an editor. When the editor is closed and our toolbar disposed, we continue to receive calls on the update method. We can avoid the exception by checking the NPE on getControl().. But the real issue is thaht some parts of the application model do not seem to be released whereas the corresponding control are disposed: in the model, I was able to find our contributions even after the toolbar disposal. Furthermore, our toolbar refresh seems to take more time to refresh with 4.x (compared to 3.x) and we see the contribution becoming visible / non visible (it looks like a 'blink' effect). It might come from the non disposed/released element for old contributions. Regards, Maxime
Issue reopened: see Maxime explanations in the comment above
Please open new bugs for any issues that now surface. PW
.
Hi, I have just created Bug 410432 and Bug 410436 to deal with the issues I reported here yesterday. Regards Maxime
Excuse me for the spam, I wanted to point Bug 410432 and Bug 410426. Regards Maxime (In reply to comment #39) > Hi, > > I have just created Bug 410432 and Bug 410436 to deal with the issues I > reported here yesterday. > > > Regards > > Maxime
I still don't get item contributed with extension point. Is this fixed or move into another bug? My Code: I debug this and see that the extension point doesn't evaluate. So how should the popluated work? My Code:[code] // Create cool bar ToolBarManager configuratorToolBarManager = new ToolBarManager(SWT.HORIZONTAL); IWorkbenchWindow workbenchWindow = PlatformUI.getWorkbench().getActiveWorkbenchWindow(); IMenuService menuService = (IMenuService)workbenchWindow.getService(IMenuService.class); if (menuService != null) { menuService.populateContributionManager(configuratorToolBarManager, "toolbar:my.toolbar"); configuratorToolBarManager.update(true); System.out.println(configuratorToolBarManager.getSize()); System.out.println(configuratorToolBarManager.getItems().length); } [/code] [code] <extension point="org.eclipse.ui.menus"> <menuContribution allPopups="false" locationURI="toolbar:my.toolbar"> <toolbar id="myToolbar2"> <command id="myToolBarItem2" label="MyToolbarItem2" style="push" tooltip="MyToolbarItem2"> </command> </toolbar> </menuContribution> [/code]
I find this lines of Code in the MenuAdditionCacheEntry and so my code and extension point can't work, but i don't understand why the main toolbar has a special treatment and can have submenus and "normal" toolbars not? if (inToolbar()) { if (isInWorkbenchTrim(location)) { processTrimChildren(trimContributions, toolBarContributions, configElement); } else { String query = location.getQuery(); if (query == null || query.length() == 0) { query = "after=additions"; //$NON-NLS-1$ } processToolbarChildren(toolBarContributions, configElement, location.getPath(), query); } return; } In the process Childen there are no support for sub toolbars if (IWorkbenchRegistryConstants.TAG_COMMAND.equals(itemType)) { MToolBarElement element = createToolBarCommandAddition(items[i]); toolBarContribution.getChildren().add(element); } else if (IWorkbenchRegistryConstants.TAG_SEPARATOR.equals(itemType)) { MToolBarElement element = createToolBarSeparatorAddition(items[i]); toolBarContribution.getChildren().add(element); } else if (IWorkbenchRegistryConstants.TAG_CONTROL.equals(itemType)) { MToolBarElement element = createToolControlAddition(items[i]); toolBarContribution.getChildren().add(element); }
(In reply to comment #41) > I still don't get item contributed with extension point. Is this fixed or > move into another bug? You can open another bug. > [code] > <extension > point="org.eclipse.ui.menus"> > <menuContribution > allPopups="false" > locationURI="toolbar:my.toolbar"> > <toolbar > id="myToolbar2"> > <command > id="myToolBarItem2" > label="MyToolbarItem2" > style="push" > tooltip="MyToolbarItem2"> > </command> > </toolbar> > </menuContribution> > [/code] This code cannot be applied to an existing toolbar. Since a toolbar is your target (what you called populateContributionManager(*) on) you wouldn't have toolbar/@id='myToolbar2'. You would contribute your command directly to my.toolbar PW
