Issue Details (XML | Word | Printable)

Key: JAVASERVERFACES_SPEC_PUBLIC-755
Type: Sub-task Sub-task
Status: Open Open
Priority: Major Major
Assignee: Unassigned
Reporter: Jakob Korherr
Votes: 7
Watchers: 3
Operations

If you were logged in you would be able to see more operations.
javaserverfaces-spec-public
JAVASERVERFACES_SPEC_PUBLIC-901

passing through of actionListener, action,.. not possible between composite components

Created: 27/Feb/10 03:22 AM   Updated: 08/Nov/13 09:15 PM
Component/s: Facelets/VDL
Affects Version/s: 2.0
Fix Version/s: 2.3

Time Tracking:
Not Specified

File Attachments: 1. Text File 755-cc-attribute-full-solution.patch (45 kB) 03/Nov/10 04:19 PM - lu4242
2. Text File 755-cc-attribute-Nested-No-EL.patch (22 kB) 29/Oct/10 01:28 PM - lu4242
3. Text File 755-cc-attribute-with-EL.patch (29 kB) 26/Oct/10 05:16 PM - lu4242
4. Zip Archive 755-reproducer.zip (4 kB) 27/Oct/10 09:58 AM - Ed Burns
5. Zip Archive composite-component-targets-test-4.zip (42 kB) 29/Oct/10 01:52 PM - lu4242
6. File composite-component-targets-test.war (2.28 MB) 29/Oct/10 01:30 PM - lu4242
7. File composite-component-targets-test.war (2.26 MB) 26/Oct/10 05:19 PM - lu4242
8. Zip Archive composite-component-targets-test.zip (23 kB) 26/Oct/10 05:17 PM - lu4242
9. Text File diffs.patch (9 kB) 04/Feb/11 07:41 AM - Ed Burns
10. Text File diffs.patch (9 kB) 13/Oct/10 01:53 PM - Ed Burns
11. File jsf-systest.war (1.05 MB) 13/Oct/10 01:55 PM - Ed Burns
12. File jsf-systest.war (1.05 MB) 13/Oct/10 07:59 AM - Ed Burns

Environment:

Operating System: All
Platform: All

Issue Links:
Related
 

Issuezilla Id: 755
Status Whiteboard:

cat2 frame size_medium importance_large

Tags:
Participants: Ed Burns, ganeshpuri, Jakob Korherr and lu4242


 Description  « Hide

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>



Ed Burns added a comment - 04/Mar/10 12:22 PM

cat2


Ed Burns added a comment - 22/Mar/10 11:39 AM

frame


Ed Burns added a comment - 15/May/10 07:54 AM

These are targeted at 2.1.


Ed Burns added a comment - 08/Jun/10 01:17 PM

triage


Ed Burns added a comment - 24/Jun/10 02:41 PM

GlassFish 3.1 M6 at the latest.


Ed Burns added a comment - 10/Sep/10 01:31 PM

Move these to 2.2


ganeshpuri added a comment - 13/Oct/10 03:54 AM

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.


Ed Burns added a comment - 13/Oct/10 07:58 AM

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"; }


Ed Burns added a comment - 13/Oct/10 07:59 AM

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


Jakob Korherr added a comment - 13/Oct/10 10:05 AM

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?


Ed Burns added a comment - 13/Oct/10 01:52 PM

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


Ed Burns added a comment - 13/Oct/10 01:53 PM

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


Ed Burns added a comment - 13/Oct/10 01:55 PM

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


Ed Burns added a comment - 13/Oct/10 01:57 PM

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.


Ed Burns added a comment - 25/Oct/10 10:05 AM

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".


lu4242 added a comment - 26/Oct/10 05:16 PM

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


lu4242 added a comment - 26/Oct/10 05:17 PM

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


lu4242 added a comment - 26/Oct/10 05:19 PM

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


Ed Burns added a comment - 27/Oct/10 09:38 AM

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


Ed Burns added a comment - 27/Oct/10 09:57 AM

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.


Ed Burns added a comment - 27/Oct/10 09:58 AM

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


lu4242 added a comment - 29/Oct/10 01:28 PM

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


lu4242 added a comment - 29/Oct/10 01:30 PM

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


lu4242 added a comment - 29/Oct/10 01:52 PM

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


lu4242 added a comment - 03/Nov/10 04:19 PM

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


Ed Burns added a comment - 04/Feb/11 07:41 AM

Snapshot of reproducer, in progress.