[V6] remove carriage return on field of text type

Bug #872251 reported by Vincent Renaville@camptocamp
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Odoo GTK Client (MOVED TO GITHUB)
Status tracked in Trunk
6.0
Fix Released
Medium
OpenERP Publisher's Warranty Team
Trunk
Fix Released
Medium
OpenERP sa GTK client R&D

Bug Description

Hello,

When you want to a carriage return in a field of text type, the carriage return was automatically removed from the text field when you saved it.
For my customer it's really annoying because my customer use it to make invoice layout on RML report.
The GTK Client mustn't do automatic modification on text field.

Thanks for your help,

Vincent

Thanks for your help,

Vincent

Tags: maintenance

Related branches

Changed in openobject-client:
assignee: nobody → OpenERP Publisher's Warranty Team (openerp-opw)
tags: added: maintenance
Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

Hi,

This mostly looks like a wishlist to me... Being able to type spaces or newlines at the end of input fields is a nice-to-have that conflicts with the often-requested feature of de-duplicating data caused by unnecessary trailing spaces.
There is no feature in OpenERP that requires adding spaces or newlines at the end of input data, so the only cases where this matters are, as you said, tricks you use to alter the layout of some document.

That being said, I think the fix for bug 559788 that caused this change was not correct, it should have been fixed by displaying the real database value to allow the user to edit/fix it. We should not strip trailing whitespace from fields.text, not when setting the value, nor when reading it. It may be useful to do it for fields.char because that helps avoid mistakes and duplicates, but fields of type "fields.text" are not used for checking duplicates usually, so it does not help.

Still, I'm not sure this qualifies as an OPW case, especially when there are so many workarounds for doing the same thing (i.e. adding a very small or invisible non-space character at the end of the field, etc.).

Maybe we could do this:
- for 6.0: revert the patch for bug 559788, as it's not good, and leave it like that
- for trunk: revert the patch and find a better way to solve bug 559788, e.g. avoid stripping field.text values on display

Changed in openobject-client:
importance: Undecided → Wishlist
status: New → Confirmed
Revision history for this message
Naresh(OpenERP) (nch-openerp) wrote :

Hello Olivier,

Thanks for your response and advice.

Indeed I agree with vincent and you "Client should not change any data".

I would also like to throw some light on this . first of all the fix for bug:559788 was on char field and not on text field. (in/widget/view/form_gtk/char.py). which strips the data of the char field when a user inputs data through the client.

Secondly: The problem for the bug is in field.py where the stripping is done before setting the value internally in the model of client.(this was actually a fix when demo data contains extra spaces,newline char etc indeed this will affect all the field type which has been inherited from char class(like the text field type).

Conclusion and Questions:
* What I prefer based on this three bugs(2 lp + demo data) that we should remove the strip function from fields.py and if the demo data contains any extra spaces that should be simply considered as a user error rather then a bug.
* I dont opt to revert the fix for 559788. (as its a fix for user enter values directly from client only for char field type)

I would like to revert the same from trunk too?

Correct me if I had missed something !

Regards

Revision history for this message
Somesh Khare (somesh.khare) wrote :

Hello Vincent,

As per as the comments by Oliver and Naresh over this bug, I have fixed this by removing the strip method from the fields.py.

Issue has been fixed into the lp:~openerp-dev/openobject-client/6.0-opw-18150-skh branch.

Revision ID: <email address hidden>
Revision Number: 1912

It will be merged soon into the stable after the review by our experts.

Thanks

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote : Re: [Bug 872251] Re: [V6] remove carriage return on field of text type

On 11/04/2011 10:35 AM, Naresh(OpenERP) wrote:
> I would also like to throw some light on this . first of all the fix for
> bug:559788 was on char field and not on text field.
> (in/widget/view/form_gtk/char.py). which strips the data of the char
> field when a user inputs data through the client.

Ah, good point, thanks! It seems sensible to strip whitespace at the end
of user input in char fields, but probably we should be careful that
this behavior is not inherited by other field types, e.g. text, where
this is not desired.

> Secondly: The problem for the bug is in field.py where the stripping is
> done before setting the value internally in the model of client.(this
> was actually a fix when demo data contains extra spaces,newline char etc
> indeed this will affect all the field type which has been inherited from
> char class(like the text field type).

Thanks for the additional analysis!

> Conclusion and Questions:
> * What I prefer based on this three bugs(2
> lp + demo data) that we should remove the strip function from
> fields.py and if the demo data contains any extra spaces that should
> be simply considered as a user error rather then a bug. * I dont opt
> to revert the fix for 559788. (as its a fix for user enter values
> directly from client only for char field type)
>
> I would like to revert the same from trunk too?

This approach sounds fine to me, based on your analysis above.

Basically, we should never strip whitespace from data coming from the
database, nor when typing data in textareas (fields.text).
The only case where stripping whitespace is desired is when manually
typing or copy/pasting a value in a simple textfield (probably just for
char, and perhaps integer/float fields), to avoid undesired duplicate
values.

Thanks!

Revision history for this message
Naresh(OpenERP) (nch-openerp) wrote :

Thanks for all your inputs !

It has been improved in trunk @1993 <email address hidden>

Regards

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.