Closed Bug 1268014 Opened 5 years ago Closed 5 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)

45 Branch
defect
Not set
normal

Tracking

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

RESOLVED FIXED
Thunderbird 49.0
Tracking Status
thunderbird46 --- wontfix
thunderbird47 --- fixed
thunderbird48 --- fixed
thunderbird49 --- fixed
thunderbird_esr45 46+ fixed

People

(Reporter: claypigeon, Assigned: aceman)

References

(Blocks 1 open bug)

Details

(Keywords: regression, Whiteboard: [regression:TB45])

Attachments

(1 file, 1 obsolete file)

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
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
Wayne, did I understand you correctly that you want to hear about all regressions?
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]
Strange, I thought signatures are covered by that patch.
What about we make it class=moz-* ?
Flags: needinfo?(acelists)
(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.
Attached 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 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 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+
Attached 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 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+
Thanks.
https://hg.mozilla.org/comm-central/rev/c1bf457da82c5fad5936c007be9a73f94d461d18
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
OS: Unspecified → All
Hardware: Unspecified → All
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 49.0
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+
You need to log in before you can comment on or make changes to this bug.