Community
Participate
Working Groups
Sometimes when History view is opened, the toolbar button with the tooltip text 'View Menu' is not present. How to reproduce: 1. run eclipse 2. create new general project 3. share the project: * click on project's context menu 'Team' -> 'Share Project...' * create new local repository if needed 4. switch to 'Git' perspective 5. select project's repository in 'Git Repositories' view Toolbar button 'View Menu' is not visible (see menu_not_visible.png). If this does not reproduce the issue, try closing the History view, switch between other views (Synchronize, Git Staging, Git Reflog, Properties), open the History view and select project's repository in 'Git Repositories' view again. There is an easy workaround by switching to another view (Synchronize, Git Staging) and back. After this, toolbar button suddenly appears (see menu_visible.png) My configuration: Eclipse Platform Version: 2020-09 (4.17) Build id: I20200824-0600 OS: Fedora 32 / gtk 3.24.21 Java version: 11.0.8
Created attachment 283959 [details] menu_not_visible.png
Created attachment 283960 [details] menu_visible.png
Sounds like a platform UI issue.
Forgot to mention my EGit version: 5.8.1.202007141445-r.
I can reproduce with I20200824-1900. This is not EGit issue, it is platform UI bug. Martin - is this regression in 4.17 (I guess yes)?
Rolf, could it be the regression from recent changes on bug 90757 or bug 428697?
(In reply to Andrey Loskutov from comment #6) > Rolf, could it be the regression from recent changes on bug 90757 or bug > 428697? Bug 90757 should not be related, the Toolbar and Viewmenu are created separately in the StackRenderer. Bug 428697 might be related. More likely that it is caused by previous changes in the StackRenderer, though then I expected that the issue was already seen M3 and before. Issue could be related to Bug 564290, but I haven't been able to reproduce that while investigating the issue. Will try to do some digging tonight.
(In reply to Andrey Loskutov from comment #6) > Rolf, could it be the regression from recent changes on bug 90757 or bug > 428697? If I've properly bisected that, regression seems to be already in 4.17 M1, and I can't reproduce it in 4.16. My steps: 1) have a project connected to git in the workspace. 2) Open Git perspective, close all other perspectives 3) Initially, History view is shown and it is empty, toolbar shows few buttons 4) Select repository in repositories view and History view shows the content - at this moment the view menu is not added to the toolbar, but many git buttons are added.
Regression is from bug 562892 commit 9b03a05a3907988a1a8f381e925471569875cba1.
(In reply to Rolf Theunissen from comment #7) > More likely that it is caused by previous changes in the StackRenderer, yes, I assume this change is the root cause: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/162592/10/bundles/org.eclipse.e4.ui.workbench.renderers.swt/src/org/eclipse/e4/ui/workbench/renderers/swt/StackRenderer.java
(In reply to Andrey Loskutov from comment #8) > (In reply to Andrey Loskutov from comment #6) > > Rolf, could it be the regression from recent changes on bug 90757 or bug > > 428697? > > If I've properly bisected that, regression seems to be already in 4.17 M1, > and I can't reproduce it in 4.16. > > My steps: > 1) have a project connected to git in the workspace. > 2) Open Git perspective, close all other perspectives > 3) Initially, History view is shown and it is empty, toolbar shows few > buttons > 4) Select repository in repositories view and History view shows the content > - at this moment the view menu is not added to the toolbar, but many git > buttons are added. Sorry for late reply, I see the issue in 4.17M1, but can't reproduce in 4.16 as well...
The problem is two fold: 1. The update of the top-right did not correctly respond to changes in children of a model element. 2. The childern of the menu model element are not even propagated to the menu, see Bug 543827. However, it is too risky to enable this synchronization now. I expect (and already seen) many unexpected interactions. Workaround is to call the update of the UI directly, bypassing the E4 model and renderers.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/168214
Thanks to Martin for the detailed steps to reproduce, Andrey for finding the related commit and Rolf for the fix. I can confirm that the fix works for Martins scenario. Also a quick check did not show other toolbar / view related issues. But due to my personal time constrains I'm not able to review / debug the patch in detail nor can I test a runtime Eclipse extensively. So I'm uncertain if we should merge this patch for 4.17 so shortly for the release. Rolf, you are the best to judge how dangerous the fix is for 4.17. Would you recommend to merge the patch for 4.17? Or should we want and fix this in 4.18?
I notice that not only the History view menu is affected, also the Outline view has similar issues. When the outline shows the view menu on startup (e.g. when a Java editor is already open) it does show. Otherwise it will not show, also not when an editor is opened. Only after switching tabs it will show. When the editors are closed, the view menu is not hidden. Only after switching tabs it will hide. So I think that the impact of the issue is quite high. I consider the risk in the patch low, one part of the change is basically removing duplicate code (minus a bug in the duplication). The other part is forcing an update of the UI. This update can result in too many updates, but I don't think that will result in any issues. I would recommend to merge this for 4.17.
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/168214 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=95971633200aadb08d626086d444f6778cc4656a
(In reply to Rolf Theunissen from comment #12) > 2. The childern of the menu model element are not even propagated to the > menu, see Bug 543827. However, it is too risky to enable this > synchronization now. I expect (and already seen) many unexpected > interactions. Workaround is to call the update of the UI directly, bypassing > the E4 model and renderers. Thanks, Rolf for working on this. I assume you implemented the workaround. Shall we open a new bug for the model sync or is this handled by Bug 543827?
(In reply to Lars Vogel from comment #17) > (In reply to Rolf Theunissen from comment #12) > > 2. The childern of the menu model element are not even propagated to the > > menu, see Bug 543827. However, it is too risky to enable this > > synchronization now. I expect (and already seen) many unexpected > > interactions. Workaround is to call the update of the UI directly, bypassing > > the E4 model and renderers. > > Thanks, Rolf for working on this. I assume you implemented the workaround. > Shall we open a new bug for the model sync or is this handled by Bug 543827? The sync is indeed the scope of Bug 543827. This issue again depends on the (nasty) add/remove story for toolbar and menu items. Therefore I chose for the workaround now. I do not expect a working solution for add/remove issues until late in the 4.18 cycle.
The force update call makes the toolbar in the outline view go crazy. Not all toolbar items are shown. Without the force update the view menu is not hidden always and sometimes it never comes back after it is hidden.
(In reply to Rolf Theunissen from comment #19) > The force update call makes the toolbar in the outline view go crazy. Not > all toolbar items are shown. > Without the force update the view menu is not hidden always and sometimes it > never comes back after it is hidden. The regression in this patch is caused by unexpected interactions with the patch for Bug 90757. The root cause of the unexpected interaction is in ToolBarManager.update(true), the relayout method is not executed when the new number of elements is zero, even when force is requested. This corner case is missed in the patch of Bug 186800 Comment 18. When the outline switches between tabs, it is shortly showing the default tab which doesn't have toolbar items. This together with the force layout call causes the toolbar to be rendered at a incorrect position. The relayout method is protected, and the behavior that relayout will not execute when the new number of elements is zero is clearly on documented on the class. I am not going to patch this behavior, as I consider this an API change if changed. Luckily, when the force layout is called after the toolbar is updated, the error-nous rendering does not occur either. The force layout does introduce another flicker, but that can be suppressed too. Patch will follow soon.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/168411
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/168411 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=44192018fe7ef1cbbd3bf91c0ba88b52367f52cb
I verified this. I tried to provoke the symptoms but all behaves as expected. I'm running windows.
I just checked, it works for me as well, thanks!
