411437 - Clipping of "Free Text" fields when user enters more then 255 characters.

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[Alex Eiser]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_1; en-us) AppleWebKit/525.1+ (KHTML, like Gecko) Version/3.0.4 Safari/523.10.6
Build Identifier: Bugzilla 3.1.2

The user should be prevented, or warned when entering more then 255 characters into a free text field. If the user does enter more then that amount of data, the data will be clipped with out telling/warning the user.

Reproducible: Always

Steps to Reproduce:
1. Create an instance of Bugzilla
2. Add a Free Text custom field.
3. Create a new record
4. Enter 300 characters into the new custom field.
5. Save
6. Compare what you saved, and the value stored in the DB, and that displayed to the user when bug is loaded
Actual Results:  
Content of custom field is clipped to 255 characters.

Expected Results:  
 The user is prevented/warned that he has entered to much data.

Simple fix.. add a maxlength to the template code.

Comment 1

[Frédéric Buclin]

Requesting blocking as it may be seen as dataloss, especially if you paste some important data in it and don't check whether the string has been deleted or not.

Comment 2

[Frédéric Buclin]

Attachment: patch, v1

Comment 3

[Frédéric Buclin]

Attachment: patch, v1.1

Woah, I forgot to edit a line.

Comment 4

[Max Kanat-Alexander]

Comment on attachment 296153 [details] [diff] [review]
patch, v1.1

  This looks great.

>Index: Bugzilla/Constants.pm
> [snip]
>+    MAX_FREETEXT_LENGTH
>+

  Nit: Aren't there other MAX_?_LEN constants this could be grouped with?

  The reason for the r- is that the error needs to be added to Bugzilla::WebService::Constants, and possibly documented in the WebService POD, if it gets its own number. (I don't think it will need its own number, there's already some similar "invalid_field" error that it can share the number of.)

Comment 5

[Frédéric Buclin]

Attachment: patch, v2

Comment 6

[Max Kanat-Alexander]

Comment on attachment 296705 [details] [diff] [review]
patch, v2

Looks fine. We should actually re-use MAX_FREETEXT_LENGTH in the SQL_DEFINITION constant in Bugzilla::Field.

Also, the POD you changed needs a comma after "invalid value".

Comment 7

[Frédéric Buclin]

I moved _check_* methods in the "custom fields" section of Bug.pm as requested by Max on IRC. Leaving this bug open as I still have to backport it for Bugzilla 3.0.4.

tip:

Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.225; previous revision: 1.224
done
Checking in Bugzilla/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v  <--  Constants.pm
new revision: 1.87; previous revision: 1.86
done
Checking in Bugzilla/WebService/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Bug.pm,v  <--  Bug.pm
new revision: 1.9; previous revision: 1.8
done
Checking in Bugzilla/WebService/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Constants.pm,v  <--  Constants.pm
new revision: 1.13; previous revision: 1.12
done
Checking in template/en/default/bug/field.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/field.html.tmpl,v  <--  field.html.tmpl
new revision: 1.14; previous revision: 1.13
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v  <--  user-error.html.tmpl
new revision: 1.239; previous revision: 1.238
done

Comment 8

[Frédéric Buclin]

Attachment: patch for 3.0.x, v1

Here is the backport for 3.0.x.

Comment 9

[Frédéric Buclin]

Attachment: patch for 3.0.x, v2

Without _check_default_field() as Max doesn't like it.

Comment 10

[Max Kanat-Alexander]

Comment on attachment 296743 [details] [diff] [review]
patch for 3.0.x, v2

Yeah, looks fine.

Comment 11

[Frédéric Buclin]

3.0.3:

Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.351.2.7; previous revision: 1.351.2.6
done
Checking in Bugzilla/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Bug.pm,v  <--  Bug.pm
new revision: 1.171.2.3; previous revision: 1.171.2.2
done
Checking in Bugzilla/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Constants.pm,v  <--  Constants.pm
new revision: 1.68.2.13; previous revision: 1.68.2.12
done
Checking in Bugzilla/WebService/Bug.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Bug.pm,v  <--  Bug.pm
new revision: 1.4.2.3; previous revision: 1.4.2.2
done
Checking in Bugzilla/WebService/Constants.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/WebService/Constants.pm,v  <--  Constants.pm
new revision: 1.6.2.3; previous revision: 1.6.2.2
done
Checking in template/en/default/bug/field.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/field.html.tmpl,v  <--  field.html.tmpl
new revision: 1.8.2.1; previous revision: 1.8
done
Checking in template/en/default/global/user-error.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/global/user-error.html.tmpl,v  <--  user-error.html.tmpl
new revision: 1.204.2.9; previous revision: 1.204.2.8
done

Comment 12

[Mark Ruedy]

Does bug 411437 only pertain to particular browsers and only on Apple systems?  If so, then why limit all users to 255 character limit for freetext fields?

I have tested larger field data on Windows and Unix, using IE 7.0, FireFox 2.0.0.14 and 3.0 with no clipping issues.

Comment 13

[Alex Eiser]

I would need to do some testing, but the actual issue is the Database field def is varchar(255) the DB should not be able to save more then 255 characters.

Comment 14

[Mark Ruedy]

Ah, I see now that you are correct.  My CF field is a Mediumtext and I remember that this is because I needed to import existing data (and 255 is anyway too small for my purposes).  The problem came to a head when a Windows user was trying to save even though he had not changed the CF data.

Anyway, I guess now I will have to watch out for these Apple browsers.  Thanks.

Comment 15

[Alex Eiser]

I really don't think it has anything to do with the apple browser... we can talk about it on IM/IRC if you want.