Double Separators can be created with empty DynamicMenuContributions
Let's make sure issue is not already fixed in latest builds first.
- [x] I verified I can reproduce this issue against latest Integration Build of Eclipse SDK
Steps to reproduce
AFAIK, JFace has a logic to remove double Separators from Context Menus. But this only works if they are next to each other after the contributions are collected. When some of these contributions do not contribute anything, the double Separators are not removed.
I tried:
The Second Contribution in this screenshot creates a dynamic menu contribution and a separator before that.
The dynamic contribution does not add any elements (This could be the case that in the current selection, none of the possible results are valid, depending on implementation)
I expected: That the double and end separators are removed
But got:
There is a Separator at the end of the Context Menu that is not removed.
Tested under this environment:
- OS & version: Windows 11 and Linux
- Eclipse IDE/Platform version (as shown in Help > About): latest 2024-03
Community
- [x] I understand reporting an issue to this OSS project does not mandate anyone to fix it. Other contributors may consider the issue, or not, at their own convenience. The most efficient way to get it fixed is that I fix it myself and contribute it back as a good quality patch to the project.
The change https://github.com/eclipse-platform/eclipse.platform.ui/pull/1669 causes a severe regression. Almost every context menu invocation in almost every view I've tried (Package Explorer, Git Repositories, Error Log, History) results in one or multiple errors logged to the error log.
eclipse.buildId=4.32.0.I20240410-1800
java.version=21.0.2-13
java.vendor=N/A
BootLoader constants: OS=linux, ARCH=x86_64, WS=gtk, NL=en_US
Command-line arguments: -os linux -ws gtk -arch x86_64 -data /data/4x_platform_workspace
org.eclipse.ui
Error
Thu Apr 11 08:28:28 CEST 2024
Unhandled event loop exception
org.eclipse.e4.core.di.InjectionException: org.eclipse.swt.SWTException: Widget is disposed
at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:68)
at org.eclipse.swt.widgets.Synchronizer.syncExec(Synchronizer.java:183)
at org.eclipse.ui.internal.UISynchronizer.syncExec(UISynchronizer.java:133)
at org.eclipse.swt.widgets.Display.syncExec(Display.java:5960)
at org.eclipse.e4.ui.workbench.swt.DisplayUISynchronize.syncExec(DisplayUISynchronize.java:34)
at org.eclipse.e4.ui.internal.di.UIEventObjectSupplier$UIEventHandler.handleEvent(UIEventObjectSupplier.java:64)
at org.eclipse.equinox.internal.event.EventHandlerWrapper.handleEvent(EventHandlerWrapper.java:206)
at org.eclipse.equinox.internal.event.EventHandlerTracker.dispatchEvent(EventHandlerTracker.java:201)
at org.eclipse.equinox.internal.event.EventHandlerTracker.dispatchEvent(EventHandlerTracker.java:1)
at org.eclipse.osgi.framework.eventmgr.EventManager.dispatchEvent(EventManager.java:230)
at org.eclipse.osgi.framework.eventmgr.ListenerQueue.dispatchEventSynchronous(ListenerQueue.java:151)
at org.eclipse.equinox.internal.event.EventAdminImpl.dispatchEvent(EventAdminImpl.java:131)
at org.eclipse.equinox.internal.event.EventAdminImpl.sendEvent(EventAdminImpl.java:73)
at org.eclipse.equinox.internal.event.EventComponent.sendEvent(EventComponent.java:44)
at org.eclipse.e4.ui.services.internal.events.EventBroker.send(EventBroker.java:55)
at org.eclipse.e4.ui.internal.workbench.UIEventPublisher.notifyChanged(UIEventPublisher.java:60)
at org.eclipse.emf.common.notify.impl.BasicNotifierImpl.eNotify(BasicNotifierImpl.java:424)
at org.eclipse.e4.ui.model.application.ui.impl.UIElementImpl.setVisible(UIElementImpl.java:365)
at org.eclipse.e4.ui.workbench.renderers.swt.ContributionRecord.updateVisibility(ContributionRecord.java:102)
at org.eclipse.e4.ui.workbench.renderers.swt.MenuManagerRendererFilter.updateElementVisibility(MenuManagerRendererFilter.java:173)
at org.eclipse.e4.ui.workbench.renderers.swt.MenuManagerRendererFilter.updateElementVisibility(MenuManagerRendererFilter.java:184)
at org.eclipse.e4.ui.workbench.renderers.swt.MenuManagerShowProcessor.showMenu(MenuManagerShowProcessor.java:248)
at org.eclipse.e4.ui.workbench.renderers.swt.MenuManagerShowProcessor.menuAboutToHide(MenuManagerShowProcessor.java:114)
at org.eclipse.jface.internal.MenuManagerEventHelper.showEventPostHelper(MenuManagerEventHelper.java:89)
at org.eclipse.jface.action.MenuManager.handleAboutToShow(MenuManager.java:468)
at org.eclipse.jface.action.MenuManager$2.menuShown(MenuManager.java:494)
at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:272)
at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:91)
at org.eclipse.swt.widgets.Display.sendEvent(Display.java:5855)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1617)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1643)
at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1622)
at org.eclipse.swt.widgets.Menu._setVisible(Menu.java:290)
at org.eclipse.swt.widgets.Display.runPopups(Display.java:5107)
at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:4508)
at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$5.run(PartRenderingEngine.java:1151)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1042)
at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:152)
at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:639)
at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:339)
at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:546)
at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:173)
at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:152)
at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:208)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:143)
at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:109)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:439)
at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:271)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)
at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:679)
at org.eclipse.equinox.launcher.Main.basicRun(Main.java:616)
at org.eclipse.equinox.launcher.Main.run(Main.java:1492)
at org.eclipse.equinox.launcher.Main.main(Main.java:1465)
Caused by: org.eclipse.swt.SWTException: Widget is disposed
at org.eclipse.swt.SWT.error(SWT.java:4922)
at org.eclipse.swt.SWT.error(SWT.java:4837)
at org.eclipse.swt.SWT.error(SWT.java:4808)
at org.eclipse.swt.widgets.Widget.error(Widget.java:597)
at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:512)
at org.eclipse.swt.widgets.Widget.getData(Widget.java:623)
at org.eclipse.jface.action.MenuManager.update(MenuManager.java:842)
at org.eclipse.jface.action.MenuManager.update(MenuManager.java:672)
at org.eclipse.e4.ui.workbench.renderers.swt.MenuManagerRenderer.scheduleManagerUpdate(MenuManagerRenderer.java:1161)
at org.eclipse.e4.ui.workbench.renderers.swt.MenuManagerRenderer.subscribeUIElementTopicVisible(MenuManagerRenderer.java:215)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)
at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:58)
... 54 more
Please fix ASAP or revert the change.
@iloveeclipse I'm sorry I look into this right now. Maybe there changed something else in between my implementation and the merging
@iloveeclipse I'm sorry I look into this right now.
Thanks.
Maybe there changed something else in between my implementation and the merging
Not really, there were no changes in this area recently AFAIK. It can be difference between Linux/Windows SWT implementations, assuming you were testing on Windows.
Looking on the code I assume before every (overoptimistic) call to item.getData() has to be guarded by check if the item is already disposed, as the code adds extra disposals on items it iterates.
Not really, there were no changes in this area recently AFAIK. It can be difference between Linux/Windows SWT implementations, assuming you were testing on Windows.
I overlooked that in your stack trace. I developed this on Windows, I check again on GTK, I can't reproduce it right now on Windows.
@iloveeclipse Can you take a look at #1812
I'm still not able to reproduce the issue on GTK, added now isDisposed checks before accessing the data. Can you check if this works for you, without breaking something else?
I see no issue with plain SDK.
But if I install all the bundles from https://github.com/eclipse-platform/eclipse.platform.ui/blob/master/releng/org.eclipse.ui.releng/platformUiTools.p2f (via File -> Import -> Install -> Install Software Items from File) I see errors. So the "double" separators seem to be added by some bundles installed and that runs into the problem with new code.
With the nightly build, I am able to reproduce this. But still not in the Debugger. I'm currently trying to build it locally.
@iloveeclipse I was now able to verify my fix in #1812 for me, it works now without any exceptions
As noticed while reviewing possible fix: not only errors are reported, but I see now duplicated separators in the SDK which were not present in the last build.
So I'm in favour of reverting the change completely.
The change was reverted via https://github.com/eclipse-platform/eclipse.platform.ui/pull/1815