You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by ne...@ijet.com on 2010/10/27 17:30:59 UTC

NPE in Required Initialization for labels addon (property version)

I had to make a small change in the Required Initialization for labels (property version) to resolve a NullPointerException.
 
In DefaultRequiredLabelInitializer I added a null value check. Without, I was getting NullPointerExceptions.
 
protected void applyRequiredMarker(FacesContext facesContext, UIOutput uiOutput) 
{
    ValueExpression expression = uiOutput.getValueExpression("value");
 
        if (expression != null)
        {
            applyRequiredMarkerUsingExpression(facesContext, uiOutput, expression.getExpressionString());
        }
        else
        {
            String value = (String) uiOutput.getValue();
            if (value != null) {                
                applyRequiredMarkerUsingValue(facesContext, uiOutput, value);
            }
        }
        ...
}
 
Thanks!
Ben

Re: NPE in Required Initialization for labels addon (property version)

Posted by Gerhard <ge...@gmail.com>.
hi ben,

i've committed both - see [1]

regards,
gerhard

[1] http://code.google.com/p/os890/source/detail?r=378

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces


2010/10/28 <ne...@ijet.com>

>  Great, I see the change in the bean-validation version.
> Don't forget the property version ;)
>
> # This patch file was generated by NetBeans IDE
> # It uses platform neutral UTF-8 encoding and \n newlines.
> --- Base (BASE)
> +++ Locally Modified (Based On LOCAL)
> @@ -117,8 +117,11 @@
>
>          }
>          else
>          {
> +            String value = (String) uiOutput.getValue();
> +            if (value != null) {
>              applyRequiredMarkerUsingValue(facesContext, uiOutput, (String)
> uiOutput.getValue());
>          }
> +        }
>          if (setEscapeToFalse)
>          {
>              doSetEscapeToFalse(uiOutput);
> --Ben
>
> -----Original Message-----
> *From:* Gerhard [mailto:gerhard.petracek@gmail.com]
> *Sent:* Wednesday, October 27, 2010 6:32 PM
> *To:* MyFaces Development
> *Subject:* Re: NPE in Required Initialization for labels addon (property
> version)
>
> thx ben - i've committed it.
>
> regards,
> gerhard
>
> http://www.irian.at
>
> Your JSF powerhouse -
> JSF Consulting, Development and
> Courses in English and German
>
> Professional Support for Apache MyFaces
>
>
>
> 2010/10/27 <ne...@ijet.com>
>
>>  This null-check was also needed in the Bean Validation version.
>>
>> Thanks!
>> Ben
>>
>> -----Original Message-----
>> *From:* Ben Neuman
>> *Sent:* Wednesday, October 27, 2010 11:31 AM
>> *To:* 'MyFaces Development'
>> *Subject:* NPE in Required Initialization for labels addon (property
>> version)
>>
>> I had to make a small change in the Required Initialization for labels
>> (property version) to resolve a NullPointerException.
>>
>> In DefaultRequiredLabelInitializer I added a null value check. Without, I
>> was getting NullPointerExceptions.
>>
>> protected void applyRequiredMarker(FacesContext facesContext, UIOutput
>> uiOutput)
>> {
>>     ValueExpression expression = uiOutput.getValueExpression("value");
>>
>>         if (expression != null)
>>         {
>>             applyRequiredMarkerUsingExpression(facesContext, uiOutput,
>> expression.getExpressionString());
>>         }
>>         else
>>         {
>>             String value = (String) uiOutput.getValue();
>>             if (value != null) {
>>                 applyRequiredMarkerUsingValue(facesContext, uiOutput,
>> value);
>>             }
>>         }
>>         ...
>> }
>>
>> Thanks!
>> Ben
>>
>>
>

Re: Required Label Marker Placement

Posted by Rudy De Busscher <rd...@gmail.com>.
Ben,

We need some indication indeed if we have to put something on non required
fields also.  So I think you're solution would be OK (will see it in the
code)

regards
Rudy.

On 4 November 2010 14:47, <ne...@ijet.com> wrote:

>  I haven't disappeared. Just very busy with the paying job and life.
> I have made the changes to work for me and it looks great but have not had
> time to work with the unit tests.
>
> Just an FYI, this change did require that I pass a boolean 'required' flag
> along through the 'applyMarker's chain of methods, since we are marking both
> required and unrequired fields.
> I'm not aware of a better aproach at this time. Any thoughts?
>
> BEFORE_WITH_DUMMY sounds perfect.
>
> Thanks!
> Ben
>
>
> -----Original Message-----
> *From:* Rudy De Busscher [mailto:rdebusscher@gmail.com]
> *Sent:* Thursday, November 04, 2010 9:21 AM
> *To:* MyFaces Development
> *Subject:* Re: Required Label Marker Placement
>
> Hi,
>
> We should make it configurable and use BEFORE_WITH_DUMMY (or something)
> instead of BEFORE.
>
> I'll make the changes next week, or if you like, you can send us a patch
> that we can use.
>
> regards
> Rudy.
>
>
> On 28 October 2010 18:36, Gerhard <ge...@gmail.com> wrote:
>
>> +1
>> i'll review and apply the patch as soon as you have finished the mentioned
>> features.
>>
>> regards,
>> gerhard
>>
>> http://www.irian.at
>>
>> Your JSF powerhouse -
>> JSF Consulting, Development and
>> Courses in English and German
>>
>> Professional Support for Apache MyFaces
>>
>>
>>
>> 2010/10/28 <ne...@ijet.com>
>>
>>  Hi all.
>>> I've been playing with the required label addon and have found that a
>>> modification in marker rendering has
>>> improved the appearance of my app.
>>> I just want to get feedback from the rest as to whether this might be a
>>> change worth integrating into your module.
>>>
>>> Essentially, the current implementation simply prepends the marker to the
>>> label (when applying marker before value).
>>> Visually, I do not like my required marker displacing the label. I would
>>> prefer that all labels, required and otherwise, align vertically based on
>>> the label text, and that the required markers appear left of this vertical
>>> alignment.
>>> As so:
>>> *First Name:
>>> *Last Name:
>>>  Middle Name:
>>>
>>> In order to do this, I've prepended the required marker where necessary
>>> and a space where not necessary.
>>> I have also set the font of the 'marker' to monospace.
>>>
>>> Would this be a worthy addition to the addon? We could of course allow
>>> the developer to turn this off through some configuration param.
>>>
>>> Thanks!
>>> Ben Neuman
>>>
>>>
>>>
>>>
>>>
>>
>>
>

RE: Required Label Marker Placement

Posted by ne...@ijet.com.
I haven't disappeared. Just very busy with the paying job and life. 
I have made the changes to work for me and it looks great but have not had time to work with the unit tests. 
 
Just an FYI, this change did require that I pass a boolean 'required' flag along through the 'applyMarker's chain of methods, since we are marking both required and unrequired fields. 
I'm not aware of a better aproach at this time. Any thoughts?
 
BEFORE_WITH_DUMMY sounds perfect. 
 
Thanks!
Ben
 

-----Original Message-----
From: Rudy De Busscher [mailto:rdebusscher@gmail.com]
Sent: Thursday, November 04, 2010 9:21 AM
To: MyFaces Development
Subject: Re: Required Label Marker Placement


Hi,

We should make it configurable and use BEFORE_WITH_DUMMY (or something) instead of BEFORE.

I'll make the changes next week, or if you like, you can send us a patch that we can use.

regards
Rudy.



On 28 October 2010 18:36, Gerhard < gerhard.petracek@gmail.com> wrote:


+1 
i'll review and apply the patch as soon as you have finished the mentioned features.

regards,
gerhard

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces




2010/10/28 < neumanb@ijet.com> 


Hi all. 
I've been playing with the required label addon and have found that a modification in marker rendering has 
improved the appearance of my app. 
I just want to get feedback from the rest as to whether this might be a change worth integrating into your module.
 
Essentially, the current implementation simply prepends the marker to the label (when applying marker before value).
Visually, I do not like my required marker displacing the label. I would prefer that all labels, required and otherwise, align vertically based on the label text, and that the required markers appear left of this vertical alignment.
As so:
*First Name:
*Last Name:
 Middle Name:
 
In order to do this, I've prepended the required marker where necessary and a space where not necessary. 
I have also set the font of the 'marker' to monospace.
 
Would this be a worthy addition to the addon? We could of course allow the developer to turn this off through some configuration param.
 
Thanks!
Ben Neuman
 
 
 
 




Re: Required Label Marker Placement

Posted by Rudy De Busscher <rd...@gmail.com>.
Hi,

We should make it configurable and use BEFORE_WITH_DUMMY (or something)
instead of BEFORE.

I'll make the changes next week, or if you like, you can send us a patch
that we can use.

regards
Rudy.


On 28 October 2010 18:36, Gerhard <ge...@gmail.com> wrote:

> +1
> i'll review and apply the patch as soon as you have finished the mentioned
> features.
>
> regards,
> gerhard
>
> http://www.irian.at
>
> Your JSF powerhouse -
> JSF Consulting, Development and
> Courses in English and German
>
> Professional Support for Apache MyFaces
>
>
>
> 2010/10/28 <ne...@ijet.com>
>
>  Hi all.
>> I've been playing with the required label addon and have found that a
>> modification in marker rendering has
>> improved the appearance of my app.
>> I just want to get feedback from the rest as to whether this might be a
>> change worth integrating into your module.
>>
>> Essentially, the current implementation simply prepends the marker to the
>> label (when applying marker before value).
>> Visually, I do not like my required marker displacing the label. I would
>> prefer that all labels, required and otherwise, align vertically based on
>> the label text, and that the required markers appear left of this vertical
>> alignment.
>> As so:
>> *First Name:
>> *Last Name:
>>  Middle Name:
>>
>> In order to do this, I've prepended the required marker where necessary
>> and a space where not necessary.
>> I have also set the font of the 'marker' to monospace.
>>
>> Would this be a worthy addition to the addon? We could of course allow the
>> developer to turn this off through some configuration param.
>>
>> Thanks!
>> Ben Neuman
>>
>>
>>
>>
>>
>
>

Re: Required Label Marker Placement

Posted by Gerhard <ge...@gmail.com>.
+1
i'll review and apply the patch as soon as you have finished the mentioned
features.

regards,
gerhard

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces



2010/10/28 <ne...@ijet.com>

>  Hi all.
> I've been playing with the required label addon and have found that a
> modification in marker rendering has
> improved the appearance of my app.
> I just want to get feedback from the rest as to whether this might be a
> change worth integrating into your module.
>
> Essentially, the current implementation simply prepends the marker to the
> label (when applying marker before value).
> Visually, I do not like my required marker displacing the label. I would
> prefer that all labels, required and otherwise, align vertically based on
> the label text, and that the required markers appear left of this vertical
> alignment.
> As so:
> *First Name:
> *Last Name:
>  Middle Name:
>
> In order to do this, I've prepended the required marker where necessary and
> a space where not necessary.
> I have also set the font of the 'marker' to monospace.
>
> Would this be a worthy addition to the addon? We could of course allow the
> developer to turn this off through some configuration param.
>
> Thanks!
> Ben Neuman
>
>
>
>
>

Required Label Marker Placement

Posted by ne...@ijet.com.
Hi all. 
I've been playing with the required label addon and have found that a modification in marker rendering has 
improved the appearance of my app. 
I just want to get feedback from the rest as to whether this might be a change worth integrating into your module.
 
Essentially, the current implementation simply prepends the marker to the label (when applying marker before value).
Visually, I do not like my required marker displacing the label. I would prefer that all labels, required and otherwise, align vertically based on the label text, and that the required markers appear left of this vertical alignment.
As so:
*First Name:
*Last Name:
 Middle Name:
 
In order to do this, I've prepended the required marker where necessary and a space where not necessary. 
I have also set the font of the 'marker' to monospace.
 
Would this be a worthy addition to the addon? We could of course allow the developer to turn this off through some configuration param.
 
Thanks!
Ben Neuman
 
 
 
 

RE: NPE in Required Initialization for labels addon (property version)

Posted by ne...@ijet.com.
Great, I see the change in the bean-validation version. 
Don't forget the property version ;)
 
# This patch file was generated by NetBeans IDE
# It uses platform neutral UTF-8 encoding and \n newlines.
--- Base (BASE)
+++ Locally Modified (Based On LOCAL)
@@ -117,8 +117,11 @@
         }
         else
         {
+            String value = (String) uiOutput.getValue();
+            if (value != null) {
             applyRequiredMarkerUsingValue(facesContext, uiOutput, (String) uiOutput.getValue());
         }
+        }
         if (setEscapeToFalse)
         {
             doSetEscapeToFalse(uiOutput);

--Ben

-----Original Message-----
From: Gerhard [mailto:gerhard.petracek@gmail.com]
Sent: Wednesday, October 27, 2010 6:32 PM
To: MyFaces Development
Subject: Re: NPE in Required Initialization for labels addon (property version)


thx ben - i've committed it. 

regards,
gerhard

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces




2010/10/27 < neumanb@ijet.com>


This null-check was also needed in the Bean Validation version.
 
Thanks!
Ben

-----Original Message-----
From: Ben Neuman 
Sent: Wednesday, October 27, 2010 11:31 AM
To: 'MyFaces Development'
Subject: NPE in Required Initialization for labels addon (property version)


I had to make a small change in the Required Initialization for labels (property version) to resolve a NullPointerException.
 
In DefaultRequiredLabelInitializer I added a null value check. Without, I was getting NullPointerExceptions.
 
protected void applyRequiredMarker(FacesContext facesContext, UIOutput uiOutput) 
{
    ValueExpression expression = uiOutput.getValueExpression("value");
 
        if (expression != null)
        {
            applyRequiredMarkerUsingExpression(facesContext, uiOutput, expression.getExpressionString());
        }
        else
        {
            String value = (String) uiOutput.getValue();
            if (value != null) {                
                applyRequiredMarkerUsingValue(facesContext, uiOutput, value);
            }
        }
        ...
}
 
Thanks!
Ben



Re: NPE in Required Initialization for labels addon (property version)

Posted by Gerhard <ge...@gmail.com>.
thx ben - i've committed it.

regards,
gerhard

http://www.irian.at

Your JSF powerhouse -
JSF Consulting, Development and
Courses in English and German

Professional Support for Apache MyFaces



2010/10/27 <ne...@ijet.com>

>  This null-check was also needed in the Bean Validation version.
>
> Thanks!
> Ben
>
> -----Original Message-----
> *From:* Ben Neuman
> *Sent:* Wednesday, October 27, 2010 11:31 AM
> *To:* 'MyFaces Development'
> *Subject:* NPE in Required Initialization for labels addon (property
> version)
>
> I had to make a small change in the Required Initialization for labels
> (property version) to resolve a NullPointerException.
>
> In DefaultRequiredLabelInitializer I added a null value check. Without, I
> was getting NullPointerExceptions.
>
> protected void applyRequiredMarker(FacesContext facesContext, UIOutput
> uiOutput)
> {
>     ValueExpression expression = uiOutput.getValueExpression("value");
>
>         if (expression != null)
>         {
>             applyRequiredMarkerUsingExpression(facesContext, uiOutput,
> expression.getExpressionString());
>         }
>         else
>         {
>             String value = (String) uiOutput.getValue();
>             if (value != null) {
>                 applyRequiredMarkerUsingValue(facesContext, uiOutput,
> value);
>             }
>         }
>         ...
> }
>
> Thanks!
> Ben
>
>

RE: NPE in Required Initialization for labels addon (property version)

Posted by ne...@ijet.com.
This null-check was also needed in the Bean Validation version.
 
Thanks!
Ben

-----Original Message-----
From: Ben Neuman 
Sent: Wednesday, October 27, 2010 11:31 AM
To: 'MyFaces Development'
Subject: NPE in Required Initialization for labels addon (property version)


I had to make a small change in the Required Initialization for labels (property version) to resolve a NullPointerException.
 
In DefaultRequiredLabelInitializer I added a null value check. Without, I was getting NullPointerExceptions.
 
protected void applyRequiredMarker(FacesContext facesContext, UIOutput uiOutput) 
{
    ValueExpression expression = uiOutput.getValueExpression("value");
 
        if (expression != null)
        {
            applyRequiredMarkerUsingExpression(facesContext, uiOutput, expression.getExpressionString());
        }
        else
        {
            String value = (String) uiOutput.getValue();
            if (value != null) {                
                applyRequiredMarkerUsingValue(facesContext, uiOutput, value);
            }
        }
        ...
}
 
Thanks!
Ben