MyFaces Core
  1. MyFaces Core
  2. MYFACES-2927

Invalid handling of null values in listboxes and menus

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.3
    • Component/s: General
    • Labels:
      None

      Description

      I discovered that MyFaces doesn't seem to handle null values correctly in listboxes and menus.

      See the following example showing a h:selectOneMenu used to select an enum value:

      @ManagedBean
      @RequestScoped
      public class TestBean {

      public enum Level

      { HIGH, MEDIUM, LOW }

      private Level level;

      public String action()

      { FacesContext.getCurrentInstance().addMessage(null, new FacesMessage("Submitted level: " + level)); return null; }

      }

      And the following view:

      <h:selectOneMenu value="#

      {testBean.level}

      " label="Level">
      <f:selectItem itemValue="#

      {null}

      " itemLabel="Unknown"/>
      <f:selectItem itemValue="HIGH" itemLabel="High"/>
      <f:selectItem itemValue="MEDIUM" itemLabel="Medium"/>
      <f:selectItem itemValue="LOW" itemLabel="Level"/>
      </h:selectOneMenu>

      <h:commandButton action="#

      {testBean.action}

      " value="Go"/>

      Clicking the "Go" button will result in the validation error: "Level: 'Unknown' must be convertible to an enum."

      If the value of the select item is null, MyFaces won't render the value attribute of the option and so the browser will submit the label as its value:

      <select id="levelForm:level" name="levelForm:level" size="1">
      <option selected="selected">Unknown</option>
      <option value="HIGH">High</option>
      <option value="MEDIUM">Medium</option>
      <option value="LOW">Level</option>
      </select>

      Mojarra instead renders an empty value attribute in this situation:

      <option value="" selected="selected">Unknown</option>

      I've created a patch fixing this issue against the current trunk of myfaces-shared and attached it to this ticket.

      You can see the sample application reproducing this issue here:

      http://github.com/chkal/myfaces-tests/tree/menu-null-value

      And download it here:

      http://github.com/chkal/myfaces-tests/archives/menu-null-value

      1. MYFACES-2927.patch
        0.8 kB
        Christian Kaltepoth

        Issue Links

          Activity

          Hide
          Christian Kaltepoth added a comment -

          Patch against myfaces-shared trunk

          Show
          Christian Kaltepoth added a comment - Patch against myfaces-shared trunk
          Hide
          Leonardo Uribe added a comment -

          I checked it and it is correct. Just one small note, to work correctly it requires:

          org.apache.myfaces.ENUM_CONVERTER_ALLOW_STRING_PASSTROUGH

          param set to true to work as expected (otherwise it will thrown an exception because the String is not a Enum value).

          I did a test on mojarra 2.0.3 and it seems there h:selectOneMenu does not call getAsString on the example, but in my opinion myfaces is doing right, always calling it and instead using a param to control this behavior.

          Thanks to Christian Kaltepoth for provide this patch.

          Show
          Leonardo Uribe added a comment - I checked it and it is correct. Just one small note, to work correctly it requires: org.apache.myfaces.ENUM_CONVERTER_ALLOW_STRING_PASSTROUGH param set to true to work as expected (otherwise it will thrown an exception because the String is not a Enum value). I did a test on mojarra 2.0.3 and it seems there h:selectOneMenu does not call getAsString on the example, but in my opinion myfaces is doing right, always calling it and instead using a param to control this behavior. Thanks to Christian Kaltepoth for provide this patch.
          Hide
          Christian Kaltepoth added a comment -

          Hi Leonardo,

          thank you very much for looking at this issue and applying my patch.

          Just for your information: I had a quick look at the Mojarra source and saw that they do not use a converter if the current value is already a String.

          See line 501 in HtmlBasicRenderer:

          http://java.net/projects/mojarra/sources/svn/content/trunk/jsf-ri/src/main/java/com/sun/faces/renderkit/html_basic/HtmlBasicRenderer.java

          In my opinion this behavior makes sense. Why should the renderer call Converter.getAsString() for a value that is already available in its String representation.

          Just my two cents!

          Show
          Christian Kaltepoth added a comment - Hi Leonardo, thank you very much for looking at this issue and applying my patch. Just for your information: I had a quick look at the Mojarra source and saw that they do not use a converter if the current value is already a String. See line 501 in HtmlBasicRenderer: http://java.net/projects/mojarra/sources/svn/content/trunk/jsf-ri/src/main/java/com/sun/faces/renderkit/html_basic/HtmlBasicRenderer.java In my opinion this behavior makes sense. Why should the renderer call Converter.getAsString() for a value that is already available in its String representation. Just my two cents!
          Hide
          Jakob Korherr added a comment -

          Hi Christian,

          Actually this is a Mojarra bug! A JSF impl should call converter.getAsString() even if the value is already a String, when a fitting converter is available.

          For more details see the description of the mojarra issue at http://java.net/jira/browse/JAVASERVERFACES-1694

          However, thanks a lot for your contribution

          Regards,
          Jakob

          Show
          Jakob Korherr added a comment - Hi Christian, Actually this is a Mojarra bug! A JSF impl should call converter.getAsString() even if the value is already a String, when a fitting converter is available. For more details see the description of the mojarra issue at http://java.net/jira/browse/JAVASERVERFACES-1694 However, thanks a lot for your contribution Regards, Jakob
          Hide
          Christian Kaltepoth added a comment -

          Hi Jakob,

          thank you very much for the clarification. I checked the issue and totally agree to your argumentation. Mojarra isn't spec compliant regarding this.

          Regards

          Christian

          Show
          Christian Kaltepoth added a comment - Hi Jakob, thank you very much for the clarification. I checked the issue and totally agree to your argumentation. Mojarra isn't spec compliant regarding this. Regards Christian

            People

            • Assignee:
              Leonardo Uribe
              Reporter:
              Christian Kaltepoth
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development