Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 371130 - 'Delete' key does not work in 'Breakpoints' view in I20120208-2200
Summary: 'Delete' key does not work in 'Breakpoints' view in I20120208-2200
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 4.2   Edit
Hardware: PC Windows 7
: P3 major (vote)
Target Milestone: 3.8 M6   Edit
Assignee: Pawel Piech CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 365453 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-02-09 15:07 EST by Remy Suen CLA
Modified: 2012-03-13 23:08 EDT (History)
5 users (show)

See Also:


Attachments
Patch (1.89 KB, patch)
2012-02-28 10:29 EST, Remy Suen CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Remy Suen CLA 2012-02-09 15:07:46 EST
I can't use the keyboard to delete breakpoints. The 'Delete' entry in the 'Edit' menu is also disabled.
Comment 1 Remy Suen CLA 2012-02-10 08:14:40 EST
We get to here and then nothing happens since the returned application context is null. I need to compare the behaviour with older builds to see if that's actually the problem. The KeyBindingDispatcher doesn't do anything because we can't find a handler.

MakeHandlersGo.execute(ExecutionEvent) line: 57	
Command.execute(ExecutionEvent) line: 412	
CommandLegacyActionWrapper.runWithEvent(Event) line: 322	
CommandLegacyActionWrapper.run() line: 314	
BreakpointsView(AbstractDebugView).handleKeyPressed(KeyEvent) line: 719	
AbstractDebugView$1.keyPressed(KeyEvent) line: 334	
TypedListener.handleEvent(Event) line: 167	
EventTable.sendEvent(Event) line: 84	
Tree(Widget).sendEvent(Event) line: 1053	
Tree(Widget).sendEvent(int, Event, boolean) line: 1077	
Tree(Widget).sendEvent(int, Event) line: 1062	
Tree(Widget).sendKeyEvent(int, int, long, long, Event) line: 1104	
Tree(Widget).sendKeyEvent(int, int, long, long) line: 1100	
Tree(Widget).wmKeyDown(long, long, long) line: 1809	
Tree(Control).WM_KEYDOWN(long, long) line: 4892	
Tree.WM_KEYDOWN(long, long) line: 6071	
Tree(Control).windowProc(long, int, long, long) line: 4560	
Tree.windowProc(long, int, long, long) line: 5939	
Display.windowProc(long, long, long, long) line: 4985	
OS.DispatchMessageW(MSG) line: not available [native method]	
OS.DispatchMessage(MSG) line: 2545	
Display.readAndDispatch() line: 3752	
PartRenderingEngine$9.run() line: 999	
Realm.runWithDefault(Realm, Runnable) line: 332	
PartRenderingEngine.run(MApplicationElement, IEclipseContext) line: 893	
E4Workbench.createAndRunUI(MApplicationElement) line: 85	
Workbench$4.run() line: 579	
Realm.runWithDefault(Realm, Runnable) line: 332	
Workbench.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 534	
PlatformUI.createAndRunWorkbench(Display, WorkbenchAdvisor) line: 149	
IDEApplication.start(IApplicationContext) line: 124	
EclipseAppHandle.run(Object) line: 196	
EclipseAppLauncher.runApplication(Object) line: 110	
EclipseAppLauncher.start(Object) line: 79	
EclipseStarter.run(Object) line: 352	
EclipseStarter.run(String[], Runnable) line: 179	
NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method]	
NativeMethodAccessorImpl.invoke(Object, Object[]) line: 88	
DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 55	
Method.invoke(Object, Object...) line: 613	
Main.invokeFramework(String[], URL[]) line: 624	
Main.basicRun(String[]) line: 579	
Main.run(String[]) line: 1433	
Main.main(String[]) line: 1409
Comment 2 Remy Suen CLA 2012-02-10 08:46:21 EST
This is the trace from I20120126-1300. So it seems we have some code that's setting replacing the RemoveBreakpointAction with a CommandLegacyActionWrapper.

RemoveBreakpointAction.run(IAction) line: 153	
ViewPluginAction(PluginAction).runWithEvent(Event) line: 251	
ViewPluginAction(PluginAction).run() line: 214	
BreakpointsView(AbstractDebugView).handleKeyPressed(KeyEvent) line: 719	
AbstractDebugView$1.keyPressed(KeyEvent) line: 334	
TypedListener.handleEvent(Event) line: 167	
EventTable.sendEvent(Event) line: 84	
Tree(Widget).sendEvent(Event) line: 1053	
Tree(Widget).sendEvent(int, Event, boolean) line: 1077	
Tree(Widget).sendEvent(int, Event) line: 1062	
Tree(Widget).sendKeyEvent(int, int, long, long, Event) line: 1104	
Tree(Widget).sendKeyEvent(int, int, long, long) line: 1100	
Tree(Widget).wmKeyDown(long, long, long) line: 1809	
Tree(Control).WM_KEYDOWN(long, long) line: 4892	
Tree.WM_KEYDOWN(long, long) line: 6071	
Tree(Control).windowProc(long, int, long, long) line: 4560	
Tree.windowProc(long, int, long, long) line: 5939	
Display.windowProc(long, long, long, long) line: 4972	
OS.DispatchMessageW(MSG) line: not available [native method]	
OS.DispatchMessage(MSG) line: 2545	
Display.readAndDispatch() line: 3752
Comment 3 Remy Suen CLA 2012-02-10 09:03:13 EST
Caused by the fix for bug 361562.

First it is okay, but then it gets overridden by the ADHP.

---------------

BreakpointsView(AbstractDebugView).setAction(String, IAction) line: 676	
RemoveBreakpointAction(AbstractRemoveActionDelegate).init(IViewPart) line: 35	
ViewPluginAction.initDelegate() line: 59	
ViewPluginAction(PluginAction).createDelegate() line: 125	
ViewPluginAction(PluginAction).selectionChanged(ISelection) line: 275	
ViewPluginAction(PartPluginAction).registerSelectionListener(IWorkbenchPart) line: 40	
ViewPluginAction.<init>(IConfigurationElement, IViewPart, String, int) line: 38	
ActionDescriptor.createAction(int, IConfigurationElement, Object, String) line: 259	
ActionDescriptor.<init>(IConfigurationElement, int, Object) line: 176	
ViewerActionBuilder.createActionDescriptor(IConfigurationElement) line: 51	
ViewerActionBuilder(PluginActionBuilder).readElement(IConfigurationElement) line: 161	
ViewerActionBuilder.readElement(IConfigurationElement) line: 88	
ViewerActionBuilder(RegistryReader).readElements(IConfigurationElement[]) line: 144	
ViewerActionBuilder(RegistryReader).readElementChildren(IConfigurationElement) line: 133	
ViewerActionBuilder(PluginActionBuilder).readElement(IConfigurationElement) line: 144	
ViewerActionBuilder.readElement(IConfigurationElement) line: 88	
ViewerActionBuilder(RegistryReader).readElements(IConfigurationElement[]) line: 144	
ViewerActionBuilder(RegistryReader).readExtension(IExtension) line: 155	
ViewerActionBuilder(RegistryReader).readRegistry(IExtensionRegistry, String, String) line: 176	
ViewerActionBuilder(PluginActionBuilder).readContributions(String, String, String) line: 115	
ViewerActionBuilder.readViewerContributions(String, ISelectionProvider, IWorkbenchPart) line: 106	
PopupMenuExtender.readStaticActionsFor(String) line: 524	
PopupMenuExtender.addMenuId(String) line: 228	
PopupMenuExtender.<init>(String, MenuManager, ISelectionProvider, IWorkbenchPart, boolean) line: 148	
PartSite.registerContextMenu(String, MenuManager, ISelectionProvider, boolean, IWorkbenchPart, Collection) line: 130	
ViewSite(PartSite).registerContextMenu(String, MenuManager, ISelectionProvider) line: 495	
ViewSite(PartSite).registerContextMenu(MenuManager, ISelectionProvider) line: 504	
BreakpointsView(AbstractDebugView).createContextMenu(Control) line: 544	
BreakpointsView(VariablesView).createContextMenu(Control) line: 942	
BreakpointsView(AbstractDebugView).createPartControl(Composite) line: 325	
CompatibilityView(CompatibilityPart).createPartControl(IWorkbenchPart, Composite) line: 122	
CompatibilityView.createPartControl(IWorkbenchPart, Composite) line: 104	
CompatibilityView(CompatibilityPart).create() line: 265

---------------

BreakpointsView(AbstractDebugView).setAction(String, IAction) line: 676	
RemoveBreakpointAction(AbstractRemoveActionDelegate).init(IViewPart) line: 35	
ActionDelegateHandlerProxy$2.run() line: 479	
SafeRunner.run(ISafeRunnable) line: 42	
ActionDelegateHandlerProxy.initDelegate() line: 485	
ActionDelegateHandlerProxy.loadDelegate() line: 603	
ActionDelegateHandlerProxy.updateDelegate(IAction, IEvaluationContext) line: 312	
ActionDelegateHandlerProxy.setEnabled(Object) line: 522	
E4HandlerProxy.canExecute(IEclipseContext, IEvaluationContext) line: 50	
GeneratedMethodAccessor8.invoke(Object, Object[]) line: not available	
DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 55	
Method.invoke(Object, Object...) line: 613	
MethodRequestor.execute() line: 56	
InjectorImpl.invokeUsingClass(Object, Class<?>, Class<Annotation>, Object, PrimaryObjectSupplier, PrimaryObjectSupplier, boolean) line: 228	
InjectorImpl.invoke(Object, Class<Annotation>, Object, PrimaryObjectSupplier, PrimaryObjectSupplier) line: 209	
ContextInjectionFactory.invoke(Object, Class<Annotation>, IEclipseContext, IEclipseContext, Object) line: 123	
HandlerServiceImpl.canExecute(ParameterizedCommand, IEclipseContext) line: 108	
HandledContributionItem.canExecuteItem(Event) line: 777	
HandledContributionItem.access$1(HandledContributionItem, Event) line: 766	
HandledContributionItem$3.run() line: 188	
SafeRunner.run(ISafeRunnable) line: 42	
HandledContributionItem.updateItemEnablement() line: 215	
HandledContributionItem$ToolItemUpdateTimer.run() line: 127	
Display.runTimer(long) line: 4266	
Display.messageProc(long, long, long, long) line: 3353	
OS.DispatchMessageW(MSG) line: not available [native method]	
OS.DispatchMessage(MSG) line: 2545	
Display.readAndDispatch() line: 3752
Comment 4 Remy Suen CLA 2012-02-10 09:27:31 EST

*** This bug has been marked as a duplicate of bug 361562 ***
Comment 5 Remy Suen CLA 2012-02-10 09:49:52 EST
I take that back. This can be broken even with the ADHP change reverted.

1. Select a breakpoint in the 'Breakpoints' view.
2. Hit 'Delete'. It gets removed.
3. Select another breakpoint in the 'Breakpoints' view.
4. Click on the 'Remove Breakpoint' X tool item in the view's tool bar. It gets removed.
5. Select another breakpoint in the 'Breakpoints' view.
6. Hit 'Delete'. It will _not_ get removed.

!ENTRY org.eclipse.ui 4 4 2012-02-10 09:46:06.604
!MESSAGE Calling "Command(AUTOGEN:::org.eclipse.debug.ui.breakpointsview.toolbar/org.eclipse.debug.ui.breakpointsView.toolbar.remove,Remove,
		Remove Selected Breakpoints,
		Category(org.eclipse.core.commands.categories.autogenerated,Uncategorized,Commands that were either auto-generated or have no category,true),
		org.eclipse.ui.internal.MakeHandlersGo@fd1e6f07,
		,,true)" command directly
Comment 6 Remy Suen CLA 2012-02-16 09:25:29 EST
Wonder if this could be caused by the same problem as bug 371131?
Comment 7 Michael Rennie CLA 2012-02-23 17:05:47 EST
(In reply to comment #6)
> Wonder if this could be caused by the same problem as bug 371131?

The same problem happens in the Expressions view as well using the same steps from  comment #5
Comment 8 Remy Suen CLA 2012-02-27 09:33:15 EST
If we define a definitionId for the breakpoint removal action and also get rid of the action setting code in AbstractRemoveActionDelegate then the problem seems to go away.
Comment 9 Paul Webster CLA 2012-02-28 08:44:41 EST
(In reply to comment #5)
> !ENTRY org.eclipse.ui 4 4 2012-02-10 09:46:06.604
> !MESSAGE Calling
> "Command(AUTOGEN:::org.eclipse.debug.ui.breakpointsview.toolbar/org.eclipse.debug.ui.breakpointsView.toolbar.remove,Remove,
>         Remove Selected Breakpoints,
>        
> Category(org.eclipse.core.commands.categories.autogenerated,Uncategorized,Commands
> that were either auto-generated or have no category,true),
>         org.eclipse.ui.internal.MakeHandlersGo@fd1e6f07,
>         ,,true)" command directly

While not great, I believe we changed this to print out the message *and* still try and execute a handler.  Maybe it couldn't find one?

PW
Comment 10 Remy Suen CLA 2012-02-28 10:03:27 EST
(In reply to comment #9)
> While not great, I believe we changed this to print out the message *and* still
> try and execute a handler.  Maybe it couldn't find one?

When the ExecutionEvent is spawned by CommandLegacyActionWrapper in runWithEvent(*), it doesn't have a context so nothing happens.

MakeHandlersGo.execute(ExecutionEvent) line: 81	
Command.execute(ExecutionEvent) line: 412	
CommandLegacyActionWrapper.runWithEvent(Event) line: 322	
CommandLegacyActionWrapper.run() line: 314	
BreakpointsView(AbstractDebugView).handleKeyPressed(KeyEvent) line: 719	
AbstractDebugView$1.keyPressed(KeyEvent) line: 334	
TypedListener.handleEvent(Event) line: 167	
EventTable.sendEvent(Event) line: 84	
Tree(Widget).sendEvent(Event) line: 1053	
Tree(Widget).sendEvent(int, Event, boolean) line: 1077	
Tree(Widget).sendEvent(int, Event) line: 1062	
Tree(Widget).sendKeyEvent(int, int, long, long, Event) line: 1104	
Tree(Widget).sendKeyEvent(int, int, long, long) line: 1100	
Tree(Widget).wmKeyDown(long, long, long) line: 1809	
Tree(Control).WM_KEYDOWN(long, long) line: 4892	
Tree.WM_KEYDOWN(long, long) line: 6071	
Tree(Control).windowProc(long, int, long, long) line: 4560	
Tree.windowProc(long, int, long, long) line: 5939	
Display.windowProc(long, long, long, long) line: 4985	
OS.DispatchMessageW(MSG) line: not available [native method]	
OS.DispatchMessage(MSG) line: 2545	
Display.readAndDispatch() line: 3752
Comment 11 Remy Suen CLA 2012-02-28 10:29:22 EST
Created attachment 211735 [details]
Patch

This fix applies the same logic that was used for bug 371131.

Not sure if you want that (now empty) init(IViewPart) method anymore but anyway...
Comment 12 Pawel Piech CLA 2012-02-28 14:17:32 EST
Thank you for the fix.  I committed: http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=bd4d43e331051fa8d92d990ec2c8d76445225081

I removed the obsolete abstract remove action and added the definition ID for the remove exression action as well.
Comment 13 Remy Suen CLA 2012-02-28 14:19:44 EST
(In reply to comment #12)
> I removed the obsolete abstract remove action and added the definition ID for
> the remove exression action as well.

Thanks, Pawel, didn't think about the expression one at all.
Comment 14 Michael Rennie CLA 2012-02-28 14:25:36 EST
(In reply to comment #12)
> Thank you for the fix.  I committed:
> http://git.eclipse.org/c/platform/eclipse.platform.debug.git/commit/?id=bd4d43e331051fa8d92d990ec2c8d76445225081
> 
> I removed the obsolete abstract remove action and added the definition ID for
> the remove exression action as well.

Thanks Pawel, I was planning to get this this afternoon
Comment 15 Remy Suen CLA 2012-03-13 15:21:21 EDT
*** Bug 365453 has been marked as a duplicate of this bug. ***
Comment 16 Pawel Piech CLA 2012-03-13 23:08:55 EDT
Verified with  I20120313-0610