Uploaded image for project: 'OFBiz'
  1. OFBiz
  2. OFBIZ-9306

Check there are no white spaces around the name of a form field when updating a form

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Implemented
    • Affects Version/s: Trunk
    • Fix Version/s: Upcoming Release
    • Component/s: ALL APPLICATIONS
    • Labels:
      None

      Description

      Sometimes ago, I lost an hour because of a space at the end of a field's name in a form. In creation it does not pass (the field lacks) but when updating the form it passes and it's hard to find from where comes the problem.

      1. OFBIZ-9306-ModelFormField.java.patch
        0.4 kB
        Jacques Le Roux
      2. OFBIZ-9306-ModelFormField.java.patch
        0.4 kB
        Jacques Le Roux

        Activity

        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Done at revision: 1805141

        Thanks Michael for review and spotting a possible NPE in my 1st patch

        Show
        jacques.le.roux Jacques Le Roux added a comment - Done at revision: 1805141 Thanks Michael for review and spotting a possible NPE in my 1st patch
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Michael,

        Oh right, a new pair of eyes was needed, I did not consider the code above. Here is a new patch.

        BTW I wonder what happens when we get null there. I did not check calling locations, I guess it's OK.

        Show
        jacques.le.roux Jacques Le Roux added a comment - Michael, Oh right, a new pair of eyes was needed, I did not consider the code above. Here is a new patch. BTW I wonder what happens when we get null there. I did not check calling locations, I guess it's OK.
        Hide
        mbrohl Michael Brohl added a comment -

        Jacques,

        if returnValue is null, you'll get a NullPointerException...

        Show
        mbrohl Michael Brohl added a comment - Jacques, if returnValue is null, you'll get a NullPointerException...
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        Thanks Michael,

        Why do you think introducing a check for null should be done now? Is it related to this change or in general (as it's now in trunk)?

        I'm not sure it's needed because this change is only removing white-spaces around the name. If the name is already empty I guess it's already handled with current code.

        This said I'm not against checking for null first, I just want to understand why and maybe where in code (I guess you mean just before the return, right?)

        Show
        jacques.le.roux Jacques Le Roux added a comment - Thanks Michael, Why do you think introducing a check for null should be done now? Is it related to this change or in general (as it's now in trunk)? I'm not sure it's needed because this change is only removing white-spaces around the name. If the name is already empty I guess it's already handled with current code. This said I'm not against checking for null first, I just want to understand why and maybe where in code (I guess you mean just before the return, right?)
        Hide
        mbrohl Michael Brohl added a comment -

        I agree with the trim, but you should check for null first.

        Show
        mbrohl Michael Brohl added a comment - I agree with the trim, but you should check for null first.
        Hide
        jacques.le.roux Jacques Le Roux added a comment -

        It seems to me that a name should not be preceded by a white-space. So this uses a trim() to removes all preceding or trailing white-spaces.

        I wait a week to see if people agree with this notion before committing...

        Show
        jacques.le.roux Jacques Le Roux added a comment - It seems to me that a name should not be preceded by a white-space. So this uses a trim() to removes all preceding or trailing white-spaces. I wait a week to see if people agree with this notion before committing...
        Hide
        pfm.smits Pierre Smits added a comment -

        Maybe a - generic - function that removes spaces helps.

        Show
        pfm.smits Pierre Smits added a comment - Maybe a - generic - function that removes spaces helps.

          People

          • Assignee:
            jacques.le.roux Jacques Le Roux
            Reporter:
            jacques.le.roux Jacques Le Roux
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development