dev@javaserverfaces.java.net

Re: issue 1663

From: Sheetal Vartak <sheetal.vartak_at_oracle.com>
Date: Thu, 21 Oct 2010 16:17:33 -0700

Hi Ed,
Thanks for pointing out the issue with running scrumtoys. I have made a small change that has fixed the issue with UIViewParameter requiring a form. Please take a look. All the tests also passed.

Thanks
Sheetal

https://javaserverfaces.dev.java.net/issues/show_bug.cgi?id=1663

Modified file :

M jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java

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 8675)
+++ jsf-ri/src/main/java/com/sun/faces/facelets/tag/jsf/ComponentSupport.java (working copy)
@@ -66,8 +66,10 @@
 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.UIForm;
+import javax.faces.component.UIData;
 import javax.faces.component.UINamingContainer;
+import javax.faces.component.UIViewParameter;
 import javax.faces.component.ActionSource;
 import javax.faces.component.ActionSource2;
 import javax.faces.component.EditableValueHolder;
@@ -86,9 +88,6 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
-import javax.faces.component.ActionSource;
-import javax.faces.component.ActionSource2;
-import javax.faces.component.EditableValueHolder;
 import javax.faces.view.facelets.Tag;
 
 /**
@@ -102,6 +101,14 @@
     public final static String MARK_CREATED = "com.sun.faces.facelets.MARK_ID";
     private final static String IMPLICIT_PANEL = "com.sun.faces.facelets.IMPLICIT_PANEL";
 
+ //this boolean is used for issue 1663.
+ //once a child is discovered as requiring Form to be in ancestry
+ //one needs to remember it with this boolean
+ //so that when UIPanel/UIData/UINamingContainer is the child,
+ //we can look up the ancestry chain to see if
+ //Form exists.
+ private static boolean makeSureAncestorIsForm = false;
+
     /**
      * Key to a FacesContext scoped Map where the keys are UIComponent instances and the
      * values are Tag instances.
@@ -417,28 +424,32 @@
     /**
      * 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 a UIlForm.
+ * then that component needs to be wrapped inside a UIForm.
      * return true if even one of the interfaces is one of the above.
      * @param child
      * @return boolean
      */
 
     private static boolean inspectInterfacesToCheckIfFormOmitted(UIComponent child) {
- return (child instanceof ActionSource || child instanceof ActionSource2 || child instanceof EditableValueHolder);
+ if (child instanceof UIViewParameter) {
+ return false;
+ }
+ return (child instanceof ActionSource ||
+ child instanceof ActionSource2 ||
+ child instanceof EditableValueHolder);
     }
 
     /**
      * method to inspect of the the parent ancestry of the component
- * contains HtmlForm as one of the ancestors.
+ * contains UIForm as one of the ancestors.
      * @param component
- * @return true of HtmlForm is one of the ancestors
+ * @return true of UIForm 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) {
+ if (component instanceof UIForm) {
                     return true;
                 }
                 component = component.getParent();
@@ -446,6 +457,29 @@
         }
         return false;
     }
+
+ /**
+ * method for adding a message regarding missing form to context
+ * @param ctx
+ */
+ private static void addFormOmittedMessage(FaceletContext ctx) {
+ String key = MessageUtils.MISSING_FORM_ERROR;
+ Object[] params = new Object[]{};
+ boolean missingFormReported = false;
+
+ FacesMessage m = MessageUtils.getExceptionMessage(key, params);
+ 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);
+ }
+ }
     
     /**
      * <p class="changed_added_2_0">Add the child component to the parent. If the parent is a facet,
@@ -456,30 +490,58 @@
     public static void addComponent(FaceletContext ctx, UIComponent parent, UIComponent child) {
 
         //fix for issue 1663. to be executed only if in dev mode
+ //The problem is that, with facelets,
+ //the facelet.apply() method is *ALWAYS* called from leaf to root.
+ //Therefore, when you try to ascend the hierarchy, you *ALWAYS* get parent == null
+ //For this reason, I have introduced a boolean "makeSureAncestorIsForm"
+ //If the child has UIPanel/UIData/UINamingContainer as the parent,
+ //and that parent does not have
+ //a parent yet or has null in the ancestry,
+ //then makeSureAncestorIsForm = true. This boolean will be looked at
+ //when the UIPanel/UIData/UINamingContainer is a child and has to be added.
+ //At that time, if the parent of one of the above is not one of the above,
+ //then check if Form is in the ancestry. If not, add error message
+
         if (ctx.getFacesContext().isProjectStage(ProjectStage.Development)) {
- if (!(child instanceof HtmlForm)) {
+ if (!(child instanceof UIForm)) {
+
+ if ((child instanceof UIPanel
+ || child instanceof UINamingContainer
+ || child instanceof UIData)
+ && makeSureAncestorIsForm) {
+
+ if (!(parent instanceof UIPanel
+ || parent instanceof UINamingContainer
+ || parent instanceof UIData)) {
+
+ if (!inspectParentAncestryToCheckIfFormOmitted(parent)) {
+ //no HtmlForm in the ancestry of the child
+ addFormOmittedMessage(ctx);
+ }
+ makeSureAncestorIsForm = false;
+ } //else don't do anything yet. wait until the parent becomes the child
+ }
+ //child component implements ActionSource/ActionSource2
+ //or EditableValueHolder.
+ //the child needs to have the Form in its ancestry
+ //now make sure that there is a HtmlForm in the ancestry
                 if (inspectInterfacesToCheckIfFormOmitted(child)) {
- //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[]{};
- boolean missingFormReported = false;
 
- FacesMessage m = MessageUtils.getExceptionMessage(key, params);
- List<FacesMessage> messageList = ctx.getFacesContext().getMessageList();
- for (FacesMessage fm : messageList) {
- if (fm.getDetail().equals(m.getDetail())) {
- missingFormReported = true;
- break;
- }
+ if (parent instanceof UIPanel
+ || parent instanceof UINamingContainer
+ || parent instanceof UIData) {
+ // in this case, the UIPanel in most cases is still parentless
+ //so don't dismiss this as an error case.
+ //remember the fact that we need to come back to this case later
+ //when the UIPanel is a child and has a parent and needs to be added
+ //to the view
+ makeSureAncestorIsForm = true;
+
+ } else {
+ if (!inspectParentAncestryToCheckIfFormOmitted(parent)) {
+ //no HtmlForm in the ancestry of the child
+ addFormOmittedMessage(ctx);
                         }
- if (!missingFormReported) {
- m.setSeverity(FacesMessage.SEVERITY_WARN);
- ctx.getFacesContext().addMessage(null, m);
- }
                     }
                 }
             }



On Oct 20, 2010, at 3:00 PM, Sheetal Vartak wrote:

> Hi Ed,
>
> I took a look at the sketch of the solution that you have attached to the issue. I agree with the basic idea in the solution but not so much with adding the list of components to look at to the FacesContext attrs. I think there might be a better way to achieving this right in the ComponentSupport class. I may not have all my facts right. That's where I'm looking to you for guidance.
>
> Attaching the diffs here. I have tried to add as much documentation as possible.<changebundle.txt>
>
> What I have tried to do is similar to the old code. But now I have a boolean "makeSureAncestorIsForm" which is set if the child component's parent is either UIPanel or UIData or UINamingContainer. I have noticed that, when the parent is one of the above, the parent's ancestry is null somewhere along the line. Is this a possibility for any other kind of component as well?
> So the boolean helps us remember this case and take action when ComponentSupport.addComponent() is called for one of the above as children to be added. That's when we examine the value of the boolean. If it is set, we check for UIForm in the ancestry.
>
> The solution is very much specific to cases where UIPanel/UIData/UINamingContainer are parents. I don't know of any other components that might need special case like these. Please advise.
>
> Thanks
> Sheetal
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net