Hi Ed,
Attached are the changes and the new text case. Please review.
I had a add a few more checks to ComponentSupport.java in order to avoid cases where the the component to add is the HtmlForm itself. Also in the case of composite components, when going up the hierarchy from the composite component to UINamingContainer, the parent of UINamingContainer is null. So I had to make sure my code worked in that case.
All the tests passed with my changes.
Thanks
Sheetal
https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1663
SECTION: Modified Files
----------------------------
M jsf-ri/test/com/sun/faces/util/TestUtil_messages.java
M jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java
M jsf-ri/src/main/java/com/sun/faces/util/MessageUtils.java
M jsf-ri/src/main/resources/com/sun/faces/resources/Messages.properties
M jsf-ri/src/main/resources/com/sun/faces/resources/Messages_zh_CN.properties
M jsf-ri/src/main/resources/com/sun/faces/resources/Messages_en.properties
M jsf-ri/src/main/resources/com/sun/faces/resources/Messages_pt_BR.properties
M jsf-ri/src/main/resources/com/sun/faces/resources/Messages_fr.properties
M jsf-ri/src/main/resources/com/sun/faces/resources/Messages_es.properties
M jsf-ri/src/main/resources/com/sun/faces/resources/Messages_de.properties
M jsf-ri/src/main/resources/com/sun/faces/resources/Messages_ko.properties
M jsf-ri/src/main/resources/com/sun/faces/resources/Messages_ja.properties
M jsf-ri/src/main/resources/com/sun/faces/resources/Messages_zh_TW.properties
M jsf-ri/systest/build-tests.xml
SECTION: Diffs
----------------------------
Index: jsf-ri/test/com/sun/faces/util/TestUtil_messages.java
===================================================================
--- jsf-ri/test/com/sun/faces/util/TestUtil_messages.java (revision 8588)
+++ jsf-ri/test/com/sun/faces/util/TestUtil_messages.java (working copy)
@@ -203,7 +203,8 @@
{MessageUtils.PARTIAL_STATE_ERROR_RESTORING_ID, "2"},
{MessageUtils.MISSING_COMPONENT_ATTRIBUTE_VALUE, "1"},
{MessageUtils.MISSING_COMPONENT_FACET, "1"},
- {MessageUtils.MISSING_COMPONENT_METADATA, "1" }
+ {MessageUtils.MISSING_COMPONENT_METADATA, "1" },
+ {MessageUtils.MISSING_FORM_ERROR, "0"}
};
// Attribute Instance Variables
Index: jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java
===================================================================
--- jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java (revision 8588)
+++ jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java (working copy)
@@ -56,15 +56,21 @@
import com.sun.faces.context.StateContext;
import com.sun.faces.facelets.tag.jsf.core.FacetHandler;
+import com.sun.faces.util.MessageUtils;
import javax.faces.FacesException;
import javax.faces.component.UIComponent;
import javax.faces.component.UIPanel;
import javax.faces.component.UIViewRoot;
+import javax.faces.component.html.HtmlForm;
+import javax.faces.component.UINamingContainer;
+import javax.faces.application.ProjectStage;
import javax.faces.context.FacesContext;
import javax.faces.view.facelets.FaceletContext;
import javax.faces.view.facelets.TagAttribute;
import javax.faces.view.facelets.TagAttributeException;
+import javax.faces.application.FacesMessage;
+
import java.io.IOException;
import java.util.Collection;
import java.util.HashMap;
@@ -398,6 +404,48 @@
}
}
}
+
+ /**
+ * method to inspect what interfaces the component implements.
+ * if one of the interfaces is ActionSource, ActionSource2 or EditableValueHolder
+ * then that component needs to be wrapped inside an HtmlForm.
+ * @param interfaces
+ * @return
+ */
+
+ private static boolean inspectInterfacesToCheckIfFormOmitted(Class[] interfaces) {
+ for (int i = 0; i < interfaces.length; i++) {
+ String name = interfaces[i].getName();
+
+ if (name.equals("javax.faces.component.ActionSource") ||
+ name.equals("javax.faces.component.ActionSource2") ||
+ name.equals("javax.faces.component.EditableValueHolder") ||
+ name.equals("javax.faces.component.behavior.ClientBehaviorHolder")) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ /**
+ * method to inspect of the the parent ancestry of the component
+ * contains HtmlForm as one of the ancestors.
+ * @param component
+ * @return true of HtmlForm is one of the ancestors
+ */
+
+ private static boolean inspectParentAncestryToCheckIfFormOmitted(UIComponent component) {
+ if (component != null) {
+ while (!(component instanceof UIViewRoot) && component != null) {
+ if (component instanceof HtmlForm ||
+ component instanceof UINamingContainer) {
+ return true;
+ }
+ component = component.getParent();
+ }
+ }
+ return false;
+ }
/**
* <p class="changed_added_2_0">Add the child component to the parent. If the parent is a facet,
@@ -406,7 +454,27 @@
* does not yet exist, make the child the facet.</p>
*/
public static void addComponent(FaceletContext ctx, UIComponent parent, UIComponent child) {
-
+
+ //fix for issue 1663. to be executed only if in dev mode
+ if (ctx.getFacesContext().isProjectStage(ProjectStage.Development)) {
+ if (!(child instanceof HtmlForm)) {
+ Class[] interfaces = child.getClass().getInterfaces();
+ if (inspectInterfacesToCheckIfFormOmitted(interfaces)) {
+ //child component implements ActionSource/ActionSource2
+ //or EditableValueHolder
+ //now make sure that there is a HtmlForm in the ancestry
+ if (!inspectParentAncestryToCheckIfFormOmitted(parent)) {
+ //no HtmlForm in the ancestry of the child
+ String key = MessageUtils.MISSING_FORM_ERROR;
+ Object[] params = new Object[]{};
+
+ FacesMessage m = MessageUtils.getExceptionMessage(key, params);
+ m.setSeverity(FacesMessage.SEVERITY_WARN);
+ ctx.getFacesContext().addMessage(null, m);
+ }
+ }
+ }
+ }
String facetName = getFacetName(parent);
if (facetName == null) {
parent.getChildren().add(child);
Index: jsf-ri/src/main/java/com/sun/faces/util/MessageUtils.java
===================================================================
--- jsf-ri/src/main/java/com/sun/faces/util/MessageUtils.java (revision 8588)
+++ jsf-ri/src/main/java/com/sun/faces/util/MessageUtils.java (working copy)
@@ -334,8 +334,9 @@
"com.sun.faces.MISSING_COMPONENT_FACET";
public static final String MISSING_COMPONENT_METADATA =
"com.sun.faces.MISSING_COMPONENT_METADATA";
+ public static final String MISSING_FORM_ERROR =
+ "com.sun.faces.MISSING_FORM_ERROR";
-
// ------------------------------------------------------------ Constructors
Index: jsf-ri/src/main/resources/com/sun/faces/resources/Messages.properties
===================================================================
--- jsf-ri/src/main/resources/com/sun/faces/resources/Messages.properties (revision 8588)
+++ jsf-ri/src/main/resources/com/sun/faces/resources/Messages.properties (working copy)
@@ -180,3 +180,4 @@
com.sun.faces.MISSING_COMPONENT_ATTRIBUTE_VALUE=The following attribute(s) are required, but no values have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_FACET=The following facets(s) are required, but no facets have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_METADATA=The composite component metadata for component with clientid ''{0}'' cannot be found.
+com.sun.faces.MISSING_FORM_ERROR=The button/link/text component needs to have a Form in its ancestry. Please add <h:form>.
Index: jsf-ri/src/main/resources/com/sun/faces/resources/Messages_zh_CN.properties
===================================================================
--- jsf-ri/src/main/resources/com/sun/faces/resources/Messages_zh_CN.properties (revision 8588)
+++ jsf-ri/src/main/resources/com/sun/faces/resources/Messages_zh_CN.properties (working copy)
@@ -180,3 +180,4 @@
com.sun.faces.MISSING_COMPONENT_ATTRIBUTE_VALUE=The following attribute(s) are required, but no values have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_FACET=The following facets(s) are required, but no facets have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_METADATA=The composite component metadata for component with clientid ''{0}'' cannot be found.
+com.sun.faces.MISSING_FORM_ERROR=The button/link/text component needs to have a Form in its ancestry. Please add <h:form>.
Index: jsf-ri/src/main/resources/com/sun/faces/resources/Messages_en.properties
===================================================================
--- jsf-ri/src/main/resources/com/sun/faces/resources/Messages_en.properties (revision 8588)
+++ jsf-ri/src/main/resources/com/sun/faces/resources/Messages_en.properties (working copy)
@@ -180,3 +180,4 @@
com.sun.faces.MISSING_COMPONENT_ATTRIBUTE_VALUE=The following attribute(s) are required, but no values have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_FACET=The following facets(s) are required, but no facets have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_METADATA=The composite component metadata for component with clientid ''{0}'' cannot be found.
+com.sun.faces.MISSING_FORM_ERROR=The button/link/text component needs to have a Form in its ancestry. Please add <h:form>.
Index: jsf-ri/src/main/resources/com/sun/faces/resources/Messages_pt_BR.properties
===================================================================
--- jsf-ri/src/main/resources/com/sun/faces/resources/Messages_pt_BR.properties (revision 8588)
+++ jsf-ri/src/main/resources/com/sun/faces/resources/Messages_pt_BR.properties (working copy)
@@ -180,3 +180,4 @@
com.sun.faces.MISSING_COMPONENT_ATTRIBUTE_VALUE=The following attribute(s) are required, but no values have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_FACET=The following facets(s) are required, but no facets have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_METADATA=The composite component metadata for component with clientid ''{0}'' cannot be found.
+com.sun.faces.MISSING_FORM_ERROR=The button/link/text component needs to have a Form in its ancestry. Please add <h:form>.
Index: jsf-ri/src/main/resources/com/sun/faces/resources/Messages_fr.properties
===================================================================
--- jsf-ri/src/main/resources/com/sun/faces/resources/Messages_fr.properties (revision 8588)
+++ jsf-ri/src/main/resources/com/sun/faces/resources/Messages_fr.properties (working copy)
@@ -180,3 +180,4 @@
com.sun.faces.MISSING_COMPONENT_ATTRIBUTE_VALUE=The following attribute(s) are required, but no values have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_FACET=The following facets(s) are required, but no facets have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_METADATA=The composite component metadata for component with clientid ''{0}'' cannot be found.
+com.sun.faces.MISSING_FORM_ERROR=The button/link/text component needs to have a Form in its ancestry. Please add <h:form>.
Index: jsf-ri/src/main/resources/com/sun/faces/resources/Messages_es.properties
===================================================================
--- jsf-ri/src/main/resources/com/sun/faces/resources/Messages_es.properties (revision 8588)
+++ jsf-ri/src/main/resources/com/sun/faces/resources/Messages_es.properties (working copy)
@@ -180,3 +180,4 @@
com.sun.faces.MISSING_COMPONENT_ATTRIBUTE_VALUE=The following attribute(s) are required, but no values have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_FACET=The following facets(s) are required, but no facets have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_METADATA=The composite component metadata for component with clientid ''{0}'' cannot be found.
+com.sun.faces.MISSING_FORM_ERROR=The button/link/text component needs to have a Form in its ancestry. Please add <h:form>.
Index: jsf-ri/src/main/resources/com/sun/faces/resources/Messages_de.properties
===================================================================
--- jsf-ri/src/main/resources/com/sun/faces/resources/Messages_de.properties (revision 8588)
+++ jsf-ri/src/main/resources/com/sun/faces/resources/Messages_de.properties (working copy)
@@ -177,6 +177,7 @@
com.sun.faces.RESOURCE_TARGET_NOT_AVAILABLE=Eine oder mehrere Ressourcen haben das Ziel ''{0}'', aber es wurde keine Komponente ''{0}'' in der Ansicht definiert.
com.sun.faces.ARGUMENTS_NOT_LEGAL_WITH_CC_ATTRS_EXPR=Illegal attempt to pass arguments to a composite component lookup expression (i.e. cc.attrs.[identifier]).
com.sun.faces.partial.statesaving.ERROR_RESTORING_STATE_FOR_COMPONENT=Unexpected error restoring state for component with id ''{0}''. Cause: {1}.
-com.sun.faces.MISSING_COMPONENT_ATTRIBUTE_VALUE=Folgendes Eigenschaften sind erforderlich, aber leider Sie haben keinen Werte übergegeben: ''{0}''.
-com.sun.faces.MISSING_COMPONENT_FACET=Folgendes facets(s) sind erforderlich, aber lieder Sie haben keinen Facets übergegeben: ''{0}''.
-com.sun.faces.MISSING_COMPONENT_METADATA=Composite Component metadata für component clientid ''{0}'' nicht gefunden.
+com.sun.faces.MISSING_COMPONENT_ATTRIBUTE_VALUE=Folgendes Eigenschaften sind erforderlich, aber leider Sie haben keinen Werte \u00fcbergegeben: ''{0}''.
+com.sun.faces.MISSING_COMPONENT_FACET=Folgendes facets(s) sind erforderlich, aber lieder Sie haben keinen Facets \u00fcbergegeben: ''{0}''.
+com.sun.faces.MISSING_COMPONENT_METADATA=Composite Component metadata f\u00fcr component clientid ''{0}'' nicht gefunden.
+com.sun.faces.MISSING_FORM_ERROR=The button/link/text component needs to have a Form in its ancestry. Please add <h:form>.
Index: jsf-ri/src/main/resources/com/sun/faces/resources/Messages_ko.properties
===================================================================
--- jsf-ri/src/main/resources/com/sun/faces/resources/Messages_ko.properties (revision 8588)
+++ jsf-ri/src/main/resources/com/sun/faces/resources/Messages_ko.properties (working copy)
@@ -180,3 +180,4 @@
com.sun.faces.MISSING_COMPONENT_ATTRIBUTE_VALUE=The following attribute(s) are required, but no values have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_FACET=The following facets(s) are required, but no facets have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_METADATA=The composite component metadata for component with clientid ''{0}'' cannot be found.
+com.sun.faces.MISSING_FORM_ERROR=The button/link/text component needs to have a Form in its ancestry. Please add <h:form>.
Index: jsf-ri/src/main/resources/com/sun/faces/resources/Messages_ja.properties
===================================================================
--- jsf-ri/src/main/resources/com/sun/faces/resources/Messages_ja.properties (revision 8588)
+++ jsf-ri/src/main/resources/com/sun/faces/resources/Messages_ja.properties (working copy)
@@ -180,3 +180,4 @@
com.sun.faces.MISSING_COMPONENT_ATTRIBUTE_VALUE=The following attribute(s) are required, but no values have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_FACET=The following facets(s) are required, but no facets have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_METADATA=The composite component metadata for component with clientid ''{0}'' cannot be found.
+com.sun.faces.MISSING_FORM_ERROR=The button/link/text component needs to have a Form in its ancestry. Please add <h:form>.
Index: jsf-ri/src/main/resources/com/sun/faces/resources/Messages_zh_TW.properties
===================================================================
--- jsf-ri/src/main/resources/com/sun/faces/resources/Messages_zh_TW.properties (revision 8588)
+++ jsf-ri/src/main/resources/com/sun/faces/resources/Messages_zh_TW.properties (working copy)
@@ -180,3 +180,4 @@
com.sun.faces.MISSING_COMPONENT_ATTRIBUTE_VALUE=The following attribute(s) are required, but no values have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_FACET=The following facets(s) are required, but no facets have been supplied for them: ''{0}''.
com.sun.faces.MISSING_COMPONENT_METADATA=The composite component metadata for component with clientid ''{0}'' cannot be found.
+com.sun.faces.MISSING_FORM_ERROR=The button/link/text component needs to have a Form in its ancestry. Please add <h:form>.
Index: jsf-ri/systest/build-tests.xml
===================================================================
--- jsf-ri/systest/build-tests.xml (revision 8588)
+++ jsf-ri/systest/build-tests.xml (working copy)
@@ -125,11 +125,11 @@
test.component,
test.component01,
test.navigation,
+ test.formomitted,
test.implicitnav,
test.state,
test.resourceBundleELResolver,
@@ -658,7 +659,22 @@
</target>
+
+ <target name="test.formomitted"
+ description="Test missing form">
+
+ <jsf.junit context-path="${context.path}"
+ classpath-refid="html.classpath"
+ test-results-dir="${impl.test.results.dir}">
+ <tests>
+ <fileset dir="${basedir}/build/classes"
+ includes="com/sun/faces/systest/FormOmittedTestCase.class"/>
+ </tests>
+ </jsf.junit>
+
+ </target>
+
<target name="test.state"
description="Test new state manager">
On Aug 31, 2010, at 7:35 AM, Ed Burns wrote:
>>>>>> On Fri, 27 Aug 2010 10:34:06 -0700, Sheetal Vartak <sheetal.vartak_at_oracle.com> said:
>
> SV> Thanks for your comments. Attaching the changes incorporated with
> SV> the feedback. Please review.
>
> Summary: It's getting there, but not production ready yet. please read
> these comments and produce another iteration of the changebundle.
>
> SECTION: Review on the testcase portion of the change bundle
>
> I hate to do this, because I know you want to get it in, but I really
> need an automated test to be included as well.
>
> The good news is that it's not hard to add one in this case. I'll
> sketch it out for you here. I hope this proves useful to you in
> general. If so, please consider adding some content to the Mojarra FAQ.
>
> 1. Look for an existing test that does something similar
>
> I thought there might be an existing test that deals with other
> ProjectStage==Development features. I know that one of those features
> deals with messsages, so I ran a find command in jsf-ri/systest
> looking for .java files containing the string "messages". I found
> com/sun/faces/systest/NavigationTestCase.java.
>
> This one is in the right neighborhood, but it's not a perfect fit. If
> you wanted to be a "Done, and Gets Things Smart" [1] person, you could
> make a new java package for tests that deal with
> ProjectStage==Development features and put NavigationTestCase and your
> new test case in there. However, that's you're call. I'm not going
> to ask you to do that.
>
> In any case, I suggest using your IDE's refactor copy feature to copy
> NavigationTestCase to MissingFormTestCase.
>
> 2. make a new xhtml file. Looking at
> NavigationTestCase.testNullOutcomeNoMessages(), it points to an xhtml
> file. Again, it's not a perfect fit, and you could refactor and make
> a new diretory, etc. I suggest copying the xhtml file mentioned in
> that test method to a new place,
> jsf-ri/systest/web/facelets/missingForm.xhtml, and hacking away all
> the markup and replacing it with some markup that would cause your
> error message to be displayed.
>
> 3. Hack your new MissingFormTestCase and replace it with the necessary
> content to assert that the correct message is displayed.
>
> SECTION: Review on the production code portion of the change bundle
>
> M jsf-ri/src/main/resources/com/sun/faces/resources/Messages.properties
>
> +com.sun.faces.FORM_OMITTED_FOR_ACTIONSOURCE=The button/link component
> needs to have a Form in its ancestry. Please add <h:form>.
>
> - It looks like this error message *and* symbolic constant was not
> updated when your understanding of the bug-fix was updated to reflect
> the desired intent.
>
> - When you go about adding a message like this, you have to ensure it's
> localizable. Please take a look at the diffs of Andy's recent commits
> regarding issue 1800 to see how to do this. svn log and svn diff are
> your friend here. svn-book.pdf is definitely also your friend.
>
> M jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java
>
> - + private static boolean inspectInterfaces(Class[] interfaces) {
>
> The comment on the method is accurate, but the method name is not.
> Please rename to a more descriptive name.
>
> - + private static boolean inspectParentAncestry(UIComponent
> component) {
>
> Same here.
>
> M jsf-ri/src/main/java/com/sun/faces/util/MessageUtils.java
>
> - It looks like you missed this comment
>
> // IMPORTANT - ensure that any new message constant is properly
> // tested in test/com/sun/faces/util/TestUtil_messages (see comments
> // in test class for details on the test).
>
> No problem, it's easy to miss.
>
>
>
> [1] http://steve-yegge.blogspot.com/2008/06/done-and-gets-things-smart.html
>
> --
> | ed.burns_at_sun.com | office: +1 407 458 0017
> | homepage: | http://ridingthecrest.com/
> | 5 work days until JSF 2.1 Milestone 5
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>