Wrap column ignored for messages composed as HTML, sent as Plain

RESOLVED FIXED in Thunderbird 3.1a1

Status

MailNews Core
Composition
--
minor
RESOLVED FIXED
14 years ago
9 years ago

People

(Reporter: sturges, Assigned: Philip Prindeville)

Tracking

Trunk
Thunderbird 3.1a1
x86
Windows XP

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patchlove][has patch])

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

14 years ago
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a6)
Gecko/20041210 and prior versions.

As reported in associated bugs 155622 and 274168 there are problems with the
wrap text.

I have discovered that Mozilla lies about the formatting of the emails that are
composed using the Html editor and sent as "Send as Plain Text Only
(Recommended)", this is why I submit a new bug.

What actually happens is Mozilla inserts line breaks after about 72 characters
in the outgoing email regardless of any settings [see tests below]. 

When previewing the sent emails, or opening them, Mozilla pretends it hasn't
inserted these line breaks but if you do a CTL+U you get the real story.

After composing an email with the desired line breaks in the correct places
Mozilla inserts so many line breaks that it makes a nonsense of reading the text
at the other end.

Even Netscape 4.7x did not have this problem.


Tests performed:
From an email composition window do Edit | Preferences | Mail & Newsgroups |
Composition;
Using the setting "Wrap plain test messages at 0 characters" and "Wrap plain
test messages at 1000 characters" - the result is the same.


Bugs:

1) Mozilla inserts uncontrollable line breaks in outgoing emails.
2) Mozilla renders emails differently from what is actually received or sent.

Comment 1

14 years ago
(In reply to comment #0)
> 1) Mozilla inserts uncontrollable line breaks in outgoing emails.

True enough; and I couldn't find a dupe of this.

As usual with bugs of this nature, tho, I wonder: why do you care what the wrap 
column is?  Are you lookng to have no newlines inserted unless you type them?
In that case, you need to compose as plain text and set a wide wrap column -- 
OR, format the lines as <pre> blocks in the HTML editor.

xref bug 125928.


> 2) Mozilla renders emails differently from what is actually received or sent.

This is the expected result of format=flowed -- it allows the plain text mail to 
be reflowed to match the window width.  See bug 168420 for information about 
f=f, including the informative FAQ attached to that bug.
Severity: normal → minor
Status: UNCONFIRMED → NEW
Component: MailNews: Main Mail Window → MailNews: Composition
Ever confirmed: true
Product: Mozilla Application Suite → Core
Summary: Wrap text for non HTML emails not working at all → Wrap column ignored for messages composed as HTML, sent as Plain
Version: 1.7 Branch → Trunk
(Reporter)

Comment 2

14 years ago
Mike Cowperthwaite - 

1) It is a bug; It should be possible to tell Mozilla how to format text it
sends and how to display it when incoming or what was sent. Even Netscape 4.7x
was better at this. Currently the user has no control over what Mozilla does.

If the text is written with paragraphs in the right places we don't need Mozilla
to insert line breaks all over the text but there is no way of stopping it doing
that.

The plain text editor is unusable in Mozilla 1.x, a) because it doesn't do line
wrap properly when composing [one can end up with a long sausage of text
scrolling across the screen]* and b) one cannot increase the font size [or zoom]
to facilate readability when people with poor sight or large screens are writing
a message, c) basically the plain text editor is just painful.

*Since version 1.8a you can now specify a wrap line length of 0. This makes the
plain text editor work correctly for line wrapping but because of b) above it is
still unusable.

Whats more of a problem is that the "recommended" HTML to plain text conversion
has its own uncontrollable rules, nothing to do with the settings for the plain
text editor. 

As nearly every browser has line wrap capability there is no reason AT ALL for
Mozilla to insert ANY additional line breaks during the conversion from HTML to
plain text. Why this setting is the unmodifyable default is just puzzling as it
serves nobody and only makes text less readable by the recipient. 


For point 2) - Most of my tests for the above outgoing problem were faulty
because they were masked by how Mozilla displays the incoming text so it was not
possible to see what was really happening.

I don't see why it is impossible to control how Mozilla displays the text it
receives [or has sent] in its real layout. Nowadays there is very little reason
for the system to be so "smart" that it just makes the situation worse.

The better option would be to display the text as received, wrapping the text at
the screen boundary, [line wrap] if the user requests it. Any other
interpretations should be selectable by the user, only if they need it and not
enforced.

The current position takes all control away from the user which makes the
product very difficult to recommend to others.

How do we get this situation resolved - it should be fairly easy to fix to
everyone's benefit?

Comment 3

14 years ago
(In reply to comment #2)
> 1) It is a bug; 

I didn't say it wasn't; in fact, I confirmed the bug.


> As nearly every browser has line wrap capability there is no reason AT ALL for
> Mozilla to insert ANY additional line breaks during the conversion from HTML
> to plain text. 

This is the core of the issue: you assume that everyone is reading mail on a web 
browser, which is not true.  Some people are still using fairly primitive text-
based email clients (this is why the "send as plain text" option is recommended) 
which may wrap long lines by breaking words into pieces at the 80-character 
boundary; this is harder to read than text broken at a word boundary in the 
middle of a line.

There is another very good reason for Mozilla to insert line breaks: because 
SMTP does not support lines over 1000 characters:
  http://rfc.net/rfc2821.html#s4.5.3.1
We do not send lines of arbitrary length; a newline is inserted at 990 
characters so as not to override that limit.  Unfortunately, this will also 
break words in the middle (bug 169395, bug 277185).


> I don't see why it is impossible to control how Mozilla displays the text it
> receives [or has sent] in its real layout.
> Nowadays there is very little reason for the system to be so "smart" that it
> just makes the situation worse.

Oddly, the people who tend to agree about format=flowed being a bad thing are 
those who want to use primitive software, where the receiving mail client cannot 
support it.  Not unlike your recipients' webmail software, which *should* be 
modern enough to handle incoming mail sent as f=f; format=flowed is a widely 
used standard and should be supported just as MIME and quoted-printable are 
supported.  (Even Pine supports it now.)

If the people you are sending to are all using webmail, then your best course of 
action for now is to enter them all into your address book set to "prefers HTML" 
Then the mail will go out as HTML (without bothering you with the prompt), and 
they won't see the ad-hoc line breaks mid-paragraph.


> How do we get this situation resolved - it should be fairly easy to fix to
> everyone's benefit?

You do not understand the development model here.  Unless this bug (and here, 
I'm referring *only* to the issue of the specified plain-text wrap column being 
ignored on HTML->plain conversion) catches the interest of a developer, it's not 
going to be fixed.  Not for lack of desire, but for lack of resources.  See, for 
instance, bug 125928, which is nearly three years old and which has a more 
visible and more frequently encountered symptom than this bug.

You're welcome to submit a patch yourself.  If that's beyond you (it's certainly 
beyond me), but the problem is really important to you, pay somebody to do it.


> *Since version 1.8a you can now specify a wrap line length of 0.

That setting has been possible since at least 1.4.
(Reporter)

Comment 4

14 years ago
>This is the core of the issue: you assume that everyone is reading mail on a web 
>browser, which is not true.  Some people are still using fairly primitive text-
>based email clients (this is why the "send as plain text" option is recommended) 
>which may wrap long lines by breaking words into pieces at the 80-character 
>boundary; this is harder to read than text broken at a word boundary in the 
>middle of a line.
>
Actually I am assuming that most people are using simple text based clients.
Even since Netscape 3 it was possible to wrap text to the screen without cutting
words in half. Therefore, as this was originally the norm I don't see why
Mozilla should be introducing any new scheme that no one needs.

The reason for sending email in plain text is that it won't contain viruses and
people who have turned off HTML rendering [highly recommended] can still read
the email. Such emails also avoid bloat of all the unnecessary code associated
with HTML.

>>I don't see why it is impossible to control how Mozilla displays the text it
>>receives [or has sent] in its real layout.
>>Nowadays there is very little reason for the system to be so "smart" that it
>>just makes the situation worse.
>
>
>Oddly, the people who tend to agree about format=flowed being a bad thing are 
>those who want to use primitive software, where the receiving mail client cannot 
>support it.  Not unlike your recipients' webmail software, which *should* be 
>modern enough to handle incoming mail sent as f=f; format=flowed is a widely 
>used standard and should be supported just as MIME and quoted-printable are 
>supported.  
>
For the reason above all primitive software [Netscape 4.7x or before] is able to
handle plain text adequately so there is no need for the Mozilla default to
insert any line breaks [which cannot be turned off by the user] in the
conversion from HTML composed text to plain text. [One is forced to used the
HTML editor due to the inadequacies of the plain text one].

>You do not understand the development model here.  Unless this bug (and here, 
>I'm referring *only* to the issue of the specified plain-text wrap column being 
>ignored on HTML->plain conversion) catches the interest of a developer, it's not 
>going to be fixed.  Not for lack of desire, but for lack of resources.  See, for 
>instance, bug 125928, which is nearly three years old and which has a more 
>visible and more frequently encountered symptom than this bug.
>
It was my understanding that each of the Mozilla components of open source had
authors and volunteers. Is it not the author's responsibility to oversee and fix
bugs. I note that many bugs have been outstanding for years, as a result Mozilla
is just not getting the support it needs to be properly competitive and survive.
It seems it is a project out of control [no managers] and doomed to die - shame.

I have used cut and paste from the Mozilla email for you to see what happens to
quoted text when the wretched line breaks get inserted and then you apply line
wrapping to the view of the received email [in a small window]. You can end up
with lines with a word[s] on it but without the preceding ">" indicating it was
original quoted text. In short it just makes a complete mess of the email. It
may or may not show correctly in this bug report. If it doesn't show cut the
text, insert it in Notepad [with word wrap on] and shrink the width till line
wrapping occurs.

Comment 5

14 years ago
*** Bug 277153 has been marked as a duplicate of this bug. ***

Comment 6

12 years ago
*** Bug 330960 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 7

12 years ago
It was suggested that I limit the line length to 990 instead, but this isn't making any difference...  Lines are still being wrapped at 72 characters.


(Assignee)

Comment 8

12 years ago
Created attachment 218498 [details] [diff] [review]
Use the appropriate preference to set the width instead.

Borrowed code from nsMsgAttachmentHandler::UrlExit()...

Though I'm wondering if we might want to use nsMsgCompose::GetWrapLength(), for example, to eliminate code duplication.
Attachment #218498 - Flags: review?(bienvenu)

Comment 9

12 years ago
That'a a good start.  I'm not so sure about the details of the "sanity" 
section.  Your maximum value of 30000 is too large; it should be no more than 990 -- longer lines risk being broken mid-word during preparation for SMTP transport (bug 277185).

If you use 72 for the wrap=0 case, there may be some complaints -- currently, when composing as plain-text, with wrap=0, the lines are sent out unbounded.
(Personally, I think *that* behavior is broken...)  Maybe resetting it to the max of 990 for the 0 case would make more sense (for plain text, too).

xref bug 221596
(Assignee)

Comment 10

12 years ago
(In reply to comment #9)
> That'a a good start.  I'm not so sure about the details of the "sanity" 
> section.  Your maximum value of 30000 is too large; it should be no more than
> 990 -- longer lines risk being broken mid-word during preparation for SMTP
> transport (bug 277185).

Like I said, it was lifted wholesale from nsMsgAttachmentHandler::UrlExit().

I'm not sure I understand how they arrived at their logic in the original code, but it didn't get any complaints there...  Of course, they might not have been prepping it for transport via SMTP either, but instead for writing to disk... I don't know.

Would be handy to get someone familiar with that code to review it/comment on it.


Comment 11

12 years ago
Comment on attachment 218498 [details] [diff] [review]
Use the appropriate preference to set the width instead.

you can't use nsMsgCompose::GetWrapLength() from this code because you don't have access to the nsMsgCompose object here - you could add the wrapLength as a parameter to ConvertBufToPlainText as I described earlier - that tends to get called from nsMsgCompose objects.

Re the sanity checks, the 30000 is arbitrary, and remember, it only comes into play when the user sets the pref to some outrageous value. My inclination would be to leave it as is - if the user sets that pref to an outrageous value, we might as well assume they  know what they're doing. In fact, I'd probably just remove the upper bound check completely.  For the width = 0 case, what happens if you pass in 0 to textSink->Initialize? I'm on the fence about the setting the width to 72 if they pass in 0 - having 0 mean unbounded makes more sense to me than making it mean 72.

Thanks for working on this - I leave it up to you if you want to add a parameter, defaulted to 72, to ConvertBufToPlainText, or directly get the pref here - but I would like to know what width = 0 will do in this routine, and I'd probably skip the upper bound limit.
(Assignee)

Comment 12

12 years ago
(In reply to comment #11)
> (From update of attachment 218498 [details] [diff] [review] [edit])
> you can't use nsMsgCompose::GetWrapLength() from this code because you don't
> have access to the nsMsgCompose object here - you could add the wrapLength as a
> parameter to ConvertBufToPlainText as I described earlier - that tends to get
> called from nsMsgCompose objects.

Ok, so I'm trying to figure out which instance of ConvertBufToPlainText() is being called...

It looks like the instances in QuotingOutputStreamListener::ConvertToPlainText()... but there's no nsMsgCompose object handy at that point (despite this being in the nsMsgCompose.cpp).

So how does one get back to the nsMsgCompose object?  I don't see it in scope.

> Re the sanity checks, the 30000 is arbitrary, and remember, it only comes into
> play when the user sets the pref to some outrageous value. My inclination would
> be to leave it as is - if the user sets that pref to an outrageous value, we
> might as well assume they  know what they're doing. In fact, I'd probably just
> remove the upper bound check completely.  For the width = 0 case, what happens
> if you pass in 0 to textSink->Initialize? I'm on the fence about the setting
> the width to 72 if they pass in 0 - having 0 mean unbounded makes more sense to
> me than making it mean 72.

That might need to be changed in Initialize() or wherever the width actually gets applied...  actually, it gets tucked into mWrapColumn, it looks like.  And this gets used as:

../BUILD/mozilla/content/base/src/nsPlainTextSerializer.cpp:1176:        (mPreFormatted && !mWrapColumn) ||
../BUILD/mozilla/content/base/src/nsPlainTextSerializer.cpp:1190:        (mPreFormatted && !mWrapColumn) ||
../BUILD/mozilla/content/base/src/nsPlainTextSerializer.cpp:1205:    PRUint32 width = (mWrapColumn > 0 ? mWrapColumn : 25);
../BUILD/mozilla/content/base/src/nsPlainTextSerializer.cpp:1384:    PRUint32 bonuswidth = (mWrapColumn > 20) ? 4 : 0;
../BUILD/mozilla/content/base/src/nsPlainTextSerializer.cpp:1387:    while(mCurrentLineWidth+prefixwidth > mWrapColumn+bonuswidth) {
../BUILD/mozilla/content/base/src/nsPlainTextSerializer.cpp:1395:      while(goodSpace > 0 && (width+prefixwidth > mWrapColumn)) {
...


> Thanks for working on this - I leave it up to you if you want to add a
> parameter, defaulted to 72, to ConvertBufToPlainText, or directly get the pref
> here - but I would like to know what width = 0 will do in this routine, and I'd
> probably skip the upper bound limit.


Well, I'm thinking of:

    if (wrapWidth < 0)
      wrapWidth = 0;
    else if (wrapWidth > 0 && wrapWidth < 10)
      wrapWidth = 10;

but... grrr....  wrapWidth should be a PRUint32, since it doesn't make sense for it to ever be less than 0... but that's not the way GetWrapLength() is written (plus other function prototypes).


Comment 13

12 years ago
(In reply to comment #11)
> Re the sanity checks, the 30000 is arbitrary, and remember, it only comes
> into play when the user sets the pref to some outrageous value. My
> inclination would be to leave it as is - if the user sets that pref to an
> outrageous value, we might as well assume they know what they're doing. 
> [...]
> I'm on the fence about the setting the width to 72 if they pass in 0 -
> having 0 mean unbounded makes more sense to me than making it mean 72.

If I were designing this, the 990 value (or whatever the actual number is) that's used in breaking down for SMTP would be a globally accessible "max transmitted line length" constant, and somewhere in the chain would be 
something along the lines of 
  if (width == 0 || width > maxXmitLineLength)
     width = maxXmitLineLength;
Anything that allows a line length greater than 990 is going to result in 
broken outgoing messages, and the user "knowing what he's doing" doesn't apply.

Comment 14

12 years ago
I'm fine with using 990 for wrapping width when sending.

Comment 15

12 years ago
Comment on attachment 218498 [details] [diff] [review]
Use the appropriate preference to set the width instead.

Phillip, are you ok with changing that 3000 to 990?
(Assignee)

Comment 16

12 years ago
(In reply to comment #15)
> (From update of attachment 218498 [details] [diff] [review] [edit])
> Phillip, are you ok with changing that 3000 to 990?

Yup, that's fine.
Duplicate of this bug: 381712
So are we waiting for someone to attach a new patch with 30000 replaced by 990 and re-request review?  Or is there something else here that I am missing.  This is causing me issues as I am not able to have lines longer than 72 characters and embed images.  It is kind of one or the other right now, which does not work for the types of e-mails I am required to send in the course of doing my job.
sorry for the spam.  making bugzilla reflect reality as I'm not working on these bugs.  filter on FOOBARCHEESE to remove these in bulk.
Assignee: sspitzer → nobody

Updated

10 years ago
QA Contact: composition
Product: Core → MailNews Core

Updated

9 years ago
Flags: wanted-thunderbird3?
Whiteboard: [patchlove][has patch]
Attachment #218498 - Attachment is obsolete: true
Attachment #218498 - Flags: review?(bienvenu)
Comment on attachment 218498 [details] [diff] [review]
Use the appropriate preference to set the width instead.

Patch has bitrotted.

$ patch -p0 --dry-run < ~/Desktop/tb-width-fix.patch 
patching file mailnews/compose/src/nsMsgCompUtils.cpp
Hunk #1 FAILED at 2039.
Hunk #2 FAILED at 2067.
2 out of 2 hunks FAILED -- saving rejects to file mailnews/compose/src/nsMsgCompUtils.cpp.rej
Philip, any chance for a new patch?
(Assignee)

Comment 22

9 years ago
(In reply to comment #21)
> Philip, any chance for a new patch?

Attaching... though I haven't built against it yet, since FC11 has dbus-glib-0.80 and Mozilla wants to build against dbus-glib-1.
(Assignee)

Comment 23

9 years ago
Created attachment 403381 [details] [diff] [review]
Updated to 3946:75ff08fc784b

Comment 24

9 years ago
+	else if (wrapWidth > 30000)
+	    wrapWidth = 30000;

Given that RFC822 limits line length to 1023 or something, IIRC (and even if my memory misserves me), we should use that as maximum ;-).

Comment 25

9 years ago
998, actually: <http://www.apps.ietf.org/rfc/rfc2822.html#sec-2.1.1>
And you have already discussed that above comment 9 and comment 13 - 16, sorry.
(Assignee)

Comment 26

9 years ago
I've built Fedora images with this patch:

http://www.djhsolutions.net/pprindeville/thunderbird-3.0-2.8.b4.fc11.x86_64.rpm

http://www.djhsolutions.net/pprindeville/thunderbird-3.0-2.8.b4.fc11.i586.rpm

I'll keep them up there for 30 days.

I'm running the former myself, and it seems to work fine.

Updated

9 years ago
Attachment #403381 - Flags: review?(bienvenu)

Comment 27

9 years ago
Comment on attachment 403381 [details] [diff] [review]
Updated to 3946:75ff08fc784b

bienvenu already reviewed. Open from that is mainly:
1:
> For the width = 0 case, what happens
> if you pass in 0 to textSink->Initialize? I'm on the fence about the setting
> the width to 72 if they pass in 0 - having 0 mean unbounded makes more sense to
> me than making it mean 72.

2. Change upper limit from 30000 to 990 (998 per spec minus safety margin for format=flowed trailing spaces, quoting by recipients etc.).

Updated

9 years ago
Attachment #403381 - Flags: review?(bienvenu)
(Assignee)

Comment 28

9 years ago
Created attachment 412118 [details] [diff] [review]
Updated to comments #27

Fix addresses issues pointed out in Comment #27.
Assignee: nobody → philipp
Attachment #403381 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #412118 - Flags: review?(bienvenu)
(Assignee)

Comment 29

9 years ago
Binaries are available (updated) as per Comment #26.
I think the reason for the setting the width to 72 if the pref value comes back as zero, was to cover the case where the preference is not defined.

I suspect the new code will set the width to 990 if the preference is undefined, which would seem to not be desirable.
(Assignee)

Comment 31

9 years ago
Then what's the point of initializating the variable if it's just going to get stomped on with a default?

Updated

9 years ago
Attachment #412118 - Flags: review?(bienvenu) → review+

Comment 32

9 years ago
Comment on attachment 412118 [details] [diff] [review]
Updated to comments #27

the prevailing braces style in that file doesn't seem to be K&R - other than that, seems fine, thx.
(Assignee)

Comment 33

9 years ago
Created attachment 412279 [details] [diff] [review]
Updated to fix indentation & style.

Shouldn't require re-review as it's cosmetic only.
Attachment #412118 - Attachment is obsolete: true
(In reply to comment #33)
> Shouldn't require re-review as it's cosmetic only.

However, as per https://developer.mozilla.org/en/Mailnews_and_Mail_code_review_requirements you'll need sr though.
(Assignee)

Comment 35

9 years ago
(In reply to comment #34)
> (In reply to comment #33)
> > Shouldn't require re-review as it's cosmetic only.
> 
> However, as per
> https://developer.mozilla.org/en/Mailnews_and_Mail_code_review_requirements
> you'll need sr though.

Not clear that this does need an SR, at least not according to:

http://www.mozilla.org/hacking/reviewers.html

and it's 4 bullet list.

Adding "checkin-needed".
Keywords: checkin-needed

Comment 36

9 years ago
Sorry to turn around, but I think  it's indeed safer to go with a default of 72 than 990. I just don't want to risk that we start sending out such messages, even if it's in corner cases.
(Assignee)

Comment 37

9 years ago
(In reply to comment #36)
> Sorry to turn around, but I think  it's indeed safer to go with a default of 72
> than 990. I just don't want to risk that we start sending out such messages,
> even if it's in corner cases.

You realize that this bug was opened 2005-01-04 13:21 PST and that there's been a lot of time to comment on this, right?
Philip: The bullet that says  

"Any other changes that fall outside of the above rules, but affect how two or more code modules function, must have super-review."

currently applies here, as /mailnews code affects both Thunderbird and Seamonkey.

(as a shorthand, code that is in /mail usually doesn't need sr, code that's in /mailnews usually does).
Keywords: checkin-needed
Comment on attachment 412279 [details] [diff] [review]
Updated to fix indentation & style.

I'll try and sr this in the next few days.
Attachment #412279 - Flags: superreview?(bugzilla)
(Assignee)

Comment 40

9 years ago
(In reply to comment #38)
> Philip: The bullet that says  
> 
> "Any other changes that fall outside of the above rules, but affect how two or
> more code modules function, must have super-review."
> 
> currently applies here, as /mailnews code affects both Thunderbird and
> Seamonkey.
> 
> (as a shorthand, code that is in /mail usually doesn't need sr, code that's in
> /mailnews usually does).

Ah... "modules" means "packages" in this case.  Didn't pick up on that.

Thought we were talking about Object family-boundaries or something similar.

Comment 41

9 years ago
Created attachment 412291 [details] [diff] [review]
Fix, v5 - change fallback to 72 instead of 990

Manually hacked-up patch to change the value to 72 (instead of 990) in case we have an invalid pref. I think that's a safer fallback.

r+, because:
<Standard8> BenB: at a brief glance, that code should be defaulting to 72
Attachment #412291 - Flags: superreview?(bugzilla)
Attachment #412291 - Flags: review+
(In reply to comment #41)
> r+, because:
> <Standard8> BenB: at a brief glance, that code should be defaulting to 72

That's not what I meant at the time, and I'd miss-understood the question as well.

I'll take a look in a few days when I get time.

Comment 43

9 years ago
Comment on attachment 412291 [details] [diff] [review]
Fix, v5 - change fallback to 72 instead of 990

OK, marking r?
Attachment #412291 - Flags: review+ → review?
(Assignee)

Comment 44

9 years ago
(In reply to comment #41)
> Created an attachment (id=412291) [details]
> Fix, v5 - change fallback to 72 instead of 990
> 
> Manually hacked-up patch to change the value to 72 (instead of 990) in case we
> have an invalid pref. I think that's a safer fallback.
> 
> r+, because:
> <Standard8> BenB: at a brief glance, that code should be defaulting to 72

(1) You've broken my fix.  A value >= 990 (such as 1023) should still be set to 990.  Your fix doesn't handle this.  The value of "0" is all you should be overriding to 72.

(2) Don't replace the proposed fix unless you're willing to reassign the bug to yourself and take full ownership of it.
(Assignee)

Updated

9 years ago
Assignee: philipp → ben.bucksch

Comment 45

9 years ago
> A value >= 990 (such as 1023) should still be set to 990.

No, my change was intentional. I consider both a value of 0 and of 30000 to be invalid and set it to the value that we always used.

The pref is not meant to allow line widths of 900 characters. Such long lines are insane and will break many other mailers who cannot wrap. *We* can barely wrap the subject. Wrapping is hard, or in some cases explicitly not wanted, e.g. not to destroy ASCII art, but that doesn't mean that long text lines are OK. We *have* to be very conservative about what we send out, as it *has* to work with all mailers out there (that's not optional as you say), and excessively long line are a prime source of grief.

> (2) Don't replace the proposed fix 

I have not marked yours obsolete, to not stomp on your feet.
I have provided the patch simply because you seemed annoyed about additional work.
(Assignee)

Comment 46

9 years ago
And for what it's worth, 72 characters is a pretty irrelevant (or arbitrary) number.

Unless you're running "pine" on an VT100 terminal, you're going to have variable width fonts on some sort of windowed graphical interface.

Comment 47

9 years ago
The 72 comes from:
* The conventional maximum is 80 chars, as that's the width of many text screens.
* With CR, UI borders etc, that might be 79 or 78.
* Now you reply and quote a few times, and several "> " get added.
* Then you have *some* broken mailers (*cough* Outlook [Express]) who wrap
  quoted lines, and you get "comb wrap".

That's why the default is 72. It allows for all these situations, which happen a lot in practice and were a major problem.

Philip, if you were not aware of that, you're giving a good example why a user should not be able to set this at all. If this is my call, I'd probably even wontfix it.

Comment 48

9 years ago
And, BTW, lots of people, esp. developers, still use mutt, pine and friends up to this day.
(Assignee)

Comment 49

9 years ago
(In reply to comment #45)
> > A value >= 990 (such as 1023) should still be set to 990.
> 
> No, my change was intentional. I consider both a value of 0 and of 30000 to be
> invalid and set it to the value that we always used.
> 
> The pref is not meant to allow line widths of 900 characters. Such long lines
> are insane and will break many other mailers who cannot wrap. *We* can barely
> wrap the subject. Wrapping is hard, or in some cases explicitly not wanted,
> e.g. not to destroy ASCII art, but that doesn't mean that long text lines are
> OK. We *have* to be very conservative about what we send out, as it *has* to
> work with all mailers out there (that's not optional as you say), and
> excessively long line are a prime source of grief.

990 is hardly "insane", or the RFCs wouldn't allow for 1023 character long lines.

If you don't like the RFC's, get them changed, and *then* we'll modify our code to conform to the updated standards.

"Insane" is your personal opinion.  It doesn't trump a standard, which uses language such as MUST and SHOULD.

If you have to deal with "many other" broken mailers, I suggest you divert your effort into fixing them, rather than blocking progress here.


> > (2) Don't replace the proposed fix 
> 
> I have not marked yours obsolete, to not stomp on your feet.
> I have provided the patch simply because you seemed annoyed about additional
> work.

I'm not annoyed about additional work.

I'm annoyed about coming to the party late and covering a lot of the same ground that's already been gone over for your benefit.

If *you* have to deal with users with broken mailers that can't handle long lines, then you can set *your* preferences to whatever you like and *your* problem will be solved.

Don't try and impose your interpretation of what "sanity" is on everyone else.
(Assignee)

Comment 50

9 years ago
(In reply to comment #47)
> The 72 comes from:
> * The conventional maximum is 80 chars, as that's the width of many text
> screens.
> * With CR, UI borders etc, that might be 79 or 78.
> * Now you reply and quote a few times, and several "> " get added.
> * Then you have *some* broken mailers (*cough* Outlook [Express]) who wrap
>   quoted lines, and you get "comb wrap".

"Ugly" or "aesthetically displeasing" doesn't qualify as broken (as in, failing to interoperate).

80 characters is the width of text screens?  Really?  Are we talking ADM-3a terminals?

Do you really think that many people are using glass terminals without a graphics (windowed) capability?

> That's why the default is 72. It allows for all these situations, which happen
> a lot in practice and were a major problem.

I'm not trading in my Gnome desktop for an IBM 722 card punch if that's where this is going...

We could stick to US-ASCII as the default, and that would be the safest, most conservative as well.

Alas, there are people out there with demands that compel us to make forward progress, kicking and screaming.

> Philip, if you were not aware of that, you're giving a good example why a user
> should not be able to set this at all. If this is my call, I'd probably even
> wontfix it.

Progress indeed!
(Assignee)

Comment 51

9 years ago
Comment on attachment 412291 [details] [diff] [review]
Fix, v5 - change fallback to 72 instead of 990

This is not handling the case of > 990 correctly.
Attachment #412291 - Flags: review? → review-

Comment 52

9 years ago
I don't think this discussion is worth to extend, just responding to the personal attacks:

> 990 is hardly "insane"

I think I explained why. Try to read a text with such long lines, and you will emotionally feel why it's important to get this right.

The cases in comment 47 were real-world, common problems, which caused a lot of grief for users. It was simply unreadable in practice and very user-visible, also in our software. Yes, 80 char width is still the magic number for wrapping up to this day.

We have 10 millions of users with 100 millions of recipients. Getting the outgoing format right in all cases is most important.
"Be liberal in what you accept, and conservative in what you send" (RFC 1122)

> ... or the RFCs wouldn't allow for 1023 character long lines.

They may allow 998 max length for other reasons than natural language text? There's all kinds of other uses of mail than English text.

Comment 53

9 years ago
Comment on attachment 412291 [details] [diff] [review]
Fix, v5 - change fallback to 72 instead of 990

Philip, you are in no capacity to r- my patch.

And it *is* handling it correctly, as I said. A value of 30000 is invalid.
Attachment #412291 - Flags: review- → review?(bugzilla)

Comment 54

9 years ago
Thanks Philip for pointing out:
<http://www.apps.ietf.org/rfc/rfc2822.html#sec-2.1.1>
> The more conservative 78 character recommendation is to accommodate the many
> implementations of user interfaces that display these messages which may
> truncate, or disastrously wrap, the display of more than 78 characters per
> line, in spite of the fact that such implementations are non-conformant to
> the intent of this specification

This, plus 4 quote levels (comment 47) means 72 chars.
(Assignee)

Comment 55

9 years ago
(In reply to comment #52)
> I don't think this discussion is worth to extend, just responding to the
> personal attacks:
> 
> > 990 is hardly "insane"
> 
> I think I explained why. Try to read a text with such long lines, and you will
> emotionally feel why it's important to get this right.

Fix the cause, not the symptom.  Section 2.1.1 of the RFC is quite clear:

The more conservative 78 character recommendation is to accommodate the many implementations of user interfaces that display these messages which may truncate, or disastrously wrap, the display of more than 78 characters per line, in spite of the fact that such implementations are non-conformant to the intent of this specification (and that of [RFC2821] if they actually cause information to be lost). Again, even though this limitation is put on messages, it is encumbant upon implementations which display messages to handle an arbitrarily large number of characters in a line (certainly at least up to the 998 character limit) for the sake of robustness.

"It is encumbant [sic] upon implementations which display messages to handle an arbitrarily large number of characters [...] for the sake of robustness."

In other words, it's up to them, not us, to do the right thing.

It doesn't get any more clear than that.
(Assignee)

Comment 56

9 years ago
(In reply to comment #54)
> Thanks Philip for pointing out:
> <http://www.apps.ietf.org/rfc/rfc2822.html#sec-2.1.1>
> > The more conservative 78 character recommendation is to accommodate the many
> > implementations of user interfaces that display these messages which may
> > truncate, or disastrously wrap, the display of more than 78 characters per
> > line, in spite of the fact that such implementations are non-conformant to
> > the intent of this specification
> 
> This, plus 4 quote levels (comment 47) means 72 chars.

What if the message gets forwarded 5 times?  Or 6?  What if the person doesn't use "> " for nesting, but instead "===>" (which I've seen)?

The point is that there will always be a scenario in which things break, and you can't adapt the most pessimistic solution at the cost of progress (or functionality) because there is no perfect fix (as demonstrated above).

Comment 57

9 years ago
Created attachment 412319 [details] [diff] [review]
Fix, v6. Max 72.

Yes, it basically follows the rule:
"Be liberal in what you accept, and conservative in what you send" (RFC 1122)
In this case: Don't *send* msgs with more than 78 chars, but you need to *accept* and display them properly.

So, following this, here's a patch which enforces a maximum of 72 characters.
That's 78 minus 4 quote levels, given that we know some mailers wrap quotes (comment 47).

This still allows the user to allow for more leeway / quote chars or even to wrap at 60 chars, if he thinks that prettier.
Attachment #412319 - Flags: superreview?(bugzilla)
Attachment #412319 - Flags: review?(bugzilla)
(Assignee)

Comment 58

9 years ago
(In reply to comment #30)
> I think the reason for the setting the width to 72 if the pref value comes back
> as zero, was to cover the case where the preference is not defined.
> 
> I suspect the new code will set the width to 990 if the preference is
> undefined, which would seem to not be desirable.

For Ben's sake, I'll point out that this interpretation wasn't actually correct.

Comment 59

9 years ago
I leave it up to the reviewer which patch he takes. I intentionally didn't mark the others obsolete.
(Assignee)

Comment 60

9 years ago
(In reply to comment #57)
> Created an attachment (id=412319) [details]
> Fix, v6. Max 72.

Is this a joke?

> Yes, it basically follows the rule:
> "Be liberal in what you accept, and conservative in what you send" (RFC 1122)
> In this case: Don't *send* msgs with more than 78 chars, but you need to
> *accept* and display them properly.

This is the epitome of intellectual dishonesty.  You're deliberately misquoting the RFC.

Again, quoting the RFC (since it apparently can't be done enough):

"There are two limits that this standard places on the number of characters in a line. Each line of characters MUST be no more than 998 characters, and SHOULD be no more than 78 characters, excluding the CRLF."

We can, if we desire (and nothing is more explicit an expression of desire than a user changing an profile value) set it to anything between 78 and 998.

Period.

> So, following this, here's a patch which enforces a maximum of 72 characters.
> That's 78 minus 4 quote levels, given that we know some mailers wrap quotes
> (comment 47).

Ok, cite me where it states that (a) quoting (which I frankly don't give a damn about) will always use 2-character indentation, and (b) that forwarding more than 4 times is forbidden?

I've got a chain letter in my mailbox that was forwarded 41 times... so the nesting alone takes more than 78 characters.

> This still allows the user to allow for more leeway / quote chars or even to
> wrap at 60 chars, if he thinks that prettier.

And if he thinks that 132 is better, he can't, because you don't want him to be able to.

Even though the RFC clearly allows it.

Since when did we start being more restrictive than Microsoft?
(Assignee)

Comment 61

9 years ago
I see the discussion boiling down to this.

Ben's argument is that we should limit Thunderbird's functionality to keep parity with whatever broken mailers might exist out there that TB users may or may not ever have the occasion (or need) to communicate with.

I reject this argument.

(1) if we're limited to the "lowest common denominator" of the least functional software out there, then we'll never make any progress;

(2) our actions should be driven by what we can control (i.e. TB, who uses it, and how it is used) not by what we can't control (the MUA of recipients of messages that we may or may not send);

(3) and lastly, if Ben's highest priority is the broken mailers out there, why isn't he spending his effort contributing to those mailers and making them better (or at least less broken), rather than arguing the cause for being overly restrictive to all TB users?

I've seen the argument before that "we shouldn't do X because someone out there might not be able to deal with it" over and over again, and fortunately even though it's an argument that could be made against any and all new functionality, that hasn't been the case.

Nor should it be here.
I'd like to request a stop to the patch war, and maybe see if we can come up with a calmer and maybe more productive discussion.

Maybe we can step back a bit, all the way to end-user desires, and for now ignoring the RFC or specific competing patches.

1) Philip, can you give examples of why setting the line length is important to users?  I'm looking for things like "when pasting text from web pages", "sending programs in email", or the like -- from those use-case-like scenarios we can hopefully look at the whole picture, not just this particular pref setting.  In particular, I'd like to know scenarios where the defaults are wrong.  

2) Ben, am I right that your concern is that people will adjust the message to look good in their eyes, but not be aware that their correspondents may end up seeing something altogether differently?

3) It seems to me that there are many options that aren't being considered, such as 

 - revisiting whether this pref does what users think it does

 - getting some metrics about which clients do what (the world changes all the time)

 - warning users who set long line lengths of the possible consequences for some recipients (ideally informed by the metrics above)

 - identifying specific scenarios above and then changing the line-wrapping behavior for specific emails or parts-of-emails.

 - doing a better job of separating out the prefs that affect how users read their messages (even the ones they author) and those that affect how their messages are read by others.  That comment applies not just to this pref, but to the use of inline images in HTML email, use of some fonts, colors, accessibility concerns, etc.

Comment 63

9 years ago
(I was done with this discussion, but since you ask directly:)

> 2) Ben, am I right that your concern is that people will adjust the message to
> look good in their eyes, but not be aware that their correspondents may end up
> seeing something altogether differently?

Yes, that is one concern. What we send out needs to consider all possible recipients, and that's something the users typically can't know. Or, when pointed out, simply don't care.

Related to that, there are strange effects that the user may not be aware of, because they occur when certain things play together, e.g. quoting and Outlook wrapping of quotes at 80 chars, as described in comment 47. That used to be a real-world and very frequent and visible problem. This caused our default to 72. But it's entirely non-obvious, even to many advanced users.

> identifying specific scenarios above and then changing the line-wrapping
> behavior for specific emails or parts-of-emails.

OK, I'll describe the case from the last paragraph:
1. I write a line with 76 chars. (sounds reasonable)
2. Somebody else quotes it. It now has 78 chars.
3. A third person quotes it, who uses Outlook [Express].
   The line is at 80 chars, and OE hard-wraps even quotes at that
   line. (I don't know whether they fixed it in the meantime in
   their 4 clients: Outlook, Outlook Express, Windows Mail and Hotmail Web.)
4. I (and others) recieve a line that is comb-wrapped, like I will do with this line there.
   You can actually see that in bugzilla sometimes, too, because the line-length in the
   textfield is longer than what bugzilla hard-warps to. At bugzilla and source code files,
   you can also see that the 80-char limit is still common today in many systems, for
   various reasons. As you can see here, this is really hard to read, and even more so, if
   I did that for many paragraphs, because the whole paragraph, list, text structure gets
   lost in this mess. I am glad I don't have to read such mails anymore.


Lastly, I do think that following RFCs, including SHOULD rules, is important. And also the "conservative in what you send" rule in RFC 1122. This should take care of "policing users" accusations.


I want to put on record that Philip assigned this bug to me (without me asking for it or wanting it), I did not grab it.

Comment 64

9 years ago
Oh, and I supplied patch v5 to save him work, since he (understandably) seemed frustrated in comment 37, but I found it important to get addressed.
(Assignee)

Comment 65

9 years ago
(In reply to comment #62)
> I'd like to request a stop to the patch war, and maybe see if we can come up
> with a calmer and maybe more productive discussion.
> 
> Maybe we can step back a bit, all the way to end-user desires, and for now
> ignoring the RFC or specific competing patches.
> 
> 1) Philip, can you give examples of why setting the line length is important to
> users?  I'm looking for things like "when pasting text from web pages",
> "sending programs in email", or the like -- from those use-case-like scenarios
> we can hopefully look at the whole picture, not just this particular pref
> setting.  In particular, I'd like to know scenarios where the defaults are
> wrong.

David:  Thanks for taking an interest.

Actually, your first example is such a case.

If someone cuts and pastes some article (i.e. multiple short to medium length paragraphs) unwrapped in an email message to me, and then I forward it on, it will eventually get prepended with more and more "> " noise.

If there's one line per paragraph, then removing all of that (if I want to extract the original text intact) is easier than if it's been wrapped (since I only have to remove indents from N paragraphs, as opposed to M lines, where M >> N).

> 2) Ben, am I right that your concern is that people will adjust the message to
> look good in their eyes, but not be aware that their correspondents may end up
> seeing something altogether differently?
> 
> 3) It seems to me that there are many options that aren't being considered,
> such as 
> 
>  - revisiting whether this pref does what users think it does

Currently it does nothing, and is ignored... which is probably the last thing that anyone expects.


>  - getting some metrics about which clients do what (the world changes all the
> time)

The world does indeed change all the time.  Which is why I'm opposed to limiting ourselves to work more optimally with a mailer which may be either "fixed" or (equally likely) obsoleted tomorrow.

Let's focus on what we can control, and what we're actually responsible for.

>  - warning users who set long line lengths of the possible consequences for
> some recipients (ideally informed by the metrics above)

Given that getting into the config editor and changing this value is a non-trivial task... can we make some assumption that anyone who gets that far is reasonably clueful and doesn't need to be protected from himself?

>  - identifying specific scenarios above and then changing the line-wrapping
> behavior for specific emails or parts-of-emails.

This affects *only* the scenario where the message has been composed in HTML mode, but is being "down-converted" to send to one or more recipients that can't handle HTML (or where the default behavior is to send out TEXT).

It's a fairly specific use-case scenario.

>  - doing a better job of separating out the prefs that affect how users read
> their messages (even the ones they author) and those that affect how their
> messages are read by others.  That comment applies not just to this pref, but
> to the use of inline images in HTML email, use of some fonts, colors,
> accessibility concerns, etc.

That's a much bigger scope.

I'm hoping that closure on that last item won't be a blocker for this bug.
(Assignee)

Comment 66

9 years ago
(In reply to comment #63)
> OK, I'll describe the case from the last paragraph:
> 1. I write a line with 76 chars. (sounds reasonable)
> 2. Somebody else quotes it. It now has 78 chars.
> 3. A third person quotes it, who uses Outlook [Express].
>    The line is at 80 chars, and OE hard-wraps even quotes at that
>    line. (I don't know whether they fixed it in the meantime in
>    their 4 clients: Outlook, Outlook Express, Windows Mail and Hotmail Web.)

Then by your own admission, it's with Outlook Express that the problem lies.  Not Thunderbird.

If we set the limit to 72 characters, and people reply back and forth 6 times or more, then this problem will still exist.  Nothing will have been solved.

> 4. I (and others) recieve a line that is comb-wrapped, like I will do with this
> line there.
>    You can actually see that in bugzilla sometimes, too, because the
> line-length in the
>    textfield is longer than what bugzilla hard-warps to. At bugzilla and source
> code files,
>    you can also see that the 80-char limit is still common today in many
> systems, for
>    various reasons. As you can see here, this is really hard to read, and even
> more so, if
>    I did that for many paragraphs, because the whole paragraph, list, text
> structure gets
>    lost in this mess. I am glad I don't have to read such mails anymore.

That's actually a *really* compelling reason for having text be 990 characters long!  Then most text will never have to be wrapped.  Problem solved.

> Lastly, I do think that following RFCs, including SHOULD rules, is important.
> And also the "conservative in what you send" rule in RFC 1122. This should take
> care of "policing users" accusations.

I've known Jon Postel for years (he was still the editor when I published two RFC's and when I chaired or sat on numerous other WG's).

He'd be the first to admit that this adage has plenty of exceptions, and that when security is involved (for instance), being "liberal in what you accept" is not the right course of action.

It is, after all, only a guideline.  It can be strayed from, if the person doing the straying is aware of the tradeoffs.

I think those tradeoffs have been discussed at great length here.

We've done due diligence.

> I want to put on record that Philip assigned this bug to me (without me asking
> for it or wanting it), I did not grab it.

I did indeed.  You superseded my attachment with your own, after I had addressed your comments, saying that I did not think them to be sufficient cause to change my code.

You're adding an attachment nonetheless means you think you can do better.

So do so.
If you two don't stop bickering I'm walking away from this bug (quite a threat, I know!)

Aside: is it just me, or has the UI for this pref gone away since this bug was filed?  I can't find it.

If there's no UI for it anymore, I'm going to walk away anyway =).

Comment 68

9 years ago
> has the UI for this pref gone away since this bug was filed? I can't find it.

Indeed, I also checked (an hour or so ago) and it's gone.
(Assignee)

Comment 69

9 years ago
(In reply to comment #68)
> > has the UI for this pref gone away since this bug was filed? I can't find it.
> 
> Indeed, I also checked (an hour or so ago) and it's gone.

Again, it would significantly increase the signal-to-noise ratio of this exchange if you code make an effort to be better informed before interjecting to this conversation.

As I understand it, your objections have been based on the premise that a non-existent GUI dialog allows the uninformed user to easily change the wrap width.

As this isn't the case, I'm taking back the bug and obsoleting your patch.
(Assignee)

Updated

9 years ago
Assignee: ben.bucksch → philipp
(Assignee)

Updated

9 years ago
Attachment #412319 - Flags: superreview?(bugzilla)
Attachment #412319 - Flags: review?(bugzilla)
(Assignee)

Updated

9 years ago
Attachment #412319 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Attachment #412291 - Attachment is obsolete: true
Attachment #412291 - Flags: superreview?(bugzilla)
Attachment #412291 - Flags: review?(bugzilla)
(Assignee)

Updated

9 years ago
Attachment #412279 - Flags: review?(bienvenu)

Comment 70

9 years ago
> As I understand it, your objections have been based on the premise that a
> non-existent GUI dialog allows the uninformed user to easily change

No, they were not.

I still think that my first patch is safer (v5), and that we maybe shouldn't even allow to set it wider than 72, given the problems that causes (v6).

But I leave that entirely to the reviewer to decide. I'll be happy with whatever he decides.

Updated

9 years ago
Attachment #412291 - Attachment is obsolete: false
Attachment #412291 - Flags: superreview?(bugzilla)
Attachment #412291 - Flags: review?(bugzilla)

Updated

9 years ago
Attachment #412319 - Attachment is obsolete: false

Updated

9 years ago
Attachment #412291 - Flags: superreview?(bugzilla)
Attachment #412291 - Flags: review?(bugzilla)

Updated

9 years ago
Attachment #412279 - Flags: review?(bienvenu) → review+

Comment 71

9 years ago
Comment on attachment 412279 [details] [diff] [review]
Updated to fix indentation & style.

given that this is a hidden pref, I'm OK with this. But it should be

if (pPrefBranch)
{
(Assignee)

Comment 72

9 years ago
Created attachment 413153 [details] [diff] [review]
Updated to fix indentation & style

As per bienvenu's comments.
Attachment #412279 - Attachment is obsolete: true
Attachment #412279 - Flags: superreview?(bugzilla)
(Assignee)

Updated

9 years ago
Attachment #413153 - Flags: superreview?(bugzilla)
Attachment #413153 - Flags: review?(bienvenu)
(Assignee)

Updated

9 years ago
Attachment #413153 - Flags: review?(bienvenu) → review+
(Assignee)

Comment 73

9 years ago
Carrying forward bienvenu's +r
Attachment #413153 - Flags: superreview?(bugzilla) → superreview+
(Assignee)

Updated

9 years ago
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/1211fc933b33
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Flags: wanted-thunderbird3?
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1a1
You need to log in before you can comment on or make changes to this bug.