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

RESOLVED FIXED in Thunderbird 27.0

Status

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: bugzilla, Assigned: rsx11m.pub)

Tracking

({regression})

Trunk
Thunderbird 27.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird26 fixed, thunderbird27 fixed, thunderbird28 fixed, thunderbird_esr2426+ fixed)

Details

(Whiteboard: [gs], )

Attachments

(3 attachments)

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.
Nevermind.  #917906 seems to resolve this problem.
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 917906
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.
Attachment #819812 - Attachment mime type: application/x-extension-eml → text/plain
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.
Status: RESOLVED → REOPENED
Component: Message Reader UI → Theme
Ever confirmed: true
OS: Linux → All
Hardware: x86_64 → All
Resolution: DUPLICATE → ---
Summary: Thunderbird signature detection is recursive → Thunderbird signature detection is recursive (nested plain-text messages)
Version: 24 → Trunk
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.
Assignee: nobody → rsx11m.pub
Status: REOPENED → ASSIGNED
Attachment #821953 - Flags: review?(richard.marti)
See Also: → 917906
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
Attachment #821953 - Flags: review?(richard.marti) → review+
Thanks, push for comm-central please.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/96fccf2c4bf1
Status: ASSIGNED → RESOLVED
Closed: 6 years ago6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 27.0
I'll request branch approvals after the 24.1 release.
Attachment #822946 - Attachment is patch: true
Attachment #822946 - Attachment mime type: text/x-patch → text/plain
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
Attachment #821953 - Flags: approval-comm-beta?
Comment on attachment 822946 [details] [diff] [review]
Patch for comm-esr24

[Approval Request Comment]
See comment #9.
Attachment #822946 - Flags: approval-comm-esr24?
Attachment #821953 - Flags: approval-comm-beta? → approval-comm-beta+
Keywords: checkin-needed
Whiteboard: [gs] → [gs][c-n: comm-beta]
Attachment #822946 - Flags: approval-comm-esr24? → approval-comm-esr24+
Keywords: checkin-needed
Whiteboard: [gs] → [gs][c-n: comm-esr24]
You need to log in before you can comment on or make changes to this bug.