Another round of review.
Attached is the changebundle to make sure that the error message gets printed only once for that request.
Thanks
Sheetal
<< ADD DESCRIPTION HERE
https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=XXXX >>
SECTION: Modified Files
----------------------------
M jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java
M jsf-ri/src/main/resources/com/sun/faces/resources/Messages.properties
M jsf-ri/src/main/resources/com/sun/faces/resources/Messages_en.properties
SECTION: Diffs
----------------------------
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 8601)
+++ jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java (working copy)
@@ -64,6 +64,9 @@
import javax.faces.component.UIViewRoot;
import javax.faces.component.html.HtmlForm;
import javax.faces.component.UINamingContainer;
+import javax.faces.component.ActionSource;
+import javax.faces.component.ActionSource2;
+import javax.faces.component.EditableValueHolder;
import javax.faces.application.FacesMessage;
import javax.faces.application.ProjectStage;
import javax.faces.context.FacesContext;
@@ -459,17 +462,20 @@
//no HtmlForm in the ancestry of the child
String key = MessageUtils.MISSING_FORM_ERROR;
Object[] params = new Object[]{};
+ boolean missingFormReported = false;
- // PENDING(sheetal): make it so the message is only queued
- // once per page. Change the english message text (don't bother
- // changing the i18ns) to be (all on one line)
- // the form component needs to have a UIForm in its ancestry.
- // Suggestion: enclose the necessary components within
- // <h:form>
-
FacesMessage m = MessageUtils.getExceptionMessage(key, params);
- m.setSeverity(FacesMessage.SEVERITY_WARN);
- ctx.getFacesContext().addMessage(null, m);
+ List<FacesMessage> messageList = ctx.getFacesContext().getMessageList();
+ for (FacesMessage fm : messageList) {
+ if (fm.getDetail().equals(m.getDetail())) {
+ missingFormReported = true;
+ break;
+ }
+ }
+ if (!missingFormReported) {
+ m.setSeverity(FacesMessage.SEVERITY_WARN);
+ ctx.getFacesContext().addMessage(null, m);
+ }
}
}
}
Index: jsf-ri/src/main/resources/com/sun/faces/resources/Messages.properties
===================================================================
--- jsf-ri/src/main/resources/com/sun/faces/resources/Messages.properties (revision 8601)
+++ jsf-ri/src/main/resources/com/sun/faces/resources/Messages.properties (working copy)
@@ -180,4 +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>.
+com.sun.faces.MISSING_FORM_ERROR=The form component needs to have a UIForm in its ancestry. Suggestion: enclose the necessary components within <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 8601)
+++ jsf-ri/src/main/resources/com/sun/faces/resources/Messages_en.properties (working copy)
@@ -180,4 +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>.
+com.sun.faces.MISSING_FORM_ERROR=The form component needs to have a UIForm in its ancestry. Suggestion: enclose the necessary components within <h:form>
On Sep 1, 2010, at 11:08 PM, Sheetal Vartak wrote:
> 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
> <changebundle.txt><newfiles.jar>
> 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
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net