Closed Bug 1230968 Opened 9 years ago Closed 9 years ago

At editing ISO-2022-JP email which has long lines with "Edit As New Message", the long lines have extra spaces

Categories

(Thunderbird :: Message Compose Window, defect)

x86
Windows
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 45.0

People

(Reporter: masayuki, Assigned: jorgk-bmo)

References

Details

(Keywords: jp-critical)

Attachments

(2 files, 1 obsolete file)

Attached file longline-ja-JP.eml (deleted) —
STR: 1. Save the attachment as a .eml file 2. Import it to Daily. 3. Select the email. 4. Right click and choose "Edit As New Message" Expected Result: The message doesn't include extra spaces in the long line. Actual Result: The message includes (unnecessary) extra spaces in the long line. I guess that this is not a regression actually if such email came from other MUAs. However, this is *new*, this becomes reproducible *only* with Thunderbird.
Thanks for the report. OK, the message you present is encoded in ISO-2022-JP, it is flowed with delsp=yes. It can't be a regression, since those messages weren't possible in the past. Before ISO-2022-JP messages were forcibly chopped at 36 characters (72 bytes). As you correctly point out, the software never worked which such a message so the bug is pre-existing and could have produced with a message from anther e-mail client. Looks like the "Edit message as new" doesn't deal with the delsp=yes correctly, that is, it doesn't remove the space again. It also doesn't suppress the wrapping, so you get extra spaces and extra wrapping. I'll look into it.
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Keywords: regression
Version: unspecified → Trunk
Attached patch Extra hunks from bug 26734. (obsolete) (deleted) — Splinter Review
These are the two hunks from Hiro's fix to bug 26734 which I didn't carry forward. When applied, the extra spaces go away, but the lines are still broken at 50 characters. 50, how strange. See https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=26734&attachment=8537118 for the previous review.
Carrying forward Joshua's r+ from bug 26734 comment #115. These are the two hunks from Hiro's original patch. I have fixed the nits from the original review: https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=26734&attachment=8537118 Try run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=e8e138ffe390
Attachment #8696504 - Attachment is obsolete: true
Attachment #8696509 - Flags: review+
Summary: At editing ISO-2022-JP email which has lone lines with "Edit As New Message", the lone lines have extra spaces → At editing ISO-2022-JP email which has long lines with "Edit As New Message", the long lines have extra spaces and are wrapped at 50 characters
OK, the unwanted wrapping is due to bug 1231017 which I just raised. Even in TB 38.4 editing a flowed message loses the flowing. It becomes pretty obvious with this Japanese example as well.
Summary: At editing ISO-2022-JP email which has long lines with "Edit As New Message", the long lines have extra spaces and are wrapped at 50 characters → At editing ISO-2022-JP email which has long lines with "Edit As New Message", the long lines have extra spaces
Comment on attachment 8696509 [details] [diff] [review] Extra hunks from bug 26734 - nits fixed. OK, removing the "Part 1" from the name, since this is it. Once again, this code was reviewed and approved here https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=26734&attachment=8537118 in January 2015 and it fixes the problem reported in this bug. I'm not comfortable landing something I don't understand, so please Joshua, take another look at it. Perhaps you can explain what Hiro had in mind. Apparently he calls MimeInlineTextPlainFlowed_parse_line which does some extra delsp=yes processing but also a whole lot of other stuff.
Attachment #8696509 - Attachment description: Part 1: Extra hunks from bug 26734 - nits fixed. → Extra hunks from bug 26734 - nits fixed.
Attachment #8696509 - Flags: review?(Pidgeot18)
(In reply to Jorg K (GMT+1) from comment #5) > Perhaps you can explain what Hiro had in mind. Apparently he calls > MimeInlineTextPlainFlowed_parse_line which does some extra delsp=yes > processing but also a whole lot of other stuff. Actually, it doesn't do a "whole lot of other stuff", it just does return obj->options->decompose_file_output_fn(line, ...) and this call was previously executed in MimeMessage_parse_line. So I'm more comfortable with what appears a small refactoring. I'd be interested to know why this code runs for forward and "edit as new", exactly the cases from bug 1231017 where the flowing doesn't work, but not for opening/viewing the message or replying to it, where the flowing does work.
(In reply to Jorg K (GMT+1) from comment #6) > I'd be interested to know why this code runs for forward and "edit as new", > exactly the cases from bug 1231017 where the flowing doesn't work, but not > for opening/viewing the message or replying to it, where the flowing does > work. If the code you're referring to is the new lines in mimemsg.cpp, then you're only executing that code when obj->options->decompose_file_p is in effect, and that is an option that's set to indicate the use of drafts. I had really detailed notes on all of the MIME options once, but that went up in smoke several months ago, and I've not had the heart to go back and wade through libmime again.
Attachment #8696509 - Flags: review?(Pidgeot18) → review+
Let's land this. It's not our favourite piece of code, but it does the job in this case and doesn't cause problems, see comment #3.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/99eb42a1fd3dba3a2cfaec4bb7a84bf36ebe3c1d Bug 1230968 - Two hunks from bug 26734, part 3: Need to parse format=flowed message for editing. r=jcranmer
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 45.0
This fix also fixes issues reported in bug 1230971 for forwarded messages.
Comment on attachment 8696494 [details] longline-ja-JP.eml Attaching EML messages as text/plain makes them readable in Bugzilla ;-)
Attachment #8696494 - Attachment mime type: message/rfc822 → text/plain
Depends on: 1287126
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: