dev@javaserverfaces.java.net

Re: Components opting into attributesThatAreSet feature

From: Mark Collette <mark.collette_at_icesoft.com>
Date: Wed, 18 Feb 2009 11:36:42 -0700

I've modified the Mojarra code that generates this method for all the
components, and am working on implementing this in my own codebase, so I
can test everything, before submitting a patch. One thing I ran into,
was a potential flaw in the original code, which was still present in my
recommended change. I thought I'd ask you guys about this to see what
you think, before submitting the patch.

Most of the time, when an application is using a component property, it
will either be via a ValueExpression, or from setting a constant value
from the .xhtml markup, or from calling the setter method via a
component binding. But it's possible to combine using a ValueExpression
with setting the property via a component binding, it's just that the
field value set in the setter method will mask the ValueExpression until
it is set back to null again. For example:

HtmlSelectOneRadio comp = ...;
comp.setValueExpression("styleClass", ...);
// comp.getStyleClass() will return the evaluation of the ValueExpression
comp.setStyleClass("someStyleClass");
// comp.getStyleClass() will return comp.styleClass, which is
"someStyleClass"
comp.setStyleClass(null);
// comp.getStyleClass() will return the evaluation of the ValueExpression

So, given that setting a local field null, doesn't necessarily unset the
property, if the ValueExpression has also been set, then there's a flaw
with the handleAttribute(String, Object) method:

    private void handleAttribute(String name, Object value) {
        List<String> setAttributes = (List<String>)
this.getAttributes().get("javax.faces.component.UIComponentBase.attributesThatAreSet");
        if (setAttributes == null) {
            String cname = this.getClass().getName();
            if (cname != null && cname.startsWith(OPTIMIZED_PACKAGE)) {
                setAttributes = new ArrayList<String>(6);
                
this.getAttributes().put("javax.faces.component.UIComponentBase.attributesThatAreSet",
setAttributes);
            }
        }
        if (setAttributes != null) {
            if (value == null) {
+ ValueExpression ve = getValueExpression(name);
+ if (ve == null) {
+ setAttributes.remove(name);
+ }
- setAttributes.remove(name);
            } else if (!setAttributes.contains(name)) {
                setAttributes.add(name);
            }
        }
    }


- Mark Collette


Ed Burns wrote:

>>>>>>On Tue, 10 Feb 2009 15:15:29 -0800, Ryan Lubke <Ryan.Lubke_at_Sun.COM> said:
>>>>>>
>>>>>>
>
>RL> Mark,
>RL> Would you mind opening an issue to track this enhancement? We're bogged
>RL> down with 2.0 work at the moment, so if you can spare time to create
>RL> a patch that we can apply locally we can get your request in all that
>RL> much sooner.
>
>Mark, thanks! We'd love this code!
>
>Ed
>
>
>