MyFaces Core
  1. MyFaces Core
  2. MYFACES-2418

Implement h:selectManyXXX collectionType and hideNoSelectionOption

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.0-beta
    • Component/s: JSR-314
    • Labels:
      None
    1. myfaces-2418.patch
      21 kB
      Jakob Korherr
    2. myfaces-2418-tck-problem-part2.patch
      15 kB
      Jakob Korherr
    3. myfaces-2418-tck-problems-api.patch
      24 kB
      Jakob Korherr
    4. myfaces-2418-tck-problems-proposal.patch
      37 kB
      Jakob Korherr
    5. myfaces-2418-tck-problems-shared.patch
      28 kB
      Jakob Korherr

      Activity

      Leonardo Uribe created issue -
      Hide
      Jakob Korherr added a comment -

      Here is my patch for this issue. It solves both, the collectionType attribute (I already implemented a part of this functionality in MYFACES-2136, I just added some more Collection support in this patch) and the hideNoSelectionOption attribute.

      While implementing the hideNoSelectionOption attribute, I discovered two little bugs: when h:selectManyCheckbox or h:selectOneRadio render SelectItemGroups, the IDs of the checkbox/radio-elements are not unique and they close TD and TR elements, which they maybe never opened before (I sometimes got a warning from ResponseWriterImpl). The patch fixes this two bugs too.

      Show
      Jakob Korherr added a comment - Here is my patch for this issue. It solves both, the collectionType attribute (I already implemented a part of this functionality in MYFACES-2136 , I just added some more Collection support in this patch) and the hideNoSelectionOption attribute. While implementing the hideNoSelectionOption attribute, I discovered two little bugs: when h:selectManyCheckbox or h:selectOneRadio render SelectItemGroups, the IDs of the checkbox/radio-elements are not unique and they close TD and TR elements, which they maybe never opened before (I sometimes got a warning from ResponseWriterImpl). The patch fixes this two bugs too.
      Jakob Korherr made changes -
      Field Original Value New Value
      Attachment myfaces-2418.patch [ 12425911 ]
      Jakob Korherr made changes -
      Status Open [ 1 ] Patch Available [ 10002 ]
      Hide
      Leonardo Uribe added a comment -

      I applied the patch but still there are two problems. It seems the new algorithm described in UISelectMany spec javadoc for (... the Renderer for this component must perform the following logic for getConvertedValue().... ) do something not documented but very helpful.

      If you have a bean with a method like this:

      public Object getSomeFieldValue()

      { ..... }

      and you have something like

      <h:selectManyXXX value="#{bean.someFieldValue}"/>

      and when the view is rendered first this method returns null, no conversion is done. Myfaces throws an exception since the return value is not an array or collection.

      The second problem is when you have something like this

      public List<Integer> getSomeFieldValue(){ ..... }

      In this case, ri works but myfaces fails when try to convert the value. In other words, the algorithm should detect the type and use the proper converter retrieved from target class (in this case Integer).

      Since the documentation is poor in this case (and maybe wrong!), but the functionality is obviously wanted, on myfaces side we should do the same, testing different cases on ri and fixing the algorithm. Note this two cases are tested by TCK test.

      The only mention about the intention of this stuff is here:

      https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=230

      Show
      Leonardo Uribe added a comment - I applied the patch but still there are two problems. It seems the new algorithm described in UISelectMany spec javadoc for (... the Renderer for this component must perform the following logic for getConvertedValue().... ) do something not documented but very helpful. If you have a bean with a method like this: public Object getSomeFieldValue() { ..... } and you have something like <h:selectManyXXX value="#{bean.someFieldValue}"/> and when the view is rendered first this method returns null, no conversion is done. Myfaces throws an exception since the return value is not an array or collection. The second problem is when you have something like this public List<Integer> getSomeFieldValue(){ ..... } In this case, ri works but myfaces fails when try to convert the value. In other words, the algorithm should detect the type and use the proper converter retrieved from target class (in this case Integer). Since the documentation is poor in this case (and maybe wrong!), but the functionality is obviously wanted, on myfaces side we should do the same, testing different cases on ri and fixing the algorithm. Note this two cases are tested by TCK test. The only mention about the intention of this stuff is here: https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=230
      Hide
      Jakob Korherr added a comment -

      I tested these two scenarios an mojarra and I already have a solution for the first problem.

      The second problem is a bit strange. If you use something like this

      public List<Integer> getSomeFieldValue()

      { ..... }

      on mojarra and you also have SelectItems containing Integers, mojarra renders the SelectItems, while MyFaces already fails here, because it cannot get a Converter to convert the Integers to Strings for rendering. This is, of course, easy to solve (we just have to take a look at the value type of the SelectItems).

      However, if you submit your selections on mojarra it adds the submitted values, which are of type String, without any conversion to the ArrayList, which will be set in setSomeFieldValue(List<Integer> value). This works, because java does not check the type at runtime.
      But now we have the problem that when we want to access the values, we will get Strings instead of the expected Integers, which may cause «strange looking» ClassCastExceptions in the user code.

      Should we behave exactly like mojarra at this point, which on my opinion is stupid and may confuse a lot of developers, or should we try to obtain a Converter some other way?
      I think looking at the type of the SelectItems will provide us a valid converter and would be a better way.

      The spec, of course, does not mention anything about this.

      Show
      Jakob Korherr added a comment - I tested these two scenarios an mojarra and I already have a solution for the first problem. The second problem is a bit strange. If you use something like this public List<Integer> getSomeFieldValue() { ..... } on mojarra and you also have SelectItems containing Integers, mojarra renders the SelectItems, while MyFaces already fails here, because it cannot get a Converter to convert the Integers to Strings for rendering. This is, of course, easy to solve (we just have to take a look at the value type of the SelectItems). However, if you submit your selections on mojarra it adds the submitted values, which are of type String, without any conversion to the ArrayList, which will be set in setSomeFieldValue(List<Integer> value). This works, because java does not check the type at runtime. But now we have the problem that when we want to access the values, we will get Strings instead of the expected Integers, which may cause «strange looking» ClassCastExceptions in the user code. Should we behave exactly like mojarra at this point, which on my opinion is stupid and may confuse a lot of developers, or should we try to obtain a Converter some other way? I think looking at the type of the SelectItems will provide us a valid converter and would be a better way. The spec, of course, does not mention anything about this.
      Hide
      Jakob Korherr added a comment -

      Here is my patch, which solves these problems. The patch uses the values of the SelectItems of the UISelectMany to obtain a suitable Converter (as mentioned in my previous comment).

      Show
      Jakob Korherr added a comment - Here is my patch, which solves these problems. The patch uses the values of the SelectItems of the UISelectMany to obtain a suitable Converter (as mentioned in my previous comment).
      Jakob Korherr made changes -
      Hide
      Jakob Korherr added a comment -

      Testing some cases in mojarra and again in myfaces, mojarra does not fail if the itemValue attribute is not provided on f:selectItems when value does not contain pure SelectItems. Mojarra behaves like var and itemValue attributes were set in the following way: var="item" itemValue="#

      {item}

      ".

      This patch solves this particular problem.

      Show
      Jakob Korherr added a comment - Testing some cases in mojarra and again in myfaces, mojarra does not fail if the itemValue attribute is not provided on f:selectItems when value does not contain pure SelectItems. Mojarra behaves like var and itemValue attributes were set in the following way: var="item" itemValue="# {item} ". This patch solves this particular problem.
      Jakob Korherr made changes -
      Attachment myfaces-2418-tck-problem-part2.patch [ 12426250 ]
      Hide
      Jakob Korherr added a comment -

      I merged the patches and splitted them to the api part and the shared part.

      Show
      Jakob Korherr added a comment - I merged the patches and splitted them to the api part and the shared part.
      Jakob Korherr made changes -
      Attachment myfaces-2418-tck-problems-api.patch [ 12426252 ]
      Attachment myfaces-2418-tck-problems-shared.patch [ 12426253 ]
      Hide
      Leonardo Uribe added a comment -

      Thanks to Jakob Korherr for this patch

      Show
      Leonardo Uribe added a comment - Thanks to Jakob Korherr for this patch
      Leonardo Uribe made changes -
      Status Patch Available [ 10002 ] Resolved [ 5 ]
      Assignee Leonardo Uribe [ lu4242 ]
      Fix Version/s 2.0.0-alpha-2 [ 12314409 ]
      Resolution Fixed [ 1 ]
      Leonardo Uribe made changes -
      Fix Version/s 2.0.0-beta [ 12314537 ]
      Fix Version/s 2.0.0-alpha-2 [ 12314409 ]
      Leonardo Uribe made changes -
      Status Resolved [ 5 ] Closed [ 6 ]
      Transition Time In Source Status Execution Times Last Executer Last Execution Date
      Open Open Patch Available Patch Available
      2d 23h 57m 1 Jakob Korherr 24/Nov/09 00:00
      Patch Available Patch Available Resolved Resolved
      3d 57m 1 Leonardo Uribe 27/Nov/09 00:57
      Resolved Resolved Closed Closed
      161d 4m 1 Leonardo Uribe 07/May/10 02:01

        People

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

          Dates

          • Created:
            Updated:
            Resolved:

            Development