Closed
Bug 1268014
Opened 9 years ago
Closed 9 years ago
email without HTML elements is sent as HTML, despite "Delivery Format: Auto-detect" option (since version 45 update)
Categories
(Thunderbird :: Message Compose Window, defect)
Tracking
(thunderbird46 wontfix, thunderbird47 fixed, thunderbird48 fixed, thunderbird49 fixed, thunderbird_esr4546+ fixed)
RESOLVED
FIXED
Thunderbird 49.0
People
(Reporter: claypigeon, Assigned: aceman)
References
(Blocks 1 open bug)
Details
(Keywords: regression, Whiteboard: [regression:TB45])
Attachments
(1 file, 1 obsolete file)
|
(deleted),
patch
|
jorgk-bmo
:
review+
jorgk-bmo
:
approval-comm-aurora+
rkent
:
approval-comm-beta+
rkent
:
approval-comm-esr45+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0
Build ID: 20160407164938
Steps to reproduce:
compose a message with any non-html content,
select option "Delivery Format: Auto-detect",
have a plain-text signature containing a url & Account settings | signature text: use html is off!
Actual results:
TB automatically formatted the signature as a html link and sent the mail in html format
Expected results:
a plain-text mail with unformatted, plain text url in the signature should be sent with these options
|
||
Comment 1•9 years ago
|
||
Thanks for reporting this.
Confirmed. I didn't use paragraph format and no font (variable width) and I still get this, even without a link:
<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=windows-1252">
</head>
<body bgcolor="#FFFFFF" text="#000000">
test<br>
<br>
<pre class="moz-signature" cols="72">--
plain text signature
</pre>
</body>
</html>
Aceman, why is this not downgraded? I see, regression of bug 584313. You react to class=, but forgot moz-signature in the list:
https://dxr.mozilla.org/comm-central/source/mailnews/compose/src/nsMsgCompose.cpp#5190
Can you please provide a one liner that we can include in TB 45.1? I can review. Maybe a test would be good, too.
Blocks: 584313
Status: UNCONFIRMED → NEW
status-thunderbird46:
--- → wontfix
status-thunderbird47:
--- → affected
status-thunderbird48:
--- → affected
status-thunderbird49:
--- → affected
status-thunderbird_esr45:
--- → affected
tracking-thunderbird_esr45:
--- → +
Ever confirmed: true
Flags: needinfo?(acelists)
Keywords: regression
|
|
||
Comment 2•9 years ago
|
||
Wayne, did I understand you correctly that you want to hear about all regressions?
|
|
||
Comment 3•9 years ago
|
||
What about moz-forward-container? You need to add moz-signature and moz-forward-container. Two line fix then.
|
||
Comment 4•9 years ago
|
||
(In reply to Jorg K (GMT+2) from comment #2)
> Wayne, did I understand you correctly that you want to hear about all
> regressions?
Correct. It's helpful to base release progress decisions on data :)
Also, it's an extra step I know, but it's helpful to add regression version to whiteboard once the regressing bug has been identified. (Helps with bugzilla seasrches and assessing regression issues in future months because one doesn't then need to check dates in the blocked regression bug.)
Thanks.
Whiteboard: [regression:TB45]
Strange, I thought signatures are covered by that patch.
What about we make it class=moz-* ?
Flags: needinfo?(acelists)
|
|
||
Comment 6•9 years ago
|
||
(In reply to :aceman from comment #5)
> Strange, I thought signatures are covered by that patch.
Apparently not ;-)
> What about we make it class=moz-* ?
That was my idea, too. Great minds think alike, haha. I think that should be OK. No one should use moz-something as a class and complain that it's downgraded away.
(In reply to Jorg K (GMT+2) from comment #3)
> What about moz-forward-container? You need to add moz-signature and
> moz-forward-container. Two line fix then.
It seems to me a HTML forward creates a table with the info about the original message and that is not convertible.
So this is what we discussed. But I think it does not make forwards downgrade.
|
|
||
Comment 9•9 years ago
|
||
Comment on attachment 8747322 [details] [diff] [review]
patch
Review of attachment 8747322 [details] [diff] [review]:
-----------------------------------------------------------------
Thanks, I'll test it tomorrow.
::: mailnews/compose/src/nsMsgCompose.cpp
@@ +5180,5 @@
> {
> *_retval = nsIMsgCompConvertible::No;
> return NS_OK;
> }
> + // moz-* classes are used internally by the editor, those can be discarded.
... by the editor and mail composition (like moz-cite or moz-signature) ...
|
|
||
Comment 10•9 years ago
|
||
Comment on attachment 8747322 [details] [diff] [review]
patch
Review of attachment 8747322 [details] [diff] [review]:
-----------------------------------------------------------------
This works, as expected.
I didn't run test-send-format.js (since that hunk didn't apply, see below), I assume you have run it.
Please fix the comment as indicated and attach a new patch so that we can attach the uplift flags to the patch.
Something strange has happened when applying the patch. The hunk in mail/test/mozmill/composition/format1-plain.eml failed to apply. I checked and I have format1-plain.eml with Windows line endings (CR+LF) on my machine, which is sort of understandable since this is the format for these message files, right? In the patch there's only LF. So can you please check this. I guess you edited the file by hand and your editor on Linux changed the line endings? Hmm, no that can't be either, because then the entire file would show up in the patch.
Anyway, once a new patch is available, I will make sure it applies for me.
Attachment #8747322 -
Flags: review?(mozilla) → review+
|
Assignee | |
Comment 11•9 years ago
|
||
The eml file contains cr/lf line endings so I manually added them to the new lines. But I missed som of them so it is possible your program applying the patch on Windows could get confused. Try this one.
Attachment #8747322 -
Attachment is obsolete: true
Attachment #8747424 -
Flags: review?(mozilla)
|
|
||
Comment 12•9 years ago
|
||
Comment on attachment 8747424 [details] [diff] [review]
patch v2
Review of attachment 8747424 [details] [diff] [review]:
-----------------------------------------------------------------
Perfect.
There is a problem with hg qimport:bzNNNNNN. That seems to replace the CR+LF with LF only. Grrr. Not useful.
I had to download the patch manually. Anyway, it's good, you have CR+LF where it is needed.
Attachment #8747424 -
Flags: review?(mozilla) → review+
|
|
Assignee | |
Comment 13•9 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
OS: Unspecified → All
Hardware: Unspecified → All
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 49.0
|
|
||
Updated•9 years ago
|
|
|
||
Comment 14•9 years ago
|
||
Comment on attachment 8747424 [details] [diff] [review]
patch v2
[Approval Request Comment]
Regression caused by (bug #): bug 584313
User impact if declined: Auto downgrade not working for e-mail with signatures (quite BAD).
Testing completed (on c-c, etc.): Manually and with Mozmill test.
Risk to taking this patch (and alternatives if risky):
Very low, very small and well understood change.
Attachment #8747424 -
Flags: approval-comm-esr45?
Attachment #8747424 -
Flags: approval-comm-beta?
Attachment #8747424 -
Flags: approval-comm-aurora+
|
|
||
Comment 15•9 years ago
|
||
Aurora (TB 48):
https://hg.mozilla.org/releases/comm-aurora/rev/ebdf0edc4207
|
||
Comment 16•9 years ago
|
||
Comment on attachment 8747424 [details] [diff] [review]
patch v2
http://hg.mozilla.org/releases/comm-beta/rev/df1edca8fd93
http://hg.mozilla.org/releases/comm-esr45/rev/ff383777e10e
Attachment #8747424 -
Flags: approval-comm-esr45?
Attachment #8747424 -
Flags: approval-comm-esr45+
Attachment #8747424 -
Flags: approval-comm-beta?
Attachment #8747424 -
Flags: approval-comm-beta+
|
|
||
Updated•9 years ago
|
|
||
Updated•8 years ago
|
Blocks: delivery-format-ux
You need to log in
before you can comment on or make changes to this bug.
Description
•