Community
Participate
Working Groups
Created attachment 232212 [details] anyVisibleWhen draft patch to handle Factory case in toolbar Hi, VisibleWhen expression of IContributionItems created by a toolbar contribution factory are never evaluated on Kepler RC2. We have a toolbar populated through IMenuService#populateContributionManager(*). 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(*) and some org.eclipse.e4.ui.model.application.ui.menu.impl.OpaqueMenuItemImpl are created. Our contribution factories create IContributionItem with visible when expressions. These visibleWhen expressions are never evaluated, but they 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 directly 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, but other types like IToolbarContributionManager (with the use of getToolbarManager instead of getParent) might require to be added too. You attach a draft patch which allow to handle the factory case in ToolBarContributionRecord.anyVisibleWhen() and to correct the ClassCastException I could observe with our contributions.
The provided draft patch allows to look into toolbarModel.getChildren() in the FACTORY case in order to avoid to re-createContribution by calling mergeIntoModel. The old behavior was to check only the toolbarContribution.getChildren(), but the returned list is empty when a contribution factory is used.
IContributionItem without visibleWhen might require a specific treatment (as mentionned in https://bugs.eclipse.org/bugs/show_bug.cgi?id=410432). In the draft attached patch, I just reused child.getPersistedState().get(MenuManagerRenderer.VISIBILITY_IDENTIFIER) != null but it might not be sufficient.
Any news on this? From a quick test using Luna M5 it seems we get the same behavior as we had for Kepler: all the contributions are always visible, whatever the selection is. For Sirius we had to create a "degraded mode" enabled when we run under 4.x, but we would have like to be able to disable the workaround for Luna.
No one is looking at this at the moment, but we would accept a patch. https://wiki.eclipse.org/Platform_UI/How_to_Contribute PW
Hi, I submitted a review on Gerrit: https://git.eclipse.org/r/21602 Could you give me some tips to find an existing test to modify to checks the behavior of toolbar populated from factories with contributions with visibleWhen ? or a path too an existing test on toolbars (in order to start on a good base and in the right place).
Thanks Maxime for that contribution. The menu contribution related tests are in /org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenusTestSuite.java They should be runnable as Plug-in Tests. There are failures now related to the MenuPopulationTests for various factory contributions. That's probably a good place to add a factory test with visibleWhen. PW
Thanks Paul, but I am not looking for menu contributions tests: MenuManagerRenderer and ContributionRecord are not impacted by this bug, only ToolbarManagerRenderer and ToolBarContributionRecord. So I am looking for toolbar contribution tests.
I missed Bug231304Test.java and which is a toolbar test andis ine the MenusTestSuite. Thanks for the tip. Maxime
I have updated the patch set, it now contains the corrections (ClassCastException and visibleWhen inspection) and a test class. Regards Maxime
Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=f27f2fad5a22a5a415e12dc26aaa474f3a7051cb Thanks Maxime PW
In 4.4.0.I20140303-2000 PW
