Tapestry
  1. Tapestry
  2. TAPESTRY-2085

When a user submit a form with a TextField and the value is missing or blank, the value null is passed through the component to the model property

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 5.0.10
    • Fix Version/s: 5.0.10
    • Component/s: None
    • Labels:
      None

      Description

      This was initially reported on the dev list by Geoff Callender.

      When submitting a form containing a textfield and nothing has been typed into that textfield, the value will be submitted as null rather than the empty string. In past version, the empty string would be submitted.

      The net result is NPEs and coercion issues. If the textfield is bound to a String and that String is used without checking for null, an NPE will result when none used to. If the textfield is bound to any other type, such as an Integer, the Tapestry type coercer will fail with an appropriate exception (e.g., NumberFormatException).

        Activity

        Hide
        Geoff Callender added a comment -

        From the mailing list, Howard laid out some thoughts and a workaround:

        The difference is that blank fields used to show up as the empty
        string and are now showing up in the properties as null.

        This could be addressed by a different NullFieldStrategy. Perhaps
        Tapestry should detect that the field is of type String and use
        NullFieldStrategy that converts blank fields to the empty string.

        In any case, a workaround is:

        Object onSuccess()

        { if (_firstName == null) _firstName = ""; if (_lastName == null) _lastName = ""; _nextPage.onActivate(_firstName, _lastName); return _nextPage; }
        Show
        Geoff Callender added a comment - From the mailing list, Howard laid out some thoughts and a workaround: The difference is that blank fields used to show up as the empty string and are now showing up in the properties as null. This could be addressed by a different NullFieldStrategy. Perhaps Tapestry should detect that the field is of type String and use NullFieldStrategy that converts blank fields to the empty string. In any case, a workaround is: Object onSuccess() { if (_firstName == null) _firstName = ""; if (_lastName == null) _lastName = ""; _nextPage.onActivate(_firstName, _lastName); return _nextPage; }
        Hide
        Geoff Callender added a comment -

        In case this affects the choice of solution, note that the workaround created new problems for the next page, called basicinput2...

        The URL it created for the next oage is
        http://localhost:8180/jumpstart/examples/input/basicinput2//
        instead of
        http://localhost:8180/jumpstart/examples/input/basicinput2/null/null

        The side effects are:

        1. The PageLinks on the page are no longer resolvable. eg.
        <a t:type="pagelink" t:page="Start" href="#">To Start</a>
        is interpreted as
        http://localhost:8180/jumpstart/examples/input/start
        instead of
        http://localhost:8180/jumpstart/start

        2. The default stylesheet isn't loaded, presumable because its URL is generated incorrectly.

        Show
        Geoff Callender added a comment - In case this affects the choice of solution, note that the workaround created new problems for the next page, called basicinput2... The URL it created for the next oage is http://localhost:8180/jumpstart/examples/input/basicinput2// instead of http://localhost:8180/jumpstart/examples/input/basicinput2/null/null The side effects are: 1. The PageLinks on the page are no longer resolvable. eg. <a t:type="pagelink" t:page="Start" href="#">To Start</a> is interpreted as http://localhost:8180/jumpstart/examples/input/start instead of http://localhost:8180/jumpstart/start 2. The default stylesheet isn't loaded, presumable because its URL is generated incorrectly.
        Hide
        Vjeran Marcinko added a comment -

        Value 0 for Integer and value "" for String is still A VALUE, and that is different for something having NO VALUE. Java language normally uses "null" for this purpose. Same as I want in database to have column with SQL value NULL when it is not set. Unfortunately, primitives cannot be used that way so some value has to be picked to represent no-value.
        Of course, you always have to have additional check with value != null in the code before using value, but I consider that to be normal.

        My thought is that it would be best for TextFields and TextArea to have optional parameter flag that would return empty string instead of null if this parameter is set.

        Show
        Vjeran Marcinko added a comment - Value 0 for Integer and value "" for String is still A VALUE, and that is different for something having NO VALUE. Java language normally uses "null" for this purpose. Same as I want in database to have column with SQL value NULL when it is not set. Unfortunately, primitives cannot be used that way so some value has to be picked to represent no-value. Of course, you always have to have additional check with value != null in the code before using value, but I consider that to be normal. My thought is that it would be best for TextFields and TextArea to have optional parameter flag that would return empty string instead of null if this parameter is set.
        Hide
        Kevin Menard added a comment -

        I'm not really sure what point you're arguing. The core of the matter is things that used to work no longer do and there's no trivial way to get around that.

        As for null versus empty . . . I guess it's a matter of taste, but the value POST'd is not null. There's a translation going on there and it's not symmetric.

        I guess at the very least, the expectation is that you're given a value. I think this is consistent with nearly all other Web frameworks in all other languages. Of course, I haven't tried them all, but I'm drawing from the experience I've had with a decent number of others. Call it an application of the Null Object pattern if you may.

        Anyway, it sounds like Howard has a decent idea of how this ought to be addressed, although, based on the varying opinions of correct behavior, it likely isn't going to be a fun one.

        Show
        Kevin Menard added a comment - I'm not really sure what point you're arguing. The core of the matter is things that used to work no longer do and there's no trivial way to get around that. As for null versus empty . . . I guess it's a matter of taste, but the value POST'd is not null. There's a translation going on there and it's not symmetric. I guess at the very least, the expectation is that you're given a value. I think this is consistent with nearly all other Web frameworks in all other languages. Of course, I haven't tried them all, but I'm drawing from the experience I've had with a decent number of others. Call it an application of the Null Object pattern if you may. Anyway, it sounds like Howard has a decent idea of how this ought to be addressed, although, based on the varying opinions of correct behavior, it likely isn't going to be a fun one.
        Hide
        Vjeran Marcinko added a comment -

        Ah, sorry for complaint... It seems I missed the point

        I understand that some things that worked before don't work anymore, and they should definetly be resolved somehow, and that's the issue here. Thing is that I am afraid that issues will be resolved in a way that would lead to just setting empty string values to bound property like it was before, and I think a lot of users would not prefer that way. If I remember correctly, T3 has been setting empty values and T4 has been doing it with null values. I had definetly prefered the later.

        Show
        Vjeran Marcinko added a comment - Ah, sorry for complaint... It seems I missed the point I understand that some things that worked before don't work anymore, and they should definetly be resolved somehow, and that's the issue here. Thing is that I am afraid that issues will be resolved in a way that would lead to just setting empty string values to bound property like it was before, and I think a lot of users would not prefer that way. If I remember correctly, T3 has been setting empty values and T4 has been doing it with null values. I had definetly prefered the later.
        Hide
        Howard M. Lewis Ship added a comment -

        One problem with the examples from the mailing list is the encoding of a blank string into the URL; this shows up as two adjacent slashes and that's just asking for trouble. This will not be allowed.

        Show
        Howard M. Lewis Ship added a comment - One problem with the examples from the mailing list is the encoding of a blank string into the URL; this shows up as two adjacent slashes and that's just asking for trouble. This will not be allowed.
        Hide
        Joel Wiegman added a comment -

        FWIW, using 5.0.10 I have a TextField bound to a java.lang.Integer and I can't get it to pass validation (my only validation requirement on the field is t:validate="maxlength=2").

        I get the validation error: "The input value '' is not parseable as an integer value."

        I'm using the DefaultNullStrategy (because I want it to be null if an empty TextField is submitted). When I add nulls="zero" to the TextField I have a different problem:

        Render queue error in BeginRender[app/controllist/consignment/EditControl:departmentid]: java.lang.Long cannot be cast to java.lang.Integer

        Show
        Joel Wiegman added a comment - FWIW, using 5.0.10 I have a TextField bound to a java.lang.Integer and I can't get it to pass validation (my only validation requirement on the field is t:validate="maxlength=2"). I get the validation error: "The input value '' is not parseable as an integer value." I'm using the DefaultNullStrategy (because I want it to be null if an empty TextField is submitted). When I add nulls="zero" to the TextField I have a different problem: Render queue error in BeginRender [app/controllist/consignment/EditControl:departmentid] : java.lang.Long cannot be cast to java.lang.Integer
        Hide
        Howard M. Lewis Ship added a comment -

        I think this is finally nailed.

        Show
        Howard M. Lewis Ship added a comment - I think this is finally nailed.

          People

          • Assignee:
            Howard M. Lewis Ship
            Reporter:
            Kevin Menard
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development