Bug 111778 - [Contributions] enablement expression in object contributions is parsed by legacy mechanism and new mechanism
Summary: [Contributions] enablement expression in object contributions is parsed by le...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1   Edit
Hardware: All All
: P5 major with 4 votes (vote)
Target Milestone: 3.5 M5   Edit
Assignee: Paul Webster CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted
: 106770 136686 143594 153011 263205 289151 (view as bug list)
Depends on: 77043
Blocks: 106770 136686
  Show dependency tree
 
Reported: 2005-10-06 10:25 EDT by Douglas Pollock CLA
Modified: 2009-09-11 09:24 EDT (History)
10 users (show)

See Also:


Attachments
Add a pointer to useful core expression doc v01 (13.08 KB, patch)
2009-01-19 10:06 EST, Paul Webster CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Douglas Pollock CLA 2005-10-06 10:25:54 EDT
There are two bugs in the patch submitted for Bug 59963.  This bug will cover 
both of these bugs, though Bug 77043 could be considered to cover one of them. 
 
1.) There are two enablement elements in popupMenus.exsd.  One is imported 
from commonExpression.exsd, and the other is imported from 
expressionLanguage.exsd.  As it stands in PDE, there is no way to distinguish 
which enablement expression is being used (Bug 77043).  However, it is 
possible to simply create a second element with a different name (e.g., 
enablementExpression).  If clients use this element, then the enablement will 
be parsed using the new syntax; otherwise, it will be using the old syntax 
(i.e., partly, see (2) for more info). 
 
2.) The patch for Bug 59963 changed one place where the enablement expression 
was parsed (ObjectActionContributor$ObjectContribution.setEnablementTest), but 
it doesn't change another (SelectionEnabler.parseClasses).  As a result, it is 
only possible to use expression elements that are understood by both 
ExpressionConverter and ActionExpression.
Comment 1 Gunnar Wagenknecht CLA 2006-02-20 07:12:17 EST
(In reply to comment #0)
> 1.) There are two enablement elements in popupMenus.exsd.  One is imported 
> from commonExpression.exsd, and the other is imported from 
> expressionLanguage.exsd.  As it stands in PDE, there is no way to distinguish 
> which enablement expression is being used (Bug 77043). 

3.2 M5: The side effect of this is as really bad usability issue. Useres can't use the Extensions tab in the PDE editor to edit the org.eclipse.ui.popupMenus extension point. They are forced to edit the XML directly. For example, if you right click on any "enablement" element and select "New" only "and,or,not,objectClass,objectState,pluginState,systemProperty" are offered. There is no way to use the new syntax. 

The extension point documentation is also not very helpful. If you walk jump down to the enablement description only the "old" one appears (#e.enablement). You have to scroll down to see the second one. This doesn't work because it assums that a users knows that there is a second enablement syntax he can use. However, nothing is mentioned in any preceding paragraphs.
Comment 2 Michael Van Meekeren CLA 2006-04-21 13:56:21 EDT
Moving Dougs bugs
Comment 3 Paul Webster CLA 2007-04-05 19:04:41 EDT
Assigning to component owner
PW
Comment 4 Paul Webster CLA 2007-06-21 13:13:43 EDT
This could be solved by making the new core-expression using element enabledWhen (the code would of course have to read both).

PW
Comment 5 Paul Webster CLA 2007-06-21 15:00:53 EDT
*** Bug 136686 has been marked as a duplicate of this bug. ***
Comment 6 Paul Webster CLA 2007-06-22 06:55:53 EDT
*** Bug 106770 has been marked as a duplicate of this bug. ***
Comment 7 Paul Webster CLA 2007-06-22 06:56:41 EDT
*** Bug 143594 has been marked as a duplicate of this bug. ***
Comment 8 Paul Webster CLA 2007-06-22 15:05:40 EDT
*** Bug 153011 has been marked as a duplicate of this bug. ***
Comment 9 Markus Keller CLA 2008-10-07 06:42:47 EDT
(In reply to comment #1)
+1. This is really confusing. Please at least fix the documentation, so that users have a chance to understand what's going on.
Comment 10 Paul Webster CLA 2008-10-07 07:43:32 EDT
OK, how about I add the enabledWhen element so the PDE tools can pick it up.  I'll then make a note that this should be used, and deprecate enablement if I can.

PW
Comment 11 Markus Keller CLA 2008-10-07 08:56:26 EDT
I don't think I understand where you want to add enabledWhen. Do you want to put this into expressionLanguage.exsd and deprecate the <element name="enablement"> there? I don't think that would help too much, since it just shifts the conflicts one level deeper (e.g. duplicate <and> definitions with different set of children).

And if I understood that correctly, org.eclipse.ui.popupMenus is going to be deprecated at some time in the future and clients should move to the org.eclipse.ui.menus extension point.

So I think you should not change the popupMenus ext. pt. any more now. Just add a sentence to the schema description at those places where the "enablement" children are actually parsed using core expressions, and add a link to the core expressions schema description (like the link to org.eclipse.ui.menus in the beginning of the popupMenus description).
Comment 12 Paul Webster CLA 2008-10-07 09:20:27 EDT
(In reply to comment #11)
> I don't think I understand where you want to add enabledWhen. Do you want to
> put this into expressionLanguage.exsd and deprecate the <element
> name="enablement"> there? I don't think that would help too much, since it just
> shifts the conflicts one level deeper (e.g. duplicate <and> definitions with
> different set of children).

No, I can't do it there, it's used by many other extension points.  I simply meant adding enabledWhen to popupMenus.exsd and marking the reference in objectContribution/enablement deprecated (although that may not be possible, so it would simply get mentioned in the description).

> And if I understood that correctly, org.eclipse.ui.popupMenus is going to be
> deprecated at some time in the future and clients should move to the
> org.eclipse.ui.menus extension point.
> 
> So I think you should not change the popupMenus ext. pt. any more now. Just add
> a sentence to the schema description at those places where the "enablement"
> children are actually parsed using core expressions, and add a link to the core
> expressions schema description (like the link to org.eclipse.ui.menus in the
> beginning of the popupMenus description).

This is also do-able, although it won't help anybody actually use the objectContribution/enablement element.

PW
Comment 13 Markus Keller CLA 2008-10-07 10:11:52 EDT
> This is also do-able, although it won't help anybody actually use the
> objectContribution/enablement element.

Yes, but it sounded like it would be too hard to fix the plug-in manifest editor to deal with conflicts, so even adding enabledWhen would not really resolve the problem but just shift it to the children of that element (duplicate and, or, ...). If the schema describes the problem and contains a link to the correct documentation, users at least have some guidance on how to write/parse it on the source page.
Comment 14 Paul Webster CLA 2009-01-19 10:06:49 EST
Created attachment 122941 [details]
Add a pointer to useful core expression doc v01
Comment 15 Paul Webster CLA 2009-01-19 10:07:42 EST
Released to HEAD >20090119
PW
Comment 16 Paul Webster CLA 2009-01-27 10:40:57 EST
In I20090127-0100
Comment 17 Paul Webster CLA 2009-02-03 10:19:07 EST
*** Bug 263205 has been marked as a duplicate of this bug. ***
Comment 18 Paul Webster CLA 2009-09-11 09:24:03 EDT
*** Bug 289151 has been marked as a duplicate of this bug. ***