Bug 512790 - checkEnabled=true in visibleWhen doesn't work for menuContribution in Eclipse 4.6 RCP
Summary: checkEnabled=true in visibleWhen doesn't work for menuContribution in Eclipse...
Status: CLOSED DUPLICATE of bug 201589
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.6   Edit
Hardware: All All
: P3 normal with 8 votes (vote)
Target Milestone: ---   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-02-27 21:10 EST by Endi Wu CLA
Modified: 2020-02-25 04:01 EST (History)
2 users (show)

See Also:


Attachments
RCP application to duplicate the issue. (134.93 KB, application/x-zip-compressed)
2017-02-27 21:10 EST, Endi Wu CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Endi Wu CLA 2017-02-27 21:10:52 EST
Created attachment 267019 [details]
RCP application to duplicate the issue.

I create E3 RCP application on Eclipse 4.6, contributing Test Command to popup menu using menu contribution, I defined checkEnabled=true in visibleWhen clause as below:
<extension point="org.eclipse.ui.menus">   
      <menuContribution
              allPopups="false"
              locationURI="popup:org.eclipse.ui.popup.any?after=additions">
            <command
                  commandId="com.mycom.customization.examples.testCmd" >
              <visibleWhen
                      checkEnabled="true">
                 <with variable="activeWorkbenchWindow.activePerspective">
                   
                   <equals
                        value="com.example.e3.rcp.wizard.perspective">
                    </equals>
              </with>
             </visibleWhen>                
            </command>
        </menuContribution>
 </extension>        

so the visibility of testCmd is based upon enableWhen result in handler. When the result of enableWhen in handler is false, the testCmd is disable and should NOT be shown in context menu, but it is shown. I attached rcp application zip for you to duplicate on Eclipse 4.6. Same contribution on Eclipse 3.8, the issue can not be duplicated, it works good on Eclipse 3.8.
Comment 1 Pierre-Yves Bigourdan CLA 2019-11-23 16:52:37 EST
I believe things work as expected if you pull the "visibleWhen" element outside the "command" one. In org.eclipse.e4.ui.internal.workbench.ContributionsAnalyzer.isVisible(MMenuContribution menuContribution, ExpressionContext eContext), the visibility checks are performed at the menu contribution level. None of the children elements are considered to inform the decision.

However, doing so causes a schema error/warning, as menus.exsd does not allow "menuContribution" elements to have a "visibleWhen" child.

I can see two solutions to this bug:
* either perform a small tweak to the menus.exsd schema. This will however not fix existing cases like the one showcased by Endi: users will have to modify their plugin.xml as well.
* change the code to look at nested "visibleWhen" elements within a "menuContribution"'s children. I'm unclear what the behaviour should be if a menu contribution has several children though, each one of them with their own visibility conditions. This will likely lead to a more complex implementation, but will fix examples as above without any changes from users. However, as this has been broken for so many years, it may actually be a bad idea, users who have left non functional visibleWhens behind may be caught off-guard by a sudden change in behaviour.
Comment 2 Lars Vogel CLA 2020-02-18 03:09:28 EST
(In reply to Pierre-Yves B. from comment #1)
> I believe things work as expected if you pull the "visibleWhen" element
> outside the "command" one. In
> org.eclipse.e4.ui.internal.workbench.ContributionsAnalyzer.
> isVisible(MMenuContribution menuContribution, ExpressionContext eContext),
> the visibility checks are performed at the menu contribution level. None of
> the children elements are considered to inform the decision.
> 
> However, doing so causes a schema error/warning, as menus.exsd does not
> allow "menuContribution" elements to have a "visibleWhen" child.
> 
> I can see two solutions to this bug:
> * either perform a small tweak to the menus.exsd schema. This will however
> not fix existing cases like the one showcased by Endi: users will have to
> modify their plugin.xml as well.
> * change the code to look at nested "visibleWhen" elements within a
> "menuContribution"'s children. I'm unclear what the behaviour should be if a
> menu contribution has several children though, each one of them with their
> own visibility conditions. This will likely lead to a more complex
> implementation, but will fix examples as above without any changes from
> users. However, as this has been broken for so many years, it may actually
> be a bad idea, users who have left non functional visibleWhens behind may be
> caught off-guard by a sudden change in behaviour.

Pierre-Yves, please suggest a Gerrit. IMHO it is fine to require people to adjust their plugin.xml but I'm also fine if you prefer to change the code.
Comment 3 Pierre-Yves Bigourdan CLA 2020-02-18 17:02:13 EST
Bug 201589 seems related, in particular Rolf's comment (https://bugs.eclipse.org/bugs/show_bug.cgi?id=201589#c49). Given that there's already work in progress there, I've hold off from submitting a patch just yet as this may get fixed as well in one go.
Comment 4 Lars Vogel CLA 2020-02-25 04:01:36 EST
(In reply to Pierre-Yves B. from comment #3)
> Bug 201589 seems related, in particular Rolf's comment
> (https://bugs.eclipse.org/bugs/show_bug.cgi?id=201589#c49). Given that
> there's already work in progress there, I've hold off from submitting a
> patch just yet as this may get fixed as well in one go.

Maybe you can work on this issue via Bug 201589? Sounds like Rolf can help some help.

I mark this bug as dup for Bug 201589, if that is wrong, lets reopen this bug.

*** This bug has been marked as a duplicate of bug 201589 ***