Bugzilla – Full Text Bug Listing
| Summary: | [Compatibility] All views now have view menus | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Remy Suen <remy.suen> | ||||||||||||||||||||||
| Component: | UI | Assignee: | Remy Suen <remy.suen> | ||||||||||||||||||||||
| Status: | VERIFIED FIXED | QA Contact: | Remy Suen <remy.suen> | ||||||||||||||||||||||
| Severity: | normal | ||||||||||||||||||||||||
| Priority: | P3 | CC: | daniel_megert, david_williams, j.mairboeck, Lars.Vogel, pawel.1.piech, pinnamur, pwebster, rollsisu, stephan.herrmann | ||||||||||||||||||||||
| Version: | 4.1 | ||||||||||||||||||||||||
| Target Milestone: | 4.2 M6 | ||||||||||||||||||||||||
| Hardware: | All | ||||||||||||||||||||||||
| OS: | All | ||||||||||||||||||||||||
| Whiteboard: | |||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||
|
Description
Remy Suen
I have potential fix in my workspace for this bug. I will update the bug with patch after I verify all the use-cases. Created attachment 199543 [details]
Patch v01
Find the attached patch that allows the ViewMenu to be created only when it is required to the part. Please review and post your comments. Thanks.
(In reply to comment #2) > Created attachment 199543 [details] > Patch v01 This works for the "startup" case but if the view adds contribution items to the menu manager later and asks it to update, the view menu does not get rendered. *** Bug 356356 has been marked as a duplicate of this bug. *** Created attachment 203296 [details]
alternative patch to Patch_v01 addressing issue of items added to menu
I also have interest in this bug for an RCP application I am uplifting to E4. I don't have any relationship with the original patch submitter so I have not spoken with him/her about this.
I don't know that my patch is necessarily the optimal way to fix this bug, but it seems to work in my testing. It addresses showing or hiding the menu as items are added or removed from the menu.
I am hopeful that my patch may help in progressing towards a solution to this issue. If there is a better way to address the issue and someone can guide me to towards it I may be able to provide an improved patch.
(In reply to comment #5) > Created attachment 203296 [details] > alternative patch to Patch_v01 addressing issue of items added to menu It feels to me that the rendering and unrendering of the tool item should be driven by the action bars when updateActionBars() gets called. That method is how clients should be informing the workbench about changes to the menu. Created attachment 203433 [details]
alternative patch to Patch_v01 addressing issue of items added to menu (v2)
Good point Remy. IActionBars.getMenuManager() does mention that updateActionBars() should be called. This new version of the patch replaces the previous patch, driving the update off of updateActionBars() instead of MenuManager.update().
I'm guessing this is not going to make it into 4.1.1? Maybe it needs a new target milestone? (In reply to comment #8) > I'm guessing this is not going to make it into 4.1.1? Maybe it needs a new > target milestone? You're right Ben, we don't plan on making anymore changes to 4.1.1 unless they are critical defects. (In reply to comment #7) > Created attachment 203433 [details] > alternative patch to Patch_v01 addressing issue of items added to menu (v2) I wasn't able to apply this patch to my workspace anymore. I did some copy/pasting and applied it by hand but my 'Problems' view doesn't have a menu item. Maybe I made a mistake with my copy/paste work? I will try to create a new patch on top of the latest code in the git repository and also double check that the problems view menu functions correctly. There wasn't a problem with your copy/paste -- the patch just doesn't work correctly for some cases like the Problems view. I'll see if I can come up with something that actually works. Sorry! Created attachment 205347 [details] Patch v03 Here is a new patch that addresses problems with the previously posted patches. The patch is based on the latest (as of this morning) 'master' in the following git repositories: http://git.eclipse.org/gitroot/e4/eclipse.platform.ui.compat.git http://git.eclipse.org/gitroot/e4/eclipse.platform.ui.e4.git Hopefully you should not have any trouble applying the patch. I tested applying the patch to master through the Team -> Apply Patch... wizard. One thing I was a little bit uncomfortable with is the way that I had to call renderer.createGui() to figure out if the menu should be shown. You can see this in my changes in StackRenderer.java. I'm not sure if that change is appropriate or not. I'm seeing some exception after dragging views into the shared area and resetting the perspective repeatedly. java.lang.NullPointerException at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.hasVisibleViewMenuItems(StackRenderer.java:1064) at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.setupMenuButton(StackRenderer.java:744) at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer.adjustTR(StackRenderer.java:444) at org.eclipse.ui.internal.e4.compatibility.CompatibilityView$1.actionBarsUpdated(CompatibilityView.java:150) at org.eclipse.ui.internal.e4.compatibility.ActionBars.updateActionBars(ActionBars.java:127) at org.eclipse.jdt.debug.ui.breakpoints.JavaBreakpointConditionEditor.deactivateHandlers(JavaBreakpointConditionEditor.java:530) at org.eclipse.jdt.debug.ui.breakpoints.JavaBreakpointConditionEditor.dispose(JavaBreakpointConditionEditor.java:433) at org.eclipse.jdt.debug.ui.breakpoints.JavaBreakpointConditionEditor$11.widgetDisposed(JavaBreakpointConditionEditor.java:421) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:123) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1053) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1077) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1058) at org.eclipse.swt.widgets.Widget.release(Widget.java:808) at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:873) at org.eclipse.swt.widgets.Widget.release(Widget.java:811) at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:873) at org.eclipse.swt.widgets.Widget.release(Widget.java:811) at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:873) at org.eclipse.swt.widgets.Widget.release(Widget.java:811) at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:873) at org.eclipse.swt.widgets.Widget.release(Widget.java:811) at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:873) at org.eclipse.swt.widgets.Widget.release(Widget.java:811) at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:873) at org.eclipse.swt.widgets.Widget.release(Widget.java:811) at org.eclipse.swt.widgets.Composite.releaseChildren(Composite.java:873) at org.eclipse.swt.widgets.Widget.release(Widget.java:811) at org.eclipse.swt.widgets.Widget.dispose(Widget.java:446) at org.eclipse.e4.ui.workbench.renderers.swt.SWTPartRenderer.disposeWidget(SWTPartRenderer.java:137) at org.eclipse.e4.ui.workbench.renderers.swt.ContributedPartRenderer.disposeWidget(ContributedPartRenderer.java:270) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.safeRemoveGui(PartRenderingEngine.java:812) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.access$3(PartRenderingEngine.java:756) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$8.run(PartRenderingEngine.java:751) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.removeGui(PartRenderingEngine.java:736) at org.eclipse.e4.ui.workbench.renderers.swt.ElementReferenceRenderer.disposeWidget(ElementReferenceRenderer.java:122) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.safeRemoveGui(PartRenderingEngine.java:812) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.access$3(PartRenderingEngine.java:756) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$8.run(PartRenderingEngine.java:751) at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.removeGui(PartRenderingEngine.java:736) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$1.handleEvent(PartRenderingEngine.java:137) at org.eclipse.e4.ui.services.internal.events.UIEventHandler$1.run(UIEventHandler.java:41) at org.eclipse.swt.widgets.Synchronizer.syncExec(Synchronizer.java:180) at org.eclipse.ui.internal.UISynchronizer.syncExec(UISynchronizer.java:150) at org.eclipse.swt.widgets.Display.syncExec(Display.java:4683) at org.eclipse.e4.ui.internal.workbench.swt.E4Application$1.syncExec(E4Application.java:184) at org.eclipse.e4.ui.services.internal.events.UIEventHandler.handleEvent(UIEventHandler.java:38) at org.eclipse.equinox.internal.event.EventHandlerWrapper.handleEvent(EventHandlerWrapper.java:197) at org.eclipse.equinox.internal.event.EventHandlerTracker.dispatchEvent(EventHandlerTracker.java:197) 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:148) at org.eclipse.equinox.internal.event.EventAdminImpl.dispatchEvent(EventAdminImpl.java:135) at org.eclipse.equinox.internal.event.EventAdminImpl.sendEvent(EventAdminImpl.java:78) at org.eclipse.equinox.internal.event.EventComponent.sendEvent(EventComponent.java:39) at org.eclipse.e4.ui.services.internal.events.EventBroker.send(EventBroker.java:81) at org.eclipse.e4.ui.internal.workbench.UIEventPublisher.notifyChanged(UIEventPublisher.java:55) at org.eclipse.emf.common.notify.impl.BasicNotifierImpl.eNotify(BasicNotifierImpl.java:380) at org.eclipse.e4.ui.model.application.ui.impl.UIElementImpl.setToBeRendered(UIElementImpl.java:290) at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(PartServiceImpl.java:1068) at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.hidePart(PartServiceImpl.java:1007) at org.eclipse.e4.ui.internal.workbench.ModelServiceImpl.resetPerspectiveModel(ModelServiceImpl.java:732) at org.eclipse.e4.ui.internal.workbench.ModelServiceImpl.resetPerspectiveModel(ModelServiceImpl.java:714) at org.eclipse.ui.internal.WorkbenchPage.resetPerspective(WorkbenchPage.java:2952) at org.eclipse.ui.internal.handlers.ResetPerspectiveHandler.execute(ResetPerspectiveHandler.java:81) Thanks for trying the patch Remy. I guess there must be some problem with it but unfortunately I haven't had time in the last couple of days to dig into it. I'm not sure when exactly I might get to looking at it again. How do you want to handle this? I don't know if you're waiting for a better patch from me or if maybe you or some other committer are going to pick this bug up yourself and finish it out? I don't know what you think of the code I have in the patch to begin with. Does it even look like it's on the right track? I appreciate your consideration of the patches I have submitted on this bug and others. Thanks for taking the time to look at them. Created attachment 206622 [details]
Patch v04
Still a little rough around the edges when dealing with PBVs.
(In reply to comment #16) > Still a little rough around the edges when dealing with PBVs. Technically speaking, if I remove the dispose() call that we added a while back the problem seems to go away. http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=16c4113404e0dcfa86e8bdcc3a14f89dc56e97fd May not be directly related but should probably fix bug 363340 first so that the part's list of menus is accurate and sane. Created attachment 206732 [details]
Patch v05
(In reply to comment #19) > Created attachment 206732 [details] > Patch v05 Still no good, I can get into a state where the 'Outline' view's view menu doesn't show anything. (In reply to comment #20) > Still no good, I can get into a state where the 'Outline' view's view menu > doesn't show anything. Seems to happen on a restart. The SWT menu is created before it is populated with contribution items. When a source changes and causes a view menu to appear (because one of its child menu items have become visible) in 3.x, this happens. I'm not sure our rendering story is setup in such a way to handle this at the moment.
Thread [main] (Suspended (breakpoint at line 3124 in CTabFolder))
CTabFolder.setTopRight(Control, int) line: 3124
PaneFolder.layout(boolean) line: 487
DefaultTabFolder.layout(boolean) line: 403
PresentablePartFolder.layout(boolean) line: 408
PresentablePartFolder.childPropertyChanged(IPresentablePart, int) line: 325
PresentablePartFolder.access$2(PresentablePartFolder, IPresentablePart, int) line: 303
PresentablePartFolder$3.propertyChanged(Object, int) line: 83
PresentablePart.firePropertyChange(int) line: 137
PresentablePart$1.propertyChanged(Object, int) line: 97
ViewPane(PartPane).firePropertyChange(int) line: 625
ViewPane$PaneMenuManager.update(boolean, boolean) line: 112
ViewPane$PaneMenuManager(MenuManager).update(boolean) line: 682
WorkbenchMenuService.updateManagers() line: 330
WorkbenchMenuService$4.propertyChange(PropertyChangeEvent) line: 316
EvaluationAuthority$1.run() line: 252
SafeRunner.run(ISafeRunnable) line: 42
EvaluationAuthority.fireServiceChange(String, Object, Object) line: 246
EvaluationAuthority.endSourceChange(String[]) line: 197
EvaluationAuthority.sourceChanged(String[]) line: 135
EvaluationAuthority(ExpressionAuthority).sourceChanged(int, String[]) line: 311
EvaluationAuthority(ExpressionAuthority).sourceChanged(int, Map) line: 290
AbstractSourceProvider(AbstractSourceProvider).fireSourceChanged(int, Map) line: 99
AbstractSourceProvider.change() line: 25
SampleView$1.handleEvent(Event) line: 21
Created attachment 207335 [details] Patch v06 Issue described in comment 22 is fixed. Still need to look at restart scenarios. Created attachment 207377 [details]
Patch v07
This should be the last version...I hope.
Ben, would you mind applying this patch on top of current HEAD on master (a09ed9cdb1aed5598b77b064bfac8f855c5bc2bf) and help test it out? Thanks.
Sure Remy, I will give it a try. I probably won't have the opportunity to do it until after the Thanksgiving weekend though. Thanks for working on this issue! It looks like the filenames of the 2 files changed in org.eclipse.ui.workbench are incomplete. I will probably be able to figure out what those files are and amend the patch file such that I can apply it, but it might be easier if you could just upload a patch with the filenames included? It seems it's the space in "Eclipse UI" folder within the bundle that causes this trouble with patches. (In reply to comment #26) > It looks like the filenames of the 2 files changed in org.eclipse.ui.workbench > are incomplete. I will probably be able to figure out what those files are and > amend the patch file such that I can apply it, but it might be easier if you > could just upload a patch with the filenames included? Ben, are you applying the patch from within Eclipse? When prompted, ask Eclipse to apply it from the workspace root. On the next page in the wizard, you will probably see some errors. Tell Eclipse to ignore 2 leading path name segments in the combo box in the top left hand corner of the wizard page. That should get it to apply cleanly. I should have been clearer -- I didn't actually even try to apply the patch. I just looked over the patch (both formatted and raw) in bugzilla and saw this: --- a/bundles/org.eclipse.ui.workbench/Eclipse +++ a/bundles/org.eclipse.ui.workbench/Eclipse Instead of this: --- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/ActionBars.java +++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/ActionBars.java The problem I guess is that I was looking at the diff against the previous patch instead of the v07 patch file in its entirety. I should be able to apply the patch -- sorry for the confusion. If I have any more trouble I will let you know. Comment on attachment 207377 [details]
Patch v07
There's some bad interaction with the main menu bar when multiple workbench windows are up.
Created attachment 207618 [details]
Patch v08
This new patch resolves the flickering problem by not asking menu managers to update unless one of their items have actually changed their visibility states.
(In reply to comment #30) > Created attachment 207618 [details] > Patch v08 Removed some unnecessary code and pushed it to master. http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=7a2e6f2c24786040969b51f515f5d5a1ac0e3d7e Verified with I20111205-2330 on Windows 7 that the 'Problems' view has the view menu but the 'Javadoc' and 'Declaration' views do not. Reverted. See bug 365797 (confirmed to be due to 7a2e6f2c24786040969b51f515f5d5a1ac0e3d7e) and bug 365821 (cannot reproduce at the moment). http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=524160165e659ac8852a52ae127bee047e628bdc http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=8206549da1a67f92770a000fd780b86f4465a1f9 http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=07badc6f17d030c60385e11c0015b4465fcf1c8d http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=42cf59aee19579f1a0377566952393eb3fdcd2f4 Created attachment 209131 [details]
Patch v09
*** Bug 365821 has been marked as a duplicate of this bug. *** Unmarking the bug for now. The patch causes BIRT to behave in a bad way. In all the technical discussion I'm not sure what exactly is going to be fixed here, but I see kind-of the inverse of bug 365821 in 4.2 M5: views have the triangle for a view menu although there's no menu behind. Examples: Console, Display For the console case this is actually quite confusing when searching for a way to change the Console preferences: seeing the menu triangle I was so sure it must be in that (inaccessible) menu, that it took a while until I found "Preferences..." in the context menu. (In reply to comment #37) > views have the > triangle for a view menu although there's no menu behind. Right, this bug is about the problem where all views have a view menu drawn (the inverted triangle). Unfortunately, the latest attempt at making this work breaks BIRT. We'd either need a different approach or they'd need to fix bug 365797. (In reply to comment #38) > We'd either need a different approach or they'd need to fix bug 365797. BIRT has corrected the code and I have released the latest fix to master. http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=5185d9e93948557023700a3cfd8d3ff2480223ee Verified in I20120214-2200. |