Mark Collette wrote:
> 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);
> }
> }
> }
This seems reasonable. Looking forward to your patch!
>
>
> - 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
>>
>>
>