Re: Request for code review for issue 1696

From: Ed Burns <>
Date: Wed, 8 Sep 2010 18:50:00 -0700

>>>>> On Wed, 08 Sep 2010 11:12:28 -0700, Sheetal Vartak <> said:

SV> If the action on the button is not specified correctly i.e. in our
SV> case the wrong action is "/submit.xhtml". This does not get resolved
SV> to anything during the Invoke App phase's handling of the
SV> navigation. In this case, we need to see a good error message rather
SV> than the NPE which is currently being thrown.

SV> Index: jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/
SV> ===================================================================
SV> --- jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ (revision 8594)
SV> +++ jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ (working copy)
SV> @@ -174,6 +174,8 @@
SV> if (c instanceof NamingContainer) {
SV> oldUnique = ComponentSupport.setNeedUniqueIds(ctx, false);
SV> setUniqueIds = true;
SV> + if (createCompositeComponentDelegate != null)
SV> + createCompositeComponentDelegate.setCompositeComponent(c);

First, our coding style (even though some find it it unreadable)
dictates we always use curly braces with an if/else statement.

Second, it's not appropriate to assume that just because c is a
NamingContainer it is a composite component. Please add an additional
operand to your conditional UIComponent.isCompositeComponent(c).

Like this:

           if (createCompositeComponentDelegate != null &&
               UIComponent.isCompositeComponent(c)) {

Otherwise, looks good. r=edburns


| | office: +1 407 458 0017
| homepage:               |
|  0 work days until JSF 2.1 Milestone 5