MyFaces Core
  1. MyFaces Core
  2. MYFACES-2356

HtmlRadioRendererBase calls converter method getAsString() not with the object, but with the string version when an validation error occured

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0-beta
    • Component/s: General
    • Labels:
      None

      Description

      If you have a <h:selectOneRadio> component with some SelectItems and you select an item, submit it and an validation error occurs, HtmlRadioRendererBase calls org.apache.myfaces.shared_impl.renderkit.RendererUtils.getConvertedStringValue() with the submitted value, which is not converted an therefore already the string version of the SelectItem. So the provided converter tries to convert the string version to the string version, which can end in strange behaviour.

      1. HtmlRadioRendererBase.patch
        1 kB
        Jakob Korherr
      2. MYFACES-2356-2.patch
        0.9 kB
        Michael Kurz
      3. MYFACES-2356-3.patch
        0.7 kB
        Leonardo Uribe

        Issue Links

          Activity

          Hide
          Jakob Korherr added a comment -

          The right thing here to do is to get the string value directly from RendererUtils.getStringValue(), like HtmlMenuRendererBase and HtmlListboxRendererBase do.

          Show
          Jakob Korherr added a comment - The right thing here to do is to get the string value directly from RendererUtils.getStringValue(), like HtmlMenuRendererBase and HtmlListboxRendererBase do.
          Hide
          Jakob Korherr added a comment -

          Committed my patch.

          Show
          Jakob Korherr added a comment - Committed my patch.
          Hide
          Michael Kurz added a comment -

          The commited patch throws an IllegalArgumentException in RendererUtils.getStringValue(FacesContext, UIComponent) for an empty <h:selectOneRadio/> is submitted. In this case the submitted value will be RendererUtils.NOTHING and not a string which is enforced in the method.

          The new patch adds an additional check of the submitted value for RendererUtils.NOTHING in RendererUtils.getStringValue(FacesContext, UIComponent).

          Anyway, this will fix the bug but I just saw this in the renderkit spec for jsf 2.0 (Decode Behavior for UISelectOne):

          [..] If the Map does not contain an entry, call setSubmittedValue() passing an empty String as the argument.

          Currently, RendererUtils.NOTHING is set in this case.

          Show
          Michael Kurz added a comment - The commited patch throws an IllegalArgumentException in RendererUtils.getStringValue(FacesContext, UIComponent) for an empty <h:selectOneRadio/> is submitted. In this case the submitted value will be RendererUtils.NOTHING and not a string which is enforced in the method. The new patch adds an additional check of the submitted value for RendererUtils.NOTHING in RendererUtils.getStringValue(FacesContext, UIComponent). Anyway, this will fix the bug but I just saw this in the renderkit spec for jsf 2.0 (Decode Behavior for UISelectOne): [..] If the Map does not contain an entry, call setSubmittedValue() passing an empty String as the argument. Currently, RendererUtils.NOTHING is set in this case.
          Hide
          Leonardo Uribe added a comment -

          Attached there is a patch implementing the new line added on renderkitdocs. Michael, could you confirm if this patch works as expected?

          Show
          Leonardo Uribe added a comment - Attached there is a patch implementing the new line added on renderkitdocs. Michael, could you confirm if this patch works as expected?
          Hide
          Jakob Korherr added a comment -

          I'm sorry that I don't have time to test this right now (I'm a little busy with university stuff), but please also check the behavior of UISelectMany components before the release, because they also use most of this code!

          Show
          Jakob Korherr added a comment - I'm sorry that I don't have time to test this right now (I'm a little busy with university stuff), but please also check the behavior of UISelectMany components before the release, because they also use most of this code!
          Hide
          Michael Kurz added a comment -

          Hi Leonardo, patch MYFACES-2356-3 works for me without problems. We could probably completely remove RendererUtils.NOTHING but preferably not before the beta release as this should not have any impact. I will create another issue and a patch.

          Show
          Michael Kurz added a comment - Hi Leonardo, patch MYFACES-2356 -3 works for me without problems. We could probably completely remove RendererUtils.NOTHING but preferably not before the beta release as this should not have any impact. I will create another issue and a patch.

            People

            • Assignee:
              Jakob Korherr
              Reporter:
              Jakob Korherr
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development