Community
Participate
Working Groups
Build Identifier: 3.7 I20110127-2034 I adds toolbar item(s) through org.eclipse.ui.menus using isDynamic approach and locationURI="toolbar:...." I implement my contribution item X that extends from ContributionItem. When ToolBarManager's update() is run, its isChildVisible() calls the DynamicContributionItem's isVisible() which does not call my X's visible(). As a first trial of workaround on my side, I make my X's fill(Toolbar, int) not to add tool item if my visible state is false, I end up having two adjacent separators which are contributed by other plugins. The reason of two adjacent separators is that the dynamic item always return visible=true to toolbar manager's update() method. This messes up tb mgr's logic of handling adjacent separators as shown below, for (int i = 0; i < items.length; ++i) { IContributionItem ci = items[i]; if (!isChildVisible(ci)) { continue; } if (ci.isSeparator()) { // delay creation until necessary // (handles both adjacent separators, and separator at // end) separator = ci; } else {... Reproducible: Always Steps to Reproduce: 1. Contribute a toolbar item that sits between two separators. 2. Makes the toolbar item change its visbility to false and update toolbar manager. You will see the item always visible. If you have a workaround like I did, you will see two adjacent separators.
Created attachment 190789 [details] small fix of isVisible() Looks like making DynamicMenuContributionItem's isVisible() routing the call to loadedDynamicContribution's isVisible() will work only if loadedDynamicContribution has been set up. That means, as far as DynamicMenuContributionItem's fill() is never called, loadedDynamicContribution is null and so the DynamicMenuContributionItem class calls its base class's isVisible() that return true always -- we still have problem of two adjacent separators in this case. To solve this, I would say isVisible() should set up the loadedDynamicContribution data member if it has not as shown in this patch.
Normally our policy is that items are visible until a user action loads the real object, and then we delegate. It makes sense that an SWT.Show is a user event that will load a dynamic menu contribution ... but before we allow isVisible() to load the real object you need to confirm that simply opening a workbench window does not cause an isVisible() call (for example, if this is in the menu:file menu). PW
(In reply to comment #2) > Normally our policy is that items are visible until a user action loads the > real object, and then we delegate. > It makes sense that an SWT.Show is a user event that will load a dynamic menu > contribution ... but before we allow isVisible() to load the real object you > need to confirm that simply opening a workbench window does not cause an > isVisible() call (for example, if this is in the menu:file menu). > PW Will e4 has the same problem? If so, will it be addressed in e4?
(In reply to comment #3) > Will e4 has the same problem? If so, will it be addressed in e4? E4 will follow the same dynamic loading strategy as the workbench. We're waiting on confirmation that the patch will not trigger plugin loading (if used by a contribution in File,Edit,Help,or Windows menus) until that menu is opened. PW
We just upgraded our application to use 3.7 from 3.4. And in 3.4, this worked. We have a problem in our menu contribution item, not toolbar, but i am seeing the same behavior of failure to ask the delegated contribution item for its visibility. It happens only the very first time when showing the menu. What i found is that it checks visibility, THEN loads the delegated contribution item. so the second time you show the menu, the item's visibility is correct checked.
Has this patch been confirmed to not trigger extraneous plugin loading? If so, when will this fix be incorporated into 3.7? cheers, JT (In reply to comment #4) > (In reply to comment #3) > > Will e4 has the same problem? If so, will it be addressed in e4? > > E4 will follow the same dynamic loading strategy as the workbench. > > We're waiting on confirmation that the patch will not trigger plugin loading > (if used by a contribution in File,Edit,Help,or Windows menus) until that menu > is opened. > > PW
(In reply to comment #6) > Has this patch been confirmed to not trigger extraneous plugin loading? Not confirmed yet. > If so, when will this fix be incorporated into 3.7? No, 3.7.0 is out and 3.7.1 (Indigo SR1) is also in its final stages. It might be possible to get it into SR2, Feb 2012. PW
(In reply to comment #1) > Created attachment 190789 [details] > small fix of isVisible() It looks like this will aggressively activate any plugin that contributes with org.eclipse.ui.menus dynamic element. PW
(In reply to Paul Webster from comment #8) > (In reply to comment #1) > > Created attachment 190789 [details] > > small fix of isVisible() > > It looks like this will aggressively activate any plugin that contributes > with org.eclipse.ui.menus dynamic element. > > PW Paul, So I believe this patch is not valid and we need the new one. Could you please set the proper target for this bug? thanks, Daniel
We stepped on the same issue so interested in having this fixed too. If loading a contribution when isVisible is called causing problems, maybe then MenuManager.update should be modified to remove repeating separator *after* adding all the items.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
