On Fri, Jun 13, 2008 at 1:52 AM, simon <[EMAIL PROTECTED]> wrote:

>
> >
> >         (2)
> >         Now that the "template" property has been added to
> >         JSFComponent, can the
> >         following be deleted?
> >          parent
> >          superClass
> >
> > Yes, I did a revision and deleted the usage of this attributes (front
> > end). But this properties right now are used inside
> > myfaces-builder-plugin, since the concept is valid from the backend.
>
> Sorry, but I don't understand.
>
> I see that the QDoxModelBuilder still looks for "superClass" as an
> attribute on the JSFComponent tag, and the annotation class in
> myfaces-builder-annotations still has the attributes. Is it ok if I
> remove these or not?
>
>
I have checked again if we can remove superClass or parentClass from
QDoxModelBuilder(in other words do not read it from the doclet of
annotations) and now it is safe. But the template code (componentClassXX.vm
on myfaces and tomahawk) and the Flattener still uses these properties,
because this have sense.


> >  The code that do the magic of template is this:
> >
> >         Boolean template = getBoolean(clazz,"template",props, null);
> >
> >         // If the componentClass is not the same as the class with
> >         // @JSFComponent annotation, the component class is generated
> >         if (!clazz.getFullyQualifiedName().equals(componentClass)){
> >             if (!(template != null && template.booleanValue()))
> >             {
> >                 //If template is false or null, the superClass of the
> > generated
> >                 //class is the same class, otherwise is the same as
> > the parent
> >                 //class.
> >                 superClassName =
> > getString(clazz,"superClass",props,clazz.getFullyQualifiedName());
> >             }
> >         }
>
> When in template mode, isn't the parent-class for the generated class
> just the nearest ancestor class that is annotated with @JSFComponent? ie
> the template would use:
>  $clazz.parentClassName
> in the "extends" clause when generating code, and superClassName would
> not be used anywhere.
>

> ClassMeta.parentClassName is set up in QDoxModelBuilder.initAncestry,
> and I don't currently see any reason why we would want to use anything
> else, or provide any override for that.
>
> What am I missing?
>

To see the difference let's take t:buffer component

superClass: org.apache.myfaces.custom.buffer.AbstractBuffer (use this on the
template!)
parent : javax.faces.component.UIComponentBase (use this on the flattener).
(template: false)

now h:commandButton

superClass: javax.faces.component.UICommand
parent: javax.faces.component.UICommand
(template: true)

The problem is the case t:buffer (and all tomahawk). The parent is always
different from the superClass and both properties could not be derived from
other fields.


>
> >
> >
> >
> >         (3)
> >         I'd like to clean up JSFJspProperty, as it doesn't feel quite
> >         right t
> >         me.
> >
> >         == rename it?
> >
> >         Firstly, how about renaming it to JSFAttribute, and
> >         documenting it as
> >         something like:
> >
> >         <doc>
> >         Defines a logical property of a component that is accessed via
> >         the
> >         component's Attributes map, rather than javaBean getter/setter
> >         methods.
> >         </doc>
> >
> > Maybe, but we need to check the usage of JSFJspProperty and
> > JSFAttribute and check if this is
> > compatible or not.
>
> Ok, I've been looking into this (although I've not got much spare time,
> so it's not rapid progress).
>
>
> In Tomahawk, class AbstractHtmlDataScroller has these annotations:
>  * @JSFJspProperty name = "onkeydown" tagExcluded = "true"
>  * @JSFJspProperty name = "onkeypress" tagExcluded = "true"
>  * @JSFJspProperty name = "onkeyup" tagExcluded = "true"
>  * @JSFJspProperty name = "onmousedown" tagExcluded = "true"
>  * @JSFJspProperty name = "onmousemove" tagExcluded = "true"
>  * @JSFJspProperty name = "onmouseout" tagExcluded = "true"
>  * @JSFJspProperty name = "onmouseover" tagExcluded = "true"
>  * @JSFJspProperty name = "onmouseup" tagExcluded = "true"
>
> I can't currently figure out what these are for. These properties are in
> the Attributes map, so they don't need to be referenced in
> saveState/restoreState. And because tagExcluded is true, they are not
> written into the tld.
>
> So what do these annotations do?
>

This annotations remove from the tld this properties, because there are not
provided by the renderer, but inherited from
org.apache.myfaces.component.html.ext.HtmlPanelGroup.

Solutions?

1. Use the same pattern as rendered property for UISelectItem
2. Implement this properties on the renderer.


>
> >
> >         == tagExcluded
> >
> >         I'm also trying to figure out exactly what "tagExcluded=true"
> >         means on
> >         JSFJspProperty, and whether we can get rid of it.
>
> >
> > Really, id, binding and renderer are the only properties that
> > sometimes uses extensively
> > @JSFJspProperty tagExcluded=true, because override it could cause
> > problems or be imposible (binding)
>
> As you've probably seen, I've defined getter/setter methods for id and
> rendered on the relevant classes, and used
>  @JSFProperty tagExcluded=true
> on them.
>

Yeap, I see the changes on myfaces api 1.1 and 1.2 yesterday. There is a
probem with some tomahawk component that extends from UISelectItem and use
rendered property (obviously throw Unsupported....., but the solution is
very easy.


>
> Binding does indeed need to be excluded via
>  JSFJspProperty tagExcluded=true
> but I think that's a fine example of my proposed renaming of tagExcluded
> to excludeInheritedAttributeHack. If some components don't support
> binding (and it makes no sense for UIViewRoot) then binding should not
> be a feature defined on the base class from which UIViewRoot inherits.
> Ok, we need to support it, but we don't need to pretend it is correct.
>

Yes, I agree with you.


>
> >
> >
> >         === tagExcluded in macro files
> >
> >         I see tagExcluded is used in componentClass11.vm:
> >
> >         #if($utils.isPrimitiveClass($type) && !$property.tagExcluded)
> >         #set ($arrayIndex = $arrayIndex + 1)
> >                values[$arrayIndex] = ${field}Set;
> >         #end
> >
> >         Is this ${field}Set thing a Trinidad-specific feature?
> >
> >         Incidentally, I think there *might* be a bug here: the
> >         tagExcluded test
> >         is done here, but not at the point a dozen lines earlier when
> >         the size
> >         of the values array is being computed. But I'm not entirely
> >         clear what
> >         is happening here, so might be wrong.
> >
> > The code for this template right now is this:
> >
> > #if($utils.isPrimitiveClass($type) && !$property.isTagExcluded() )
> > #set ($arrayIndex = $arrayIndex + 1)
> >         ${field}Set = ((Boolean) values[$arrayIndex]).booleanValue();
> > #end
> >
> > It is used as an additional variable for primitive values. Example:
> >
> >     public void restoreState(FacesContext facesContext, Object state)
> >     {
> >         Object[] values = (Object[])state;
> >         super.restoreState(facesContext,values[0]);
> >         _actionFor = (java.lang.String) values[1];
> >         _disabled = ((Boolean) values[2]).booleanValue();
> >         _disabledSet = ((Boolean) values[3]).booleanValue();
> >
> > disable holds the value but disableSet check if the property has been
> > changed programatically. myfaces-faces-plugin do this generation, so
> > in myfaces-builder-plugin I follow this approach. Look this:
> >
> >     public boolean isDisabled()
> >     {
> >         if (_disabledSet)
> >         {
> >             return _disabled;
> >         }
> >         ValueBinding vb = getValueBinding("disabled");
> >         if (vb != null)
> >         {
> >             return ((Boolean)
> > vb.getValue(getFacesContext())).booleanValue();
> >         }
> >         return false;
> >     }
> >
> > Maybe it would be better to convert primitive types (boolean, int,
> > long) to the boxed counterparts (Boolean, Integer, Long) and use just
> > one variable (like it was before on myfaces 1.1 and tomahawk), but I'm
> > not applied this, because requires some changes on what we are doing
> > right now.
>
> Ok, this appears to be something that changed between Myfaces 1.1.x and
> 1.2.x. For example, UIMessage has _showDetail and _showSummary members.
>
> In 1.1.x, they are just kept as Boolean objects, and no converting is
> needed when saving or restoring state. Determining whether they are set
> is just a matter of testing them for null.
>
> In 1.2.x somebody has made them primitives, and added _showDetailSet,
> _showSummarySet flags too. Then stored these Set flags in the state too.
> I wonder why this was done; it doesn't make any sense to me at all. It
> appears to be a worse solution in all situations except where the
> property is being very frequently assigned to (and is not a boolean)
> which would require wrapper Objects to be created.
>
> Even if the set flag approach is kept on the component class, there is
> absolutely no reason why two objects need to be added to the state
> array; storing a null for the property state implies that set=false.
>
> So I would suggest updating the template to generate sane
> saveState/restoreState implementations, which will also simplify the
> template significantly.
>
> Did you understand my point about a possible bug here? When computing
> the array size, tagExcluded is checked But when iterating over the
> fields to insert into the state, tagExcluded is not checked. At least I
> think that is what is happening. So the numer of elements written into
> the array might not be the same as the size of the array.
>

There are two different situations:

1. The property is defined in the parent and the property is referred in the
child with tagExcluded = true. In this case inherited = true so
ComponentMeta.getPropertyComponentList exclude this property from
generation.

2. The property is defined by first time, so the code about this property
that goes in the component should be added. But this property should be
defined in this way

public String _property;

public String getProperty(){
    return _property;
}

public void setProperty(String property){
    _property = property;
}

Please note that no ValueBinding or ValueExpression is defined, because this
property is not retrieved from tld. This code is right, but the conditionals
make the code a little bit hard to understand (thats the reason of avoid at
max the conditionals here!).


>
> Regards,
> Simon
>
>
>
regards

Leonardo Uribe

Reply via email to