Community
Participate
Working Groups
I want to place a combobox inside a ToolbarManager, this Combobox changes size over time (item added/removed), so I call toolBarManager.update(true) after each change. The javadoc of that function states: > Updates this manager's underlying widget(s) with > any changes which have been made to it or its items So I would assume this also includes size changes... Sadly this has no effect, because ControlContribution.computeWidth(Control) is only called one when the Toolitem is created.
New Gerrit change created: https://git.eclipse.org/r/150576
Does your ControlContribution return true from isDynamic()? Probably it should.
The isDynamic() is only used/accounted for MenuContributions as far as I can see.
(In reply to Christoph Laeubrich from comment #3) > The isDynamic() is only used/accounted for MenuContributions as far as I can > see. Your other change from bug 551587 just made sure that ControlContributions are added to the ToolItem (via setData()), so it seems to me that https://git.eclipse.org/r/#/c/150576/1/bundles/org.eclipse.jface/src/org/eclipse/jface/action/ToolBarManager.java@298 should now apply.
Ah I see, but thats another case, that could be used when the number/type of control changes over time, but this is not the case here, just the size of the control changes. If isDynamic would be used, then the control is disposed/created each time update is called.
See also Bug 186800 comment 21, that bug seems not to be completely fixed. This but must be the leftover of that bug. Note you should be able to call getParent().update(true) on the contribution item. However that is broken in 4.x, see Bug 426499
(In reply to Rolf Theunissen from comment #6) > Note you should be able to call getParent().update(true) on the contribution > item. However that is broken in 4.x, see Bug 426499 I'm currently using ToolBarManger directly, as far as I can see the parent is set correctly.
Gerrit change https://git.eclipse.org/r/150576 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=cf899b210fc3d5da44f06bc4790727d950143b97
(In reply to Eclipse Genie from comment #8) > Gerrit change https://git.eclipse.org/r/150576 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/ > ?id=cf899b210fc3d5da44f06bc4790727d950143b97 This causes regression for EGit history view "Scope" drop-down. To reproduce: 1) Open git repositories view, select any repo 2) Open History view 3) Observe that "Scope" drop-down has no icon and is not enabled (so drop-down is not working). Please fix regression or we should revert this patch for M3.
Created attachment 280636 [details] broken egit history scope drop down
(In reply to Andrey Loskutov from comment #9) > This causes regression for EGit history view "Scope" drop-down. > To reproduce: > 1) Open git repositories view, select any repo > 2) Open History view > 3) Observe that "Scope" drop-down has no icon and is not enabled (so > drop-down is not working). Correction: expected: icon is shown, observed: nothing (blank space instead). It is OK that drop down is disabled for a *repository* (will be enabled for a concrete path inside).
The back trace to the org.eclipse.swt.widgets.ToolItem.setDisabledImage(Image) call on ActionContributionItem holding the org.eclipse.egit.ui.internal.history.GitHistoryPage$GitHistoryPageActions$FilterAction ToolItem.setDisabledImage(Image) line: 997 ActionContributionItem.updateImages(boolean) line: 1012 ActionContributionItem.updateToolItem(ToolItem, boolean, boolean, boolean, boolean, boolean) line: 778 ActionContributionItem.update(String) line: 741 ActionContributionItem.update() line: 714 SubContributionItem.update() line: 136 ToolBarManager.update(boolean) line: 337 ActionBars.updateActionBars() line: 81 SubActionBars.updateActionBars() line: 602 GitHistoryPage$SearchBar.setVisible(boolean) line: 1565 GitHistoryPage$8.run() line: 2662 So we have probably regression on SWT here (via bug 302918), uncovered by this bug. I have a patch.
Thanks Andrew for looking into this, in general I think it is always better to fix the underlying problem instead of reverting a patch that revealed a deeper problem that previously was hidden.
Marking as resolved again, bug 302918 is the one we need to be fixed.
