This Bugzilla instance is deprecated, and most Eclipse projects now use GitHub or Eclipse GitLab. Please see the deprecation plan for details.
Bug 412927 - Context menu can be corrupted if there is an invalid property tester
Summary: Context menu can be corrupted if there is an invalid property tester
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.4   Edit
Hardware: PC All
: P3 normal (vote)
Target Milestone: 4.4 M1   Edit
Assignee: Paul Webster CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 415692
  Show dependency tree
 
Reported: 2013-07-14 17:06 EDT by Snjezana Peco CLA
Modified: 2014-05-30 09:36 EDT (History)
4 users (show)

See Also:

Attachments
A test plugin (5.66 KB, application/zip)
2013-07-14 17:06 EDT, Snjezana Peco CLA 		no flags Details
A patch (1.83 KB, patch)
2013-07-14 17:12 EDT, Snjezana Peco CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Snjezana Peco CLA 2013-07-14 17:06:21 EDT 
Created attachment 233463 [details]
A test plugin

The attached plugin contributes a popup menu action which visibility depends on a property tester class that throws a NPE.
Eclipse E4 (ContributionsAnalyzer.isVisible(MCoreExpression, ExpressionContext)) catches only CoreException which causes such a contribution to corrupt a context menu on Windows (on Linux, this menu  isn't displayed at all).
Since the isVisible method is often called, it is possible that there are some other issues that are difficult to detect/debug.

Test case:
- install the attached plugin 
- right-click some resource 

Attached is a patch.
Comment 1 Snjezana Peco CLA 2013-07-14 17:12:09 EDT 
Created attachment 233464 [details]
A patch
Comment 2 Paul Webster CLA 2013-07-22 13:18:11 EDT 
Could you please use org.eclipse.core.runtime.SafeRunner to call out to the client code?  In this case, the ref.evaluate(*).  It would be helpful if you pushed your patch to Gerrit, with the new CLA rules.

Also, could you please sign your CLA?

PW
Comment 3 Snjezana Peco CLA 2013-07-22 17:02:14 EDT 
I have signed my CLA and will push the patch to Gerrit.
Comment 4 Snjezana Peco CLA 2013-07-23 09:15:21 EDT 
I have updated the patch and added it to Gerrit - https://git.eclipse.org/r/14762
Comment 5 Paul Webster CLA 2013-07-24 14:34:17 EDT 
Hi Snjezana, thanks for the plugin and Gerrit patch.  I've added comments to the review.

PW
Comment 6 Snjezana Peco CLA 2013-07-24 17:46:43 EDT 
I have updated the patch
Comment 7 Marc Khouzam CLA 2013-08-14 10:26:30 EDT 
(In reply to comment #2)
> Could you please use org.eclipse.core.runtime.SafeRunner to call out to the
> client code?  In this case, the ref.evaluate(*).

Hi Paul,

using SafeRunner in this case is causing a *bunch* of CoreExceptions to be logged when using CDT.  It is not clear to me if those exceptions should not be happening, or if silencing them was correct.

Bug 414912 was written about this.
Comment 8 Paul Webster CLA 2013-08-14 11:34:17 EDT 
This was fixed in M1, http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=1d2a4872f5a6a01c41a655c5e041eb180ea22827

Looking at a new problem in Bug 414912

PW
Comment 9 Wojciech Sudol CLA 2014-05-30 09:36:05 EDT 
Verified in I20140528-2000.