Bug 551914

Summary: SubContributionItem maintains its own visible state
Product: [Eclipse Project] Platform Reporter: Andrey Loskutov <loskutov>
Component: UIAssignee: Platform-UI-Inbox <Platform-UI-Inbox>
Status: NEW --- QA Contact:
Severity: enhancement    
Priority: P3 CC: rolf.theunissen
Version: 4.14   
Target Milestone: ---   
Hardware: All   
OS: All   
See Also: https://git.eclipse.org/r/150762
https://bugs.eclipse.org/bugs/show_bug.cgi?id=551067
https://bugs.eclipse.org/bugs/show_bug.cgi?id=201589
Whiteboard:

Description Andrey Loskutov CLA 2019-10-08 07:08:45 EDT
We stumbled over this problem in bug 551067 comment 13, where EGit toolbar contribution was not shown again after hiding, because the ControlContribution of EGit History toolbar was wrapped into SubContributionItem that maintained its *own* visibility state so that it failed through visibility check in ToolBarManager.update(boolean).

As a "quick hack" I've pushed patch https://git.eclipse.org/r/150713 that forced that ControlContribution will push the changes of its visibility up to the containing SubContributionItem.

This "quick hack" is questionable because for some reason the SubContributionItem has its own visibility defined, and also java doc says that SubContributionItem "is used within a <code>SubContributionManager</code> to control the visibility of items."

But I honestly don't see why SubContributionManager needs the extra flag inside managed SubContributionItem's if they simply can refer to the original item's visibility.

The one possible explanation would be that the *same* contribution *object* could be created and used in *different* toolbars / menus at same time, so that the state of each one can be managed independently.

However, since ControlContribution is SWT based object, it can't be shared between different parent controls by definition of SWT API.

The questions are also:

1) Do we need the extra visibility state in SubContributionItem? If yes, why. If not - how to check this? Patch https://git.eclipse.org/r/150762 shows that we have no tests at all for this part of the code.

2) What would be the "right" approach to fix the visibility state of ControlContribution / EGit "Find" contribution to the History view?
Comment 1 Rolf Theunissen CLA 2019-10-08 09:10:54 EDT
Note that there are many open bugs related to visibility of (toolbar) items, see for instance Bug 201589.