Details

    • Type: Sub-task Sub-task
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.0
    • Fix Version/s: None
    • Component/s: Facelets/VDL
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      755
    • Status Whiteboard:
      Hide

      cat2 frame size_medium importance_large

      Show
      cat2 frame size_medium importance_large

      Description

      I already talked to Ed Burns about this at the JSFDays, but I thought opening a
      spec issue won't be bad!

      There are 4 special attributes on composite components (action, actionListener,
      validator, valueChangeListener) which are not populated on #

      {cc.attrs}

      as
      MethodExpressions, but added to their related components (defined via the
      targets attribute of cc:attribute) directly via setAction(), addActionListener(),...

      This works fine for all standard components. However if you nest another
      composite component inside the implementation of our composite component, you
      are not able to pass any of those 4 attributes through to the nested composite
      component, because it (normally) is a UINamingContainer which does not implement
      any of the needed interfaces.

      To make this more clear imagine you have a composite component which is a
      special commandButton (AJAX-commandButton or whatever) and you're creating e.g.
      a LoginPanel composite component. This LoginPanel composite component has an
      actionListener attribute to determine the actionListener for the login button
      (which in our implementation is our composite component commandButton). Now we
      are not able to pass the actionListener through to "our" commandButton, because
      it is no ActionSource2 an the MethodExpression is not stored in
      #

      {cc.attrs.actionListener}

      . The same applies to the 3 other attributes.

      <cc:interface name="loginPanel">
      <cc:attribute name="actionListener" targets="???" />
      </cc:interface>
      <cc:implementation>
      <!-- other components -->
      <ez:customCommandButton actionListener="???" />
      </cc:implementation>

      1. 755-cc-attribute-full-solution.patch
        45 kB
        lu4242
      2. 755-cc-attribute-Nested-No-EL.patch
        22 kB
        lu4242
      3. 755-cc-attribute-with-EL.patch
        29 kB
        lu4242
      4. diffs.patch
        9 kB
        Ed Burns
      5. diffs.patch
        9 kB
        Ed Burns

        Issue Links

          Activity

          Hide
          Ed Burns added a comment -

          cat2

          Show
          Ed Burns added a comment - cat2
          Hide
          Ed Burns added a comment -

          frame

          Show
          Ed Burns added a comment - frame
          Hide
          Ed Burns added a comment -

          These are targeted at 2.1.

          Show
          Ed Burns added a comment - These are targeted at 2.1.
          Hide
          Ed Burns added a comment -

          triage

          Show
          Ed Burns added a comment - triage
          Hide
          Ed Burns added a comment -

          GlassFish 3.1 M6 at the latest.

          Show
          Ed Burns added a comment - GlassFish 3.1 M6 at the latest.
          Hide
          Ed Burns added a comment -

          Move these to 2.2

          Show
          Ed Burns added a comment - Move these to 2.2
          Hide
          ganeshpuri added a comment -

          I ran into this with my first large scale JSF 2.0 customer within a few days and
          I think it should be urgently fixed, because usage of action, actionListener,
          validator and valueChangeListener is essential to composite components and
          nesting them will happen as soon a they are used in non trivial projects.

          Show
          ganeshpuri added a comment - I ran into this with my first large scale JSF 2.0 customer within a few days and I think it should be urgently fixed, because usage of action, actionListener, validator and valueChangeListener is essential to composite components and nesting them will happen as soon a they are used in non trivial projects.
          Hide
          Ed Burns added a comment -

          I am delighted to report that now taking the time to look at this more
          closely, I observe that it appears to work.

          In fact, we have an automated test and I'm about to attach the war.

          Deploy the war and visit

          http://localhost:8080/jsf-systest/faces/composite/nesting03.xhtml

          Click the button. It works if you are navigated to a page with
          <title>Navigation Result</title>.

          The key enabler is you must declare the cc-relative client-id in the
          targets.

          SECTION: Using Page nesting03.xhtml

          <html xmlns="http://www.w3.org/1999/xhtml"
          xmlns:h="http://java.sun.com/jsf/html"
          xmlns:f="http://java.sun.com/jsf/core"
          xmlns:ui="http://java.sun.com/jsf/facelets"
          xmlns:ez="http://java.sun.com/jsf/composite/composite">
          <head>
          <title>Nesting 03</title>
          </head>
          <body>
          <h:form>
          <ez:nesting3 action="#

          {compositeBean.doNav}

          " />
          </h:form>
          </body>
          </html>

          SECTION: Defining Page nesting3.xhtml

          <html xmlns="http://www.w3.org/1999/xhtml"
          xmlns:h="http://java.sun.com/jsf/html"
          xmlns:f="http://java.sun.com/jsf/core"
          xmlns:ui="http://java.sun.com/jsf/facelets"
          xmlns:composite="http://java.sun.com/jsf/composite"
          xmlns:ez="http://java.sun.com/jsf/composite/composite">
          <head>
          <title>Should Not Be Displayed</title>
          </head>
          <body>
          <composite:interface>
          <composite:attribute name="action" required="true" method-signature="String method()"
          targets="wrapper:commandButton"/>
          </composite:interface>
          <composite:implementation>
          <ez:wrapper id="wrapper">
          <h:commandButton id="commandButton" value="Click Me" />
          </ez:wrapper>
          </composite:implementation>
          </body>
          </html>

          SECTION: Managed Bean CompositeBean.java

          public String doNav()

          { return "nestingNav"; }
          Show
          Ed Burns added a comment - I am delighted to report that now taking the time to look at this more closely, I observe that it appears to work. In fact, we have an automated test and I'm about to attach the war. Deploy the war and visit http://localhost:8080/jsf-systest/faces/composite/nesting03.xhtml Click the button. It works if you are navigated to a page with <title>Navigation Result</title>. The key enabler is you must declare the cc-relative client-id in the targets. SECTION: Using Page nesting03.xhtml <html xmlns="http://www.w3.org/1999/xhtml" xmlns:h="http://java.sun.com/jsf/html" xmlns:f="http://java.sun.com/jsf/core" xmlns:ui="http://java.sun.com/jsf/facelets" xmlns:ez="http://java.sun.com/jsf/composite/composite"> <head> <title>Nesting 03</title> </head> <body> <h:form> <ez:nesting3 action="# {compositeBean.doNav} " /> </h:form> </body> </html> SECTION: Defining Page nesting3.xhtml <html xmlns="http://www.w3.org/1999/xhtml" xmlns:h="http://java.sun.com/jsf/html" xmlns:f="http://java.sun.com/jsf/core" xmlns:ui="http://java.sun.com/jsf/facelets" xmlns:composite="http://java.sun.com/jsf/composite" xmlns:ez="http://java.sun.com/jsf/composite/composite"> <head> <title>Should Not Be Displayed</title> </head> <body> <composite:interface> <composite:attribute name="action" required="true" method-signature="String method()" targets="wrapper:commandButton"/> </composite:interface> <composite:implementation> <ez:wrapper id="wrapper"> <h:commandButton id="commandButton" value="Click Me" /> </ez:wrapper> </composite:implementation> </body> </html> SECTION: Managed Bean CompositeBean.java public String doNav() { return "nestingNav"; }
          Hide
          Ed Burns added a comment -

          Created an attachment (id=304)
          mojarra systest.war

          Show
          Ed Burns added a comment - Created an attachment (id=304) mojarra systest.war
          Hide
          Jakob Korherr added a comment -

          REOPENING

          Thanks for looking into this, Ed, but unfortunately you got our scenario wrong.
          Your scenario works, but imagine the following implementation section of nesting3:

          <composite:implementation>
          <ez:someOtherNestingComponent action="????" />
          </composite:implementation>

          and the page for someOtherNestingComponent:

          <composite:interface>
          <composite:attribute name="action" required="true"
          method-signature="String method()" targets="commandButton"/>
          </composite:interface>
          <composite:implementation>
          <h:commandButton id="commandButton" value="Click Me" />
          </composite:implementation>

          Thus you have a composite component with an action attribute that itself uses a
          composite component with and action attribute that itself uses a
          h:commandButton. How can you accomplish that?

          Or to be more specific: what do you use for the action attribute of the inner
          composite component since action is not published as #

          {cc.attrs.action}

          and how
          do you prevent the ClassCastException for ActionSource2 for the inner composite
          component (since the first composite component tries to retarget the action
          listener to it)?

          Is this enough documentation or should I provide a sample webapp?

          Show
          Jakob Korherr added a comment - REOPENING Thanks for looking into this, Ed, but unfortunately you got our scenario wrong. Your scenario works, but imagine the following implementation section of nesting3: <composite:implementation> <ez:someOtherNestingComponent action="????" /> </composite:implementation> and the page for someOtherNestingComponent: <composite:interface> <composite:attribute name="action" required="true" method-signature="String method()" targets="commandButton"/> </composite:interface> <composite:implementation> <h:commandButton id="commandButton" value="Click Me" /> </composite:implementation> Thus you have a composite component with an action attribute that itself uses a composite component with and action attribute that itself uses a h:commandButton. How can you accomplish that? Or to be more specific: what do you use for the action attribute of the inner composite component since action is not published as # {cc.attrs.action} and how do you prevent the ClassCastException for ActionSource2 for the inner composite component (since the first composite component tries to retarget the action listener to it)? Is this enough documentation or should I provide a sample webapp?
          Hide
          Ed Burns added a comment -

          Ahh, thanks for reopening this.

          I have modified the systest and will attach a patch and a new version of it.

          Deploy it and you can reproduce the problem by visiting
          http://localhost:8080/jsf-systest/faces/composite/nesting09.xhtml

          Show
          Ed Burns added a comment - Ahh, thanks for reopening this. I have modified the systest and will attach a patch and a new version of it. Deploy it and you can reproduce the problem by visiting http://localhost:8080/jsf-systest/faces/composite/nesting09.xhtml
          Hide
          Ed Burns added a comment -

          Created an attachment (id=305)
          patch to systest that shows off the bug.

          Show
          Ed Burns added a comment - Created an attachment (id=305) patch to systest that shows off the bug.
          Hide
          Ed Burns added a comment -

          Created an attachment (id=306)
          systest with patch applied.

          Show
          Ed Burns added a comment - Created an attachment (id=306) systest with patch applied.
          Hide
          Ed Burns added a comment -

          I'm really sorry to do this, but we simply don't have time to make this into 2.1.

          If someone wants to submit a patch, maybe we can get it in under the wire. If so, I suggest they start by
          taking a look at FaceletViewHandlingStrategy.java, line 1408:

          ((ActionSource2) target)
          .setActionExpression(
          new ContextualCompositeMethodExpression(((sourceValue instanceof ValueExpression)
          ? (ValueExpression) sourceValue
          : null),
          me));

          }

          This is where the ClassCastException mentioned by Jakob originates.

          I'd approach a first approximation of this feature by introducing some isCompositeComponent() test
          there and take action accordingly.

          Show
          Ed Burns added a comment - I'm really sorry to do this, but we simply don't have time to make this into 2.1. If someone wants to submit a patch, maybe we can get it in under the wire. If so, I suggest they start by taking a look at FaceletViewHandlingStrategy.java, line 1408: ((ActionSource2) target) .setActionExpression( new ContextualCompositeMethodExpression(((sourceValue instanceof ValueExpression) ? (ValueExpression) sourceValue : null), me)); } This is where the ClassCastException mentioned by Jakob originates. I'd approach a first approximation of this feature by introducing some isCompositeComponent() test there and take action accordingly.
          Hide
          Ed Burns added a comment -

          On Friday 22 October at 18:33 EDT, Martin Marinschek wrote:

          MM> Hi guys,

          MM> my strong believe is that the target attribute should be immediately
          MM> deprecated.

          MM> It is wrongly placed where it is right now - an interface definition
          MM> should never provide hooks into the implementation. It should only
          MM> provide a meaningful context to which the implementation can refer.

          The targets attribute is central to the entire concept of composite
          components. I assert that it does not constitute a violation of the
          abstraction because the targets attribute is not intended for
          consumption by the page author. In fact, it is an implementation detail
          that happens to reside within the <cc:interface> section.

          MM> I am sorry I haven't voiced this objection before - but at the time
          MM> this was discussed I was not very active on the EG, and then it was
          MM> too late to talk about this.

          Yes, I'll say! I recall that we talked about this in December 2007. I
          know because we had the EG meeting while I was in the hotel at
          JavaPolis and I remember such a unique venue.

          >>>>> On Tue, 19 Oct 2010 14:30:17 -0500, Leonardo Uribe <lu4242@gmail.com> said:

          LU> I like the way it works now. For example, tomahawk t:inputHtml
          LU> component was rewritten into a composite component, and it is a good
          LU> example about when it could be useful that the only requerimiento
          LU> for a component to be composite is implement NamingContainer:

          For what it's worth, Leonardo is happy with this way it works, provided
          we address his other issues.

          On Sat Oct 23 03:46:13 EDT 2010 Ganesh wrote:

          G> Martin, I like your proposal on deprecating "targets". But how would
          G> you then handle this case: Using a cc I nest <f:actionListener
          G> "for=a" ... /> and inside the cc there is a <cc:actionSource name="a"
          G> targets="b, c" /> where b and c are buttons? The logic of the "for"
          G> attributes only includes this: "If present, this attribute refers to
          G> the value of one of the exposed attached objects within the
          G> composite component inside of which this tag is nested." IMO
          G> deprecating "targets" would require opening "for" to refer to
          G> multiple exposed attached objects within the composite component
          G> inside of which this tag is nested just the way targets is doing this
          G> right now. "targets" kind of bundles them right now.

          Yes, exactly. There was a lot of back and forth between David Geary,
          Ken Paulsen, Kito Mann, Andy Schwartz, and myself on this.

          G> To make this more convenient omitting "for" could simply be a synonym
          G> representing a "for" for all nested actionSources, actionSource2s
          G> and CCs.

          I prefer to be explicit here.

          G> Please follow Jakob's proposal on adding action, actionListener,
          G> valueChangeListener and validator the attributes map
          G> (#

          {cc.attrs.action}, #{cc.attrs.actionListener},
          G> #{cc.attrs.valueChangeListener}, #{cc.attrs.validator}). This is what
          G> a developer would expect to happen (at least I did so) and one could
          G> easily pass them on to nested components.

          JK> I think we should do this a little differently:

          JK> 1) try to add the e.g. ActionListener to the component specified in
          JK> the targets attribute, if possible. If it is not possible, log a
          JK> warning.

          JK> 2) also expose the "well-known" attributes ("action",
          JK> "actionListener", ...) on the composite component attribute map, this
          JK> means being able to access the action attribute via #{cc.attrs.action}

          JK> (currently not possible, because those attributes are not put on the
          JK> attribute map).

          JK> Thus the user can use the targets attribute if he/she wants to (and of
          JK> course, if it is possible) and also use #

          {cc.attrs.action} to refer to
          JK> the action attribute (like to any other attribute).

          JK> Frankly I really don't understand why this was separated in the first
          JK> place. Of course, the targets attribute is neat, but IMHO it is also
          JK> kinda confusing. I find it a lot easier to access every attribute
          JK> (regardless if well-known or custom) via #{cc.attrs.attributeName}.
          JK> Furthermore using this approach you can support nesting normal
          JK> components and also nesting composite components.

          JK> In addition the targets attribute can't solve a scenario in which the
          JK> action attribute of the inner component is required (most likely the
          JK> attribute from a composite component), because you need to enter a
          JK> value for the attribute, but you currently can't use
          JK> #{cc.attrs.action}

          , because this one points "nowhere".

          Show
          Ed Burns added a comment - On Friday 22 October at 18:33 EDT, Martin Marinschek wrote: MM> Hi guys, MM> my strong believe is that the target attribute should be immediately MM> deprecated. MM> It is wrongly placed where it is right now - an interface definition MM> should never provide hooks into the implementation. It should only MM> provide a meaningful context to which the implementation can refer. The targets attribute is central to the entire concept of composite components. I assert that it does not constitute a violation of the abstraction because the targets attribute is not intended for consumption by the page author. In fact, it is an implementation detail that happens to reside within the <cc:interface> section. MM> I am sorry I haven't voiced this objection before - but at the time MM> this was discussed I was not very active on the EG, and then it was MM> too late to talk about this. Yes, I'll say! I recall that we talked about this in December 2007. I know because we had the EG meeting while I was in the hotel at JavaPolis and I remember such a unique venue. >>>>> On Tue, 19 Oct 2010 14:30:17 -0500, Leonardo Uribe <lu4242@gmail.com> said: LU> I like the way it works now. For example, tomahawk t:inputHtml LU> component was rewritten into a composite component, and it is a good LU> example about when it could be useful that the only requerimiento LU> for a component to be composite is implement NamingContainer: For what it's worth, Leonardo is happy with this way it works, provided we address his other issues. On Sat Oct 23 03:46:13 EDT 2010 Ganesh wrote: G> Martin, I like your proposal on deprecating "targets". But how would G> you then handle this case: Using a cc I nest <f:actionListener G> "for=a" ... /> and inside the cc there is a <cc:actionSource name="a" G> targets="b, c" /> where b and c are buttons? The logic of the "for" G> attributes only includes this: "If present, this attribute refers to G> the value of one of the exposed attached objects within the G> composite component inside of which this tag is nested." IMO G> deprecating "targets" would require opening "for" to refer to G> multiple exposed attached objects within the composite component G> inside of which this tag is nested just the way targets is doing this G> right now. "targets" kind of bundles them right now. Yes, exactly. There was a lot of back and forth between David Geary, Ken Paulsen, Kito Mann, Andy Schwartz, and myself on this. G> To make this more convenient omitting "for" could simply be a synonym G> representing a "for" for all nested actionSources, actionSource2s G> and CCs. I prefer to be explicit here. G> Please follow Jakob's proposal on adding action, actionListener, G> valueChangeListener and validator the attributes map G> (# {cc.attrs.action}, #{cc.attrs.actionListener}, G> #{cc.attrs.valueChangeListener}, #{cc.attrs.validator}). This is what G> a developer would expect to happen (at least I did so) and one could G> easily pass them on to nested components. JK> I think we should do this a little differently: JK> 1) try to add the e.g. ActionListener to the component specified in JK> the targets attribute, if possible. If it is not possible, log a JK> warning. JK> 2) also expose the "well-known" attributes ("action", JK> "actionListener", ...) on the composite component attribute map, this JK> means being able to access the action attribute via #{cc.attrs.action} JK> (currently not possible, because those attributes are not put on the JK> attribute map). JK> Thus the user can use the targets attribute if he/she wants to (and of JK> course, if it is possible) and also use # {cc.attrs.action} to refer to JK> the action attribute (like to any other attribute). JK> Frankly I really don't understand why this was separated in the first JK> place. Of course, the targets attribute is neat, but IMHO it is also JK> kinda confusing. I find it a lot easier to access every attribute JK> (regardless if well-known or custom) via #{cc.attrs.attributeName}. JK> Furthermore using this approach you can support nesting normal JK> components and also nesting composite components. JK> In addition the targets attribute can't solve a scenario in which the JK> action attribute of the inner component is required (most likely the JK> attribute from a composite component), because you need to enter a JK> value for the attribute, but you currently can't use JK> #{cc.attrs.action} , because this one points "nowhere".
          Hide
          lu4242 added a comment -

          Created an attachment (id=319)
          Patch to use cc.attrs.action EL

          Show
          lu4242 added a comment - Created an attachment (id=319) Patch to use cc.attrs.action EL
          Hide
          lu4242 added a comment -

          Created an attachment (id=320)
          Test demo using maven. To run type: mvn clean -Djsf=mojarra jetty:run

          Show
          lu4242 added a comment - Created an attachment (id=320) Test demo using maven. To run type: mvn clean -Djsf=mojarra jetty:run
          Hide
          lu4242 added a comment -

          Created an attachment (id=321)
          WAR file from test project

          Show
          lu4242 added a comment - Created an attachment (id=321) WAR file from test project
          Hide
          Ed Burns added a comment -

          Thanks for the patch. I'm evaluating it now.

          Show
          Ed Burns added a comment - Thanks for the patch. I'm evaluating it now.
          Hide
          Ed Burns added a comment -

          I applied the patch and re-ran a test page I had previously authored (on 13 October) and still see the
          ClassCastException.

          Can you please rework the patch to fix that?

          Thanks,

          Ed

          This time I'll attach a zip of the simple xhtml files.

          Show
          Ed Burns added a comment - I applied the patch and re-ran a test page I had previously authored (on 13 October) and still see the ClassCastException. Can you please rework the patch to fix that? Thanks, Ed This time I'll attach a zip of the simple xhtml files.
          Hide
          Ed Burns added a comment -

          Created an attachment (id=325)
          zip of composite component files

          Show
          Ed Burns added a comment - Created an attachment (id=325) zip of composite component files
          Hide
          lu4242 added a comment -

          Created an attachment (id=330)
          Patch solving problem cc:attribute targets to nested composite components

          Show
          lu4242 added a comment - Created an attachment (id=330) Patch solving problem cc:attribute targets to nested composite components
          Hide
          lu4242 added a comment -

          Created an attachment (id=331)
          WAR file from test project

          Show
          lu4242 added a comment - Created an attachment (id=331) WAR file from test project
          Hide
          lu4242 added a comment -

          Created an attachment (id=332)
          Test demo using maven. To run type: mvn clean -Djsf=mojarra jetty:run

          Show
          lu4242 added a comment - Created an attachment (id=332) Test demo using maven. To run type: mvn clean -Djsf=mojarra jetty:run
          Hide
          lu4242 added a comment -

          Created an attachment (id=335)
          Full solution merging two previous patches

          Show
          lu4242 added a comment - Created an attachment (id=335) Full solution merging two previous patches
          Hide
          Ed Burns added a comment -

          Snapshot of reproducer, in progress.

          Show
          Ed Burns added a comment - Snapshot of reproducer, in progress.
          Hide
          Ed Burns added a comment -

          Set priority to baseline ahead of JSF 2.3 triage. Priorities will be assigned accurately after this exercise.

          Show
          Ed Burns added a comment - Set priority to baseline ahead of JSF 2.3 triage. Priorities will be assigned accurately after this exercise.
          Hide
          Manfred Riem added a comment -

          Setting priority to Major

          Show
          Manfred Riem added a comment - Setting priority to Major

            People

            • Assignee:
              Unassigned
              Reporter:
              Jakob Korherr
            • Votes:
              8 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated: