155219 - Word wrap preference ignored for conversion to plain text (attachments or HTML compose, text send)

Status: NEW

(MailNews Core :: Composition, defect)

Description

[Bryce Mozilla Nesbitt]

The code exists to read the user's word wrap width preference:

       if (NS_SUCCEEDED(rv) && prefs) 
          prefs->GetIntPref("mailnews.wraplength", &width);
       // Let sanity reign!
       if (width < 10)
         width = 10;
       else if (width > 30000)
         width = 30000; 

But the calculated width is never passed anywhere.  In addition 30000 is a
very abitrary number, not supported by any of the RFC's relating to creating
plain text (text/plain) messages.

I have attached a patch that moves this code to a place where it can have
some effect, rationalizes the limits to match the RFC's, and allows
specification of 0.  All this works perfectly with our without format=flowed
text.  The existing issues with utf-8 are not affected.

Plese consider applying this patch.

Comment 1

[Bryce Mozilla Nesbitt]

Attachment: Moz 1.1a patch - fix wrap prefs - allow 0.

Comment 2

[Bryce Mozilla Nesbitt]

*** Bug 155220 has been marked as a duplicate of this bug. ***

Comment 3

[Bryce Mozilla Nesbitt]

*** Bug 155221 has been marked as a duplicate of this bug. ***

Comment 4

[Boris Zbarsky [:bzbarsky]]

benb, could you review this please?

Comment 5

[Ben Bucksch (:BenB)]

Not my area. But some obvious comments about the patch:
- Don't add |rv2|, reuse |rv|.
- Don't expose special-case values to the user. "Use 0 for no wrapping" is not
only bad programming, but completely escapes the ordinary user (probably
rightfully).
What would this do anyways? No wrapping? How would this work in the plaintext
and HTML composer? What, if the user writes lines longer than 1000 chars, but
has the pref to "no wrap"?

Comment 6

[Bryce Mozilla Nesbitt]

I started this patch with a desire to enable 'no wrapping'.  I traditioanlly
send plaintext messages as one line per paragraph, which works just fine.  The
recipient's client wraps the message to whatever their window width is, and the
replies don't have weird word wrapping.

One could argue that, with the advent of 'flowed' text, that this entire user
interface element could be removed.  But if it is there, it ought to work.  And
if  it is removed then 'flowed' should be enabled for as many encodings as possible.

Comment 7

[leon.zhang]

The bug is complained by so many peoples form feedback channel.
It should be fixed ASAP.

take it.

Comment 8

[leon.zhang]

I searched "wraplength" and other similar string in source codes,and 
feel very suprised that there is just one function(ProcessSignature) related to 
above searching result.

Have wrap plaintext feature disappeared?

Comment 9

[leon.zhang]

Someone want to delete "wrap plain text " from prefrence.

current feedback from users said :
"If word-wrap is selected in preferences, word wrap does not work in Netscape 7
mail messenger.

Same results if option de-selected. This used to work in 6.2.2 but now 7.0 seems
to have broken it. Lines continue on with no wrap on compose mail and also on
some received emails, there is no reason I can see why some wrap and other do not."

Comment 10

[leon.zhang]

newest trunk can do:  wrap word acorrding to the width of composer's window, and
cannot do wrap word according to the setting value from preference.

some versions netscape 7 can also do the latter.

Comment 11

[leon.zhang]

In your account settings, you should deselect "compose messages in htnl format",
then you will find the function in preference can works.

so this bug is invalid.

Comment 12

[David Peters]

> so this bug is invalid.

No, it is not since you can't send un-wrapped messages (even if they are plain text only AND encoded as plain text) if you for some reason don't like to disable the HTML composer in the account settings.

I don't know how to do this, but can someone pls. change the status of this bug to "unresolved", please?

David.P

Comment 13

[Boris Zbarsky [:bzbarsky]]

Indeed.

Comment 14

[Magnus Melin [:mkmelin]]

Isn't this WFM? I just tried changing it to 100, worked just fine (and i do use plain text composition).

Comment 15

[Bryce Mozilla Nesbitt]

This bug has plenty of dupes... it might be "WFY", but it's not "WFM".

Comment 16

[Phil Ringnalda (:philor)]

The confusion probably stems from the summary being flat-out untrue (whether or not it was true in 2002), and comment 0 not actually saying anything about what cases it wants to catch, or what steps to hit them. If you're composing plain text, then we set the wrap width on the editor, and that works, but whatever hits that "convert an attachment to text plain" case in the patch doesn't, and according to comment 12 HTML compose, plain text send doesn't.

Comment 17

[Phil Ringnalda (:philor)]

As long as we're changing the summary, let's go all the way.

And those steps to reproduce, as many of the ways to hit it as you know, would also be a big help.

Comment 19

[Mike Cowperthwaite]

STR:
1) Set prefs:  mailnews.send_plaintext_flowed = false
               mailnews.wraplength            = 0
2) Compose HTML message with a long line
3) Send (or Send Later, just examine the Outbox contents), being sure not to send "as HTML only"
4) View Source of sent message

Actual results:
Plain text version of message is wrapped at 72

Expected results:
Plain text version of message doesn't wrap until RFC-imposed limit (990?)


I don't know what is meant by "converting attachments to plain text" but generally I would expect any attachment to be sent losslessly -- altho, conversion to quoted-printable is fine in that case, so long as it can be saved to disk as it was originally sent (modulo line-endings for plain text).

Comment 20

[:aceman]

There are several fetches of mailnews.wraplength today. These are the occurrences:
mailnews$ grep -i -r 'wraplength' *
base/util/nsMsgUtils.cpp:    pPrefBranch->GetIntPref("mailnews.wraplength", &wrapWidth);
compose/src/nsMsgCompose.cpp:nsresult nsMsgCompose::GetWrapLength(PRInt32 *aWrapLength)
compose/src/nsMsgCompose.cpp:  return prefBranch->GetIntPref("mailnews.wraplength", aWrapLength);
compose/src/nsMsgCompose.cpp:  PRInt32 wrapLength = 72; // setup default value in case GetWrapLength failed
compose/src/nsMsgCompose.cpp:  GetWrapLength(&wrapLength);
compose/src/nsMsgCompose.cpp:  GetWrapLength(&wrapping_enabled);
compose/src/nsMsgAttachmentHandler.cpp:      pPrefBranch->GetIntPref("mailnews.wraplength", &width);
mailnews.js:pref("mailnews.wraplength",                 72);

The problem is, there are different "sanity" ranges applied to this value (max at 990 or 3000) and some of the places treat 0 value as unlimited (wrapping off), some convert 0 to 72, some convert 0 to 990.

I don't know if that is intentional. I could try to consolidate this if there is a decision on how this sanity mangling should work.

On the other hand, it seems the original patch here was applied to the source (maybe by other bug). So is this bug still existent today?

Comment 21

[:aceman]

I added dependence on bug 61831 because it changes one spot in nsMessenger.cpp that was using hardcoded wrap at 72 chars. Now it obeys the mailnews.wraplength pref. Maybe it will help comment 19?

Comment 22

[:aceman]

Maybe the backend/composition owners will know what needs to happen here.

Comment 23

[David :Bienvenu]

I think things have changed somewhat since this bug was filed - I'm not sure this is a big deal; we don't get a lot of complaints about it but if someone wants to advocate for changing our behavior, I'm listening.

Comment 24

[:aceman]

OK. I have made an offer in comment 20 to consolidate the various manglings just for consistency. However I am not personally hit by this bug. So the original reporters (counting the dupes) must step in if any of the reported problems are still existing today.

Comment 25

[Wayne Mery (:wsmwk)]

(In reply to :aceman from comment #24)

OK. I have made an offer in comment 20 to consolidate the various manglings just for consistency. However I am not personally hit by this bug. So the
original reporters (counting the dupes) must step in if any of the reported problems are still existing today.

I doubt any of them are still around (maybe Bryce, Przemyslaw at most) and the blocking Bug 777205 - Should remove unused wrap length value in nsMsgAttachmentHandler::UrlExit - has since been fixed.

So do you think there is aything left to do here?