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)
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)
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.
Assignee | ||
Comment 1•9 years ago
|
||
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 | ||
Comment 2•9 years ago
|
||
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.
Assignee | ||
Comment 3•9 years ago
|
||
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+
Assignee | ||
Updated•9 years ago
|
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
Assignee | ||
Comment 4•9 years ago
|
||
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
Assignee | ||
Comment 5•9 years ago
|
||
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)
Assignee | ||
Comment 6•9 years ago
|
||
(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.
Comment 7•9 years ago
|
||
(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.
Updated•9 years ago
|
Attachment #8696509 -
Flags: review?(Pidgeot18) → review+
Assignee | ||
Comment 8•9 years ago
|
||
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
Comment 9•9 years ago
|
||
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
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 45.0
Assignee | ||
Comment 11•9 years ago
|
||
This fix also fixes issues reported in bug 1230971 for forwarded messages.
Assignee | ||
Comment 12•9 years ago
|
||
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
You need to log in
before you can comment on or make changes to this bug.
Description
•