929006 - Thunderbird signature detection is recursive (nested plain-text messages)

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Kevin Stange]

Attachment: [CentOS-announce] Updates for CentOS-5.10 Release.eml

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/31.0.1650.26 Safari/537.36

Steps to reproduce:

I received a very long message which contains a lot of copy and pasted email messages with signature separators ("--").


Actual results:

While scrolling through the message, every time Thunderbird encounters '--' it makes the text lighter by about 10%, until suddenly I'm viewing very light gray text on white, then even white text on a white background and the remainder of the email is unreadable.  I can't even highlight the text to read it at this point.

Email that 


Expected results:

Thunderbird should process "--" only once so that the text never grays below a suitable contrast level against the background.

Comment 1

[Kevin Stange]

Nevermind.  #917906 seems to resolve this problem.

Comment 2

[rsx11m]

Comment on attachment 819812 [details]
[CentOS-announce] Updates for CentOS-5.10 Release.eml

Note: This is a plain-text message, thus different from bug 977906 which specifically addresses HTML messages only.

Comment 3

[rsx11m]

So, in this case we have a message which is added to another message after the signature of that message, thus becoming part of the signature of the first message itself. The entire quoted message is displayed as signature (60% opacity). If that message has a signature on its own, the opacity rule is applied again (now at 36%). The 3rd message is considered part of the signature of the 2nd message, and if in turn it contains a signature, the opacity rule is applied a 3rd time (resulting in 21.6% opacity), etc.

Thus, the rule should only be applied to the first .moz-sig-txt encountered, not to all of them.

Reopening and confirming.

Comment 4

[rsx11m]

Attachment: Proposed patch

This patch fixes the issue for me with the testcase provided. While still leaving the text after the "-- " delimiter grayed (as it should, plain-text signature after a proper delimiter), the opacity is applied only once rather than every time another "-- " is met in the plain-text message, thus still allowing to read the text without any problems.

The plain-text case is simpler than the nested HTML-signature case described in bug 917906 comment #4, given that Thunderbird itself creates the HTML structure from the message and is linear by nature.

For each text/* MIME part, there is a <div> on top with one of the following classes (which don't appear anywhere else):

(1) moz-text-flowed: "text/plain; format=flowed" is the default plain-text mode Thunderbird is using. In this case, div.moz-txt-sig is a direct child node of div.moz-text-flowed and can be easily addressed directly. All subsequent div.moz-txt-sig underneath the top-level div.moz-txt-sig are ignored.

(2) moz-text-plain: "text/plain" only, in which case an additional <pre> node is added that needs to be accounted for. Note that for (1) and (2), quotes are put into blockquote nodes but leave the hierarchy intact for the div.moz-txt-sig node, and original signatures are removed when replying.

(3) moz-text-html: "text/html" which is trivial given that HTML signatures are no longer styled and pre.moz-signature can't nest (and won't appear in plain-text parts). Thus, no change necessary for the existing rule.

Comment 5

[Richard Marti (:Paenglab)]

Comment on attachment 821953 [details] [diff] [review]
Proposed patch

Review of attachment 821953 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM with the limited set of messages I can test here. Let's hope this catches all possibilities of signatures :)

r=me

Comment 6

[rsx11m]

Thanks, push for comm-central please.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/96fccf2c4bf1

Comment 8

[rsx11m]

Attachment: Patch for comm-esr24

I'll request branch approvals after the 24.1 release.

Comment 9

[rsx11m]

Comment on attachment 821953 [details] [diff] [review]
Proposed patch

[Approval Request Comment]
Regression caused by (bug #): bug 855135
User impact if declined: Digest-type plain-text messages may be hard to read
Testing completed (on c-c, etc.): tested on comm-central (now comm-aurora)
Risk to taking this patch (and alternatives if risky): low

Comment 10

[rsx11m]

Comment on attachment 822946 [details] [diff] [review]
Patch for comm-esr24

[Approval Request Comment]
See comment #9.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/comm-beta/rev/560d8f11186c

Comment 12

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-esr24/rev/8cf506f3c273