Wicket
  1. Wicket
  2. WICKET-3962

the wrong option in select components is rendered (Combination of reusing panels and defaultFormProcessing==false)

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.4.18, 1.5-RC5.1
    • Fix Version/s: 1.4.19
    • Component/s: wicket
    • Environment:
      mac os / windows / java 1.5 / java 1.6 / tomcat

      Description

      We are reusing a panel (let's say a PersonPanel, two instances). There is a forward / back ajax-navigation between the panels, which is implemented with component.replace. So the component path for the select of both panels are the same. The back-function is implemented with setDefaultFormProcessing(false). If now showing PersonPanel1 --> Forward --> PersonPanel2 --> Back --> PersonPanel1 --> Forward --> PersonPanel2, the selects of the panel 2 are showing the values of panel1.

      The Reason:
      In the 'Select.isSelected(...)' the getInputAsArray() is used (instead of the getRawInput()). So request parameters are overruling the raw input, if the component path is the same.

      Wicket-Code:

      if (hasRawInput()) {
      String[] paths = getInputAsArray();

      I think, instead of calling getInputAsArray() the getRawInput()(With a split for the MultiSelect-support) should be used.

      1. wicket-3962.zip
        11 kB
        Olaf Siefart
      2. select-option-uuid.patch
        5 kB
        Sven Meier
      3. Select.java
        7 kB
        Olaf Siefart
      4. PageScopedValue.patch
        11 kB
        Martin Grigorov

        Issue Links

          Activity

          Hide
          Martin Grigorov added a comment -

          Can you create a quickstart so we can play with it ?

          Show
          Martin Grigorov added a comment - Can you create a quickstart so we can play with it ?
          Hide
          Olaf Siefart added a comment -

          Quickstart Example for the bug

          Show
          Olaf Siefart added a comment - Quickstart Example for the bug
          Hide
          Sven Ludwig added a comment -

          Note that this issue is related to https://issues.apache.org/jira/browse/WICKET-2949 .

          Show
          Sven Ludwig added a comment - Note that this issue is related to https://issues.apache.org/jira/browse/WICKET-2949 .
          Hide
          Sven Meier added a comment -

          IMHO we should align Select/SelectOption with how CheckGroup/Check works, i.e. using a uuid as value.
          This would fix this bug too.

          See attached patch.

          Show
          Sven Meier added a comment - IMHO we should align Select/SelectOption with how CheckGroup/Check works, i.e. using a uuid as value. This would fix this bug too. See attached patch.
          Hide
          Martin Grigorov added a comment -

          +1 for the patch.

          I tried to encapsulate the logic for value generation but later I realized that it will take more bytes for each form component that uses PageScopedValue. So maybe it is not a good idea.

          Show
          Martin Grigorov added a comment - +1 for the patch. I tried to encapsulate the logic for value generation but later I realized that it will take more bytes for each form component that uses PageScopedValue. So maybe it is not a good idea.
          Hide
          Olaf Siefart added a comment -

          Is an uuid construct really necessary? Changing "String[] paths = getInputAsArray();" to "String[] paths = Strings.split(getRawInput(), ';');" solves the problem. I think, there is no reasonf for using the request params, if there is raw input in the component.

          Show
          Olaf Siefart added a comment - Is an uuid construct really necessary? Changing "String[] paths = getInputAsArray();" to "String[] paths = Strings.split(getRawInput(), ';');" solves the problem. I think, there is no reasonf for using the request params, if there is raw input in the component.
          Hide
          Sven Meier added a comment -

          I don't like the uuid too much either, but Check/CheckGroup and Option/OptionGroup do it this way (for whatever reason). Note that these components use getInputAsArray() too.

          Show
          Sven Meier added a comment - I don't like the uuid too much either, but Check/CheckGroup and Option/OptionGroup do it this way (for whatever reason). Note that these components use getInputAsArray() too.
          Hide
          Martin Grigorov added a comment -

          The uuid is there because of the scenario in WICKET-64.
          I think Select will suffer from the same problem in such scenario - the currently used relative path will break in a repeater.

          Show
          Martin Grigorov added a comment - The uuid is there because of the scenario in WICKET-64 . I think Select will suffer from the same problem in such scenario - the currently used relative path will break in a repeater.
          Hide
          Sven Meier added a comment -

          WICKET-64 ... thanks for the link. So uuid is the way to go ... are you going to commit PageScopedValue or do we want to cut down on a few bytes?

          Show
          Sven Meier added a comment - WICKET-64 ... thanks for the link. So uuid is the way to go ... are you going to commit PageScopedValue or do we want to cut down on a few bytes?
          Hide
          Martin Grigorov added a comment -

          Apply your patch. It can be improved anytime later.

          Show
          Martin Grigorov added a comment - Apply your patch. It can be improved anytime later.
          Hide
          Sven Meier added a comment -

          aligned Select with CheckGroup and RadioGroup, i.e. use uuid for values

          Show
          Sven Meier added a comment - aligned Select with CheckGroup and RadioGroup, i.e. use uuid for values

            People

            • Assignee:
              Sven Meier
              Reporter:
              Olaf Siefart
            • Votes:
              1 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 10m
                10m
                Remaining:
                Remaining Estimate - 10m
                10m
                Logged:
                Time Spent - Not Specified
                Not Specified

                  Development