MyFaces Trinidad
  1. MyFaces Trinidad
  2. TRINIDAD-930

tr:selectOneChoice resets value if not in form

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 1.2.6-core
    • Fix Version/s: 1.2.10-core, 1.0.10-core
    • Component/s: Components
    • Labels:
      None
    • Environment:
      JSF RI 1.2, Trinidad 1.2 trunk

      Description

      If tr:selectOneChoice is outside a h:form, it resets value used in value="#{}". Test case:

      JSP snippet:

      <tr:selectOneChoice value="#

      {testBean.property}

      " unselectedLabel="(not selected)">
      <f:selectItem itemValue="value1" itemLabel="Item 1"/>
      </tr:selectOneChoice>

      <h:form> <!-- tr:selectOneChoice is not in the form -->
      <h:commandButton value="Ok" />
      </h:form>

      managed bean in session scope:

      public class TestBean {

      private String property = "value1";

      public String getProperty()

      { return property; }

      public void setProperty(String property)

      { this.property = property; }

      }

      Steps to reproduce problem:
      1) Display jsp page - it shows "Item 1" in combobox
      2) Click on "Ok" button
      3) Value in combobox will change to "(not selected)"

        Activity

        Hide
        Martin Kočí added a comment -

        Suggested patch - removes method SimpleSelectOneRenderer.getSubmittedValue(FacesContext, UIComponent, String) containing code:

        Object submittedValue = super.getSubmittedValue(context,
        component,
        clientId);
        if (submittedValue == null)
        submittedValue = "";

        But in UIXEditableValue.validate(FacesContext) we can read:

        // Submitted value == null means "the component was not submitted
        // at all"; validation should not continue

        Show
        Martin Kočí added a comment - Suggested patch - removes method SimpleSelectOneRenderer.getSubmittedValue(FacesContext, UIComponent, String) containing code: Object submittedValue = super.getSubmittedValue(context, component, clientId); if (submittedValue == null) submittedValue = ""; But in UIXEditableValue.validate(FacesContext) we can read: // Submitted value == null means "the component was not submitted // at all"; validation should not continue
        Hide
        Scott O'Bryan added a comment -

        I'm not sure that this is a valid issue and the assumed contract on UIXEditableValue.validate might break as a result of this. On the other hand this may allow us to better handle multiple forms as well. I would open up a discussion on the dev list about this patch and see if others have any input.

        Show
        Scott O'Bryan added a comment - I'm not sure that this is a valid issue and the assumed contract on UIXEditableValue.validate might break as a result of this. On the other hand this may allow us to better handle multiple forms as well. I would open up a discussion on the dev list about this patch and see if others have any input.
        Hide
        Martin Kočí added a comment -

        I think this is unambiguously a bug:
        1) No other component/renderer has that behaviour
        2) If you look in getSubmittedValue implementations in trinidad, only SimpleSelectOneRenderer contains that strange code
        3) Why SimpleSelectOneRenderer contains algorithm saying: this component was not sumbitted, but indicate that it was - without any explanation.
        4) From user view: no JSF user expects that component modifies backing bean values although it was not in form (was not a part of a user interaction)
        5) And last but not least: we use trinidad in production with this patch applied without any unexpected results. Without this patch users reported data lost/unexpected modifications.

        Show
        Martin Kočí added a comment - I think this is unambiguously a bug: 1) No other component/renderer has that behaviour 2) If you look in getSubmittedValue implementations in trinidad, only SimpleSelectOneRenderer contains that strange code 3) Why SimpleSelectOneRenderer contains algorithm saying: this component was not sumbitted, but indicate that it was - without any explanation. 4) From user view: no JSF user expects that component modifies backing bean values although it was not in form (was not a part of a user interaction) 5) And last but not least: we use trinidad in production with this patch applied without any unexpected results. Without this patch users reported data lost/unexpected modifications.
        Hide
        Andrew Robinson added a comment -

        -1 on applying the patch. Form controls should not be used outside of forms

        Show
        Andrew Robinson added a comment - -1 on applying the patch. Form controls should not be used outside of forms
        Hide
        Scott O'Bryan added a comment -

        Point #5 is invalid because although it may work for you, that doesn't mean it will work for everyone. Andrew put in a -1 which means this, at the very least, warrants a discussion and a vote.

        Do you want to open a thread or should I? A binding -1 vote is enough to make this patch not go in so you'll probably need to make a better case. Right now I'm -1 to THIS patch for the reasons both I (and Andrew) have stated.

        Show
        Scott O'Bryan added a comment - Point #5 is invalid because although it may work for you, that doesn't mean it will work for everyone. Andrew put in a -1 which means this, at the very least, warrants a discussion and a vote. Do you want to open a thread or should I? A binding -1 vote is enough to make this patch not go in so you'll probably need to make a better case. Right now I'm -1 to THIS patch for the reasons both I (and Andrew) have stated.

          People

          • Assignee:
            Matthias Weßendorf
            Reporter:
            Martin Kočí
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development