169395 - Long words (~990 characters) are wrapped (quoted-printable)

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Boris 'pi' Piwinger]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.2a) Gecko/2002091318
Mozilla/5.0 (Windows; U; Windows NT 5.0; de-AT; rv:1.1) Gecko/20020826
and other version (users of de.comm.software.mozilla contributed to this bug)

This is happening for plain text, I don't now what happens for HTML composition.

I posted a very long URL, actually a bugzilla query
(news:am6pne$359rl$[e-mail]). This URL was wrapped after 991
characters (users using format flawed have one character less). I allow
quoted-printable, with is crucial due to the line length limitation of some news
servers (and mail servers?). In my case quoted-printable was *not* used, hence
the URL had to be wrapped. This is one part of the bug.

In another posting which was quoted-printable, the URL was also wrapped, here
without any justification. This is part two of the bug.

pi

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

does this get wrapped while composing the mail, or after clicking send? in the
latter case, it would belong to MailNews somehwere, probably Networking:NNTP

and even in the first case, it's probably MailNews:Mail Compose (if that exists)

Comment 2

[Boris 'pi' Piwinger]

I did a quick and dirty test. I pasted the URL into a MailNews window and saved
it. In the drafts folder it was already wrapped.

pi

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

changing component

Comment 4

[Stephan Grossklass]

As to part one of the bug: There were no 8-bit characters in the posting so
Mozilla used 7-bit US-ASCII. Suggestion: If QP is enabled, it should be used
when (a) 8-bit characters appear (as it's already done) and (b) when lines
exceeding 990 characters exist. (BTW it's 991 characters including one space for
f=f.) With a default of 8-bit, the current implementation should remain
unchanged, i.e. (a) only.
As to part two, this can probably only be fixed by removing the "XXL line wrap"
in message composition and moving it to a stage shortly before mail/news gets
sent. Then only non-QP messages should still have such very long lines.

Comment 5

[Oliver Putz]

I have found a similar problem with quoting. I think it reffers to this bug, so
I will post it here.

If you quote a long string *or* a long text, in both cases the text is wrapped
before it should be wrapped.

If you quote aaaaaa (~990 chars) the first ~988 chars will be quoted, and the
other chars will not be quoted.

For example:

> aaaaa (long line)
aa (rest of the string above)

But this problem does not only occur when wrapping a string, it occurs also when
you want to wrap a long text without manual linebreaks. The first 990 chars of
the text will be quotet, then it will be wrapped, and the rest of the text will
be put a one line down, but will not be quoted.

For example: (I try to quote a long text (more than 990 chars), sybolized by the
text "I like snow in winter because it is very cold then")

> I like snow in winter
> because it is very cold
then

Comment 6

[Mad Alex]

Attachment: Patch to remove the 990 line length limit and use quoted-printable with long lines

Apply in the root of the cvs working copy with patch -p0.

Note that this has only been tested with Thunderbird 1.5.0.2 on Linux. This is also my first time hacking the Mozilla sources. Expect it to break something! Hopefully it will be useful as a starting point at least.

Comment 7

[Mad Alex]

The desired behaviour described is implemented for text file attachments but not for the message body. The problem stems from mailnews/compose/src/nsMsgSend.cpp (class nsMsgComposeAndSend) handling the first message part (the main body) differently from the other parts. As several comments in the file say, the code should ideally be re-written to allow the message body to be treated largely the same as an attachment to avoid code duplication.

However I have been able to make a fix by:
- removing the nsMsgComposeAndSend::EnsureLineBreaks private member function
- updating nsMsgComposeAndSend::SnarfAndCopyBody to not call EnsureLineBreaks and instead accept the body text as it is without worrying about line lengths
- updating nsMsgComposeAndSend::GatherMimeAttachments to check for long lines in the body and use quoted-printable if any are found (this works in a very similar way to nsMsgAttachmentHandler::PickEncoding for attachments)

Scanning the body for long lines shouldn't be any more costly (in terms of speed or memory) than the hard breaking of lines by EnsureLineBreaks. It might even be better.

See the attached patch.

Comment 8

[Mike Cowperthwaite]

Mad Alex, are you still around?  Is this patch still viable?

Obviously, Ducarroz is not doing anything for us any more.  If you can get this patch working, this would be great to get into the system.  Try asking for a review from:  [e-mail]

xref bug 277185, bug 344654

Comment 9

[Mad Alex]

Comment on attachment 221653 [details] [diff] [review]
Patch to remove the 990 line length limit and use quoted-printable with long lines

That patch still applies OK, albeit with some slight offsets where things have changed. I have only given it a quick test but it compiled fine and everything seemed to work correctly.

Comment 10

[Mad Alex]

Attachment: Patch to remove the 990 line length limit and use quoted-printable with long lines

Updated the patch to make it work again with recent changes in CVS.

It would be really nice to see this make it into CVS as it brings mozilla's handling of long lines closer to that of other mail software and fixes various bugs in addition to this one; in particular the hacks (with their associated bugs) to walk backwards and insert breaks in sensible places (e.g. in HTML) become obsolete and are removed by this patch.

Comment 11

[Mad Alex]

Attachment: Patch to remove the 990 line length limit and use quoted-printable with long lines

Updated the patch for recent changes in CVS

Comment 12

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Alex, the latest sources are now in Mercurial, could you please patch against comm-central sources?

Ref: https://developer.mozilla.org/en/Comm-central_source_code_(Mercurial)

Comment 13

[Mad Alex]

Attachment: Patch to remove the 990 line length limit and use quoted-printable with long lines

Updated patch against the latest comm-central sources. Thanks for the heads-up.

Comment 14

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #13)
> Created an attachment (id=372051) [details]
> Patch to remove the 990 line length limit and use quoted-printable with long
> lines
> 
> Updated patch against the latest comm-central sources. Thanks for the heads-up.

You're welcome! :) Almost there, though.

Please also choose a reviewer from http://www.mozilla.org/mailnews/review.html and set the patch to request r? from the reviewer accordingly.

Comment 15

[Wayne Mery (:wsmwk)]

bienvenu, do you still want this review?

Mad Alex, thanks for the updated patch.

Comment 16

[Wayne Mery (:wsmwk)]

(In reply to Wayne Mery (:wsmwk) from comment #15)
> bienvenu, do you still want this review?

Comment 17

[Wayne Mery (:wsmwk)]

Joshua, any effects for this issue or patch with forthcoming jsmime?

Comment 18

[Joshua Cranmer [:jcranmer]]

(In reply to Wayne Mery (:wsmwk) from comment #17)
> Joshua, any effects for this issue or patch with forthcoming jsmime?

When I rewrite the MIME composition in jsmime, this patch will become obsolete.

Comment 19

[Wayne Mery (:wsmwk)]

Comment on attachment 372051 [details] [diff] [review]
Patch to remove the 990 line length limit and use quoted-printable with long lines

(In reply to Joshua Cranmer [:jcranmer] from comment #18)
> (In reply to Wayne Mery (:wsmwk) from comment #17)
> > Joshua, any effects for this issue or patch with forthcoming jsmime?
> 
> When I rewrite the MIME composition in jsmime, this patch will become
> obsolete.

In that case, let's kill the review request. Besides, patch is 5 years old

Comment 20

[Jorg K (CEST = GMT+2)]

Looks like this was fixed by JSMime in bug 959309. Let's close this bug.