email without HTML elements is sent as HTML, despite "Delivery Format: Auto-detect" option (since version 45 update)

RESOLVED FIXED in Thunderbird 49.0

Status

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: claypigeon, Assigned: aceman)

Tracking

(Blocks 1 bug, {regression})

45 Branch
Thunderbird 49.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird46 wontfix, thunderbird47 fixed, thunderbird48 fixed, thunderbird49 fixed, thunderbird_esr4546+ fixed)

Details

(Whiteboard: [regression:TB45])

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

3 years ago
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

3 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
Ever confirmed: true
Flags: needinfo?(acelists)
Keywords: regression

Comment 2

3 years ago
Wayne, did I understand you correctly that you want to hear about all regressions?

Comment 3

3 years ago
What about moz-forward-container? You need to add moz-signature and moz-forward-container. Two line fix then.
(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]
Assignee

Comment 5

3 years ago
Strange, I thought signatures are covered by that patch.
What about we make it class=moz-* ?
Flags: needinfo?(acelists)

Comment 6

3 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.
Assignee

Comment 7

3 years ago
(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.
Assignee

Comment 8

3 years ago
Posted patch patch (obsolete) — Splinter Review
So this is what we discussed. But I think it does not make forwards downgrade.
Assignee: nobody → acelists
Status: NEW → ASSIGNED
Attachment #8747322 - Flags: review?(mozilla)

Comment 9

3 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

3 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

3 years ago
Posted patch patch v2Splinter Review
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

3 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

3 years ago
Thanks.
https://hg.mozilla.org/comm-central/rev/c1bf457da82c5fad5936c007be9a73f94d461d18
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
OS: Unspecified → All
Hardware: Unspecified → All
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 49.0

Comment 14

3 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+

Updated

2 years ago
You need to log in before you can comment on or make changes to this bug.