Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 334163

Summary: PluginAction allows disabled actions to run
Product: [Eclipse Project] Platform Reporter: Brian Bauman <baumanbr>
Component: UIAssignee: Platform-UI-Inbox <Platform-UI-Inbox>
Status: RESOLVED INVALID QA Contact:
Severity: normal    
Priority: P3 CC: kleind, mukund, pwebster, remy.suen
Version: 3.7   
Target Milestone: ---   
Hardware: PC   
OS: All   
Whiteboard:
Attachments:
Description Flags
Proposed patch based on Eclipse 3.4.2 none

Description Brian Bauman CLA 2011-01-12 15:36:25 EST 
Build Identifier: 3.4.2

The logic in PluginAction.runWithEvent only checks for enablement the first time an action is run.  The first time through the delegate is created which is followed by checking to see if the action is enabled.  If the action is disabled then the execution returns.  If the user tries to run the action again (allowed in a code path in our product), then the enablement is never checked and the action is executed.

The solution is very simple.  Move the isEnabled check outside the "if (delegate == null)" check.  This would provide the same behavior on the first pass, but correctly handle the case if an action is trying to be rerun that is disabled.

I am attaching a patch with the proposed fix

Reproducible: Always

Steps to Reproduce:
Reproducible in IBM/Lotus product.  I have not been able to reproduce in vanilla Eclipse.
Comment 1 Brian Bauman CLA 2011-01-12 15:55:08 EST 
Created attachment 186677 [details]
Proposed patch based on Eclipse 3.4.2
Comment 2 Brian Bauman CLA 2011-01-12 15:58:59 EST 
If possible, I would really appreciate if this could be considered for Eclipse
3.7.
Comment 3 Paul Webster CLA 2011-01-13 07:43:26 EST 
It's the responsibility of the caller to check enabled state before calling.

ActionContributionItems disable the menus and tool items based on that state, so that it cannot be called.  The key binding system checks isEnabled before calling (which runs through the command system), and programmatic executing through the IHandlerService (which goes through Command) also calls isEnabled() before executing (both delegate to the action).

PW
Comment 4 Paul Webster CLA 2011-01-13 07:52:35 EST 
I'll also say I do like that your request came with a patch.

Still, I don't want to change an execution path in the heart of the legacy action delegate support.

PW