Closed
Bug 762413
Opened 13 years ago
Closed 13 years ago
Signature placed above quote in plain text, entire message body is considered signature
Categories
(MailNews Core :: Composition, defect)
Tracking
(thunderbird13+ fixed, thunderbird14+ fixed, thunderbird15+ fixed)
RESOLVED
FIXED
Thunderbird 16.0
People
(Reporter: toni.fandos, Assigned: mconley)
Details
(Keywords: regression)
Attachments
(2 files, 1 obsolete file)
17.16 KB,
image/png
|
Details | |
4.40 KB,
patch
|
Bienvenu
:
review+
Bienvenu
:
approval-comm-aurora+
Bienvenu
:
approval-comm-beta+
standard8
:
approval-comm-release+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:12.0) Gecko/20100101 Firefox/12.0
Build ID: 20120420145725
Comment 1•13 years ago
|
||
I'm also seeing this in Thunderbird 13 on Windows XP. I don't recall seeing it in Thunderbird 12.
To be clear, this is when composing an email, and only occurs if composing as plain text, not as HTML.
Comment 2•13 years ago
|
||
I have just noticed a possible data loss situation related to this bug, if you have multiple accounts in Thunderbird, or multiple Identities.
If you compose an email while in the state described in this bug with the body text in grey, then change the From address before sending, Thunderbird will wipe out all the grey body text. (Presumably it does this because it thinks it is part of the signature, and different accounts/identities can have different signatures.) This would be particularly destructive if the user changes the From address after composing a long email. Many users would not know how to recover from this situation, although luckily Ctrl+Z does bring back the text.
By the way, I expect the Component for this bug should be Message Compose Window, not Account Manager.
I don't see this on current trunk builds. There have been major changes in plain-text composition introduced with Thunderbird 13.0 by the new "Filelink" feature (bug 698925), thus adding some people for whom this may ring a bell.
Did you test without add-ons activated? Help > Restart with Add-ons Disabled
If you have signature-related extensions installed they may no longer work properly after those changes.
Component: Account Manager → Message Compose Window
QA Contact: account-manager → message-compose
Ok, you need to set "reply above the quote" and "signature below my reply". Apparently the entire message is considered signature, even the part above dash-dash-space signature separator.
Also note that with this combination, there is no longer a blank line at the beginning of the composition, thus one has to add a new line manually which may separate the signature separator from the related HTML tag.
I am looking at a 20120603 15.0a1 build on Windows 7.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
Summary: Thunderbird 13. Signature attached at top of message body. In plain text, makes message body text appear in ligth grey, instead of black → Signature placed above quote in plain text, entire message body is considered signature
Assignee | ||
Comment 5•13 years ago
|
||
(In reply to rsx11m from comment #4)
> Ok, you need to set "reply above the quote" and "signature below my reply".
> Apparently the entire message is considered signature, even the part above
> dash-dash-space signature separator.
>
> Also note that with this combination, there is no longer a blank line at the
> beginning of the composition, thus one has to add a new line manually which
> may separate the signature separator from the related HTML tag.
>
> I am looking at a 20120603 15.0a1 build on Windows 7.
I've set reply above the quote, and signature below the reply, and I'm not seeing this regression...
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20120604 Thunderbird/14.0a2
Does anybody have a minimal set of steps to reproduce?
Mike: This may be related to what you mentioned in bug 756500 comment #9, I failed to notice that the fix for this was backed out again on trunk.
Minimum steps to reproduce:
1. Default to plain-text composition
2. Reply above the quote
3. Signature below my reply
4. Click "Write", no empty line before signature
5. Place cursor before signature seperator
6. Introduce manually a new line
7. Go back to the first line and enter text
8. New text typed appears gray as if it was signature
To reproduce comment #2, set up a 2nd identity with identical options.
Comment 7•13 years ago
|
||
(In reply to rsx11m from comment #3)
> Did you test without add-ons activated? Help > Restart with Add-ons Disabled
I still observe this after restarting without add-ons activated.
(In reply to Mike Conley (:mconley) from comment #5)
> Does anybody have a minimal set of steps to reproduce?
In TB 13, from a brand new TB profile:
1, Instead of gandi.net or hover.com, "Skip this and use my existing email"
2, Setup access to email (in my case, IMAP to a MS Exchange server, but I don't think this matters)
3, Tools, Account Settings
4, Fill in anything in the Signature box, e.g. "test"
5, Composition & Addressing:
a, Untick "Compose messages in HTML format"
b, Tick "Automatically quote the original message when replying"
c, Choose "start my reply above the quote"
d, and place my signature "below my reply (above the quote)"
6, Click OK
7, Click Write
8, Tab into the message body and observe the problem
It seems odd to me that the settings about where to put the signature when replying are relevant to this, as the problem is observed when composing a brand new message, not a reply.
Assignee | ||
Comment 8•13 years ago
|
||
Ah, yes, reproduced. Thank you.
tracking-thunderbird14:
--- → ?
tracking-thunderbird15:
--- → ?
Assignee | ||
Updated•13 years ago
|
Severity: normal → major
Assignee | ||
Comment 9•13 years ago
|
||
Neil:
I have a feeling this affects SeaMonkey as well. Can you confirm?
-Mike
Comment 10•13 years ago
|
||
Yes, it's core code thus should affect SeaMonkey as well. However, there is no special color for signatures during composition, hence one wouldn't notice.
Assignee | ||
Comment 11•13 years ago
|
||
Alright, I'll take a crack at this this afternoon.
Assignee: nobody → mconley
Comment 12•13 years ago
|
||
(In reply to rsx11m from comment #4)
> Also note that with this combination, there is no longer a blank line at the
> beginning of the composition, thus one has to add a new line manually which
> may separate the signature separator from the related HTML tag.
I can reproduce this part on a recent SeaMonkey 2.11 build, thus MailNews Core.
Assignee: mconley → nobody
Component: Message Compose Window → Composition
Product: Thunderbird → MailNews Core
QA Contact: message-compose → composition
Comment 13•13 years ago
|
||
That's what I see with DOM Inspector in the composition window:
BODY
DIV class=moz-signature
#text (message text)
BR
#text "-- " (signature separator)
BR
#text (signature text)
BR
BR (13.0 build only)
Assignee | ||
Comment 14•13 years ago
|
||
This patch seems to fix the issue for me. Can one of your fine folks try it out for me while I work on some tests?
Comment 15•13 years ago
|
||
This should resolve the issue of the missing blank line with the signature above the quote setting, but I'm actually able to squeeze a #text node between the <div> and the signature separator in any mode (i.e., also when the signature is place at the bottom) by just placing the cursor before the "-- " and starting to type, then creating a new line. This also works in HTML mode, the text entered goes into the <pre class=moz-signature> part before the "-- " at least with a plain-text signature.
I'm wondering if there is any way to prevent adding anything between the <div>/<pre> opening tag and the #text node for the signature separator, but this appears to be tricky as the "-- " effectively would have to be part of the <div>/<pre> construct somehow or otherwise would have to be ensured to always be the first child of that node.
> else if (htmlEditor)
So, this doesn't apply to the plain-text editor or is plain-text editor now the HTML editor just being restricted to accept plain-text activity only? Apparently I'm confusing this with the aHTMLEditor parameter passed into nsMsgCompose::ConvertAndLoadComposeWindow()...
Given that the observed behavior of being able to squeeze something as a first child to the <div>/<pre> node existed before already, even though just restricted to HTML composition, ensuring that there is an empty line at the begin of the document where the user should start typing is probably sufficient to avoid running into this issue, and the patch seems to do that.
Assignee | ||
Comment 16•13 years ago
|
||
(In reply to rsx11m from comment #15)
> > else if (htmlEditor)
>
> So, this doesn't apply to the plain-text editor or is plain-text editor now
> the HTML editor just being restricted to accept plain-text activity only?
> Apparently I'm confusing this with the aHTMLEditor parameter passed into
> nsMsgCompose::ConvertAndLoadComposeWindow()...
It's a bit strange. We can query the editor to be both an HTML editor and a text editor, but aHTMLEditor is a boolean for whether or not we should be *considered* a HTML editor. Bleh.
>
> Given that the observed behavior of being able to squeeze something as a
> first child to the <div>/<pre> node existed before already, even though just
> restricted to HTML composition, ensuring that there is an empty line at the
> begin of the document where the user should start typing is probably
> sufficient to avoid running into this issue, and the patch seems to do that.
Cool, thanks - still writing tests...
Assignee | ||
Comment 17•13 years ago
|
||
Added a regression test, and hardened one of our other tests to check for the initial BR node under default compose circumstances.
Try builds are coming in here: https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=58dc6e74c993
Attachment #631114 -
Attachment is obsolete: true
Attachment #631390 -
Flags: review?(dbienvenu)
Comment 18•13 years ago
|
||
ugh, apologies, sorry for the delay, mozmill tests are crashing for me, unrelated to your change.
Comment 19•13 years ago
|
||
Comment on attachment 631390 [details] [diff] [review]
Patch v2 with regression test
r+ based on try build mozmill successes...
Attachment #631390 -
Flags: review?(dbienvenu) → review+
Comment 20•13 years ago
|
||
Don't want to muddy the waters on this one.
But in the case of pop3 reply.
5, Composition & Addressing:
a, Untick "Compose messages in HTML format"
b, Tick "Automatically quote the original message when replying"
c, Choose "start my reply above the quote"
d, and place my signature "below my reply (above the quote)"
The -- delimiter is not included in the sig
xref bug https://bugzilla.mozilla.org/show_bug.cgi?id=62429#c296
I don't recall that being specifically changed.
Comment 21•13 years ago
|
||
Sorry about the extraneous comment above.
I somehow thought this was about replying, not original composition.
Comment 22•13 years ago
|
||
The issue depends on the reply settings even if observed with a new message. And, no "-- " present when putting the signature above the quote is expected behavior for replies.
I get two empty lines on top of the signature, though: The first one (with the cursor) is part of the text, the second line (below the cursor) already within the <div>, thus a similar behavior /could/ be observed when typing anything into the 2nd line. However, either case requires some active interaction by the user to be encountered, and (at least on Thunderbird) there is user feedback given by graying out any text that accidentally makes it into the <div> for moz-signature, thus I'd consider it not perfect but manageable.
Comment 23•13 years ago
|
||
Mike, is this ready to land on comm-central? It would be nice to get this into a 13.0.1 (if there is one) given that the scenario of top-poster/plain-text user is likely not an isolated one.
Assignee | ||
Comment 24•13 years ago
|
||
Comment on attachment 631390 [details] [diff] [review]
Patch v2 with regression test
I don't think this is too risky to take for aurora or beta, but I'll let the powers that be decide.
Attachment #631390 -
Flags: approval-comm-beta?
Attachment #631390 -
Flags: approval-comm-aurora?
Assignee | ||
Comment 25•13 years ago
|
||
comm-central: https://hg.mozilla.org/comm-central/rev/7447698d6473
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 16.0
Comment 26•13 years ago
|
||
Comment on attachment 631390 [details] [diff] [review]
Patch v2 with regression test
sure, we've got several weeks to catch any regressions.
Attachment #631390 -
Flags: approval-comm-beta?
Attachment #631390 -
Flags: approval-comm-beta+
Attachment #631390 -
Flags: approval-comm-aurora?
Attachment #631390 -
Flags: approval-comm-aurora+
Assignee | ||
Comment 27•13 years ago
|
||
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/7b9e46c36e2f
comm-beta: https://hg.mozilla.org/releases/comm-beta/rev/97daafffb827
If we roll a 13.0.1, do we want to take this?
status-thunderbird13:
--- → affected
status-thunderbird14:
--- → fixed
status-thunderbird15:
--- → fixed
tracking-thunderbird13:
--- → ?
Comment 28•13 years ago
|
||
I think it would be OK to take this for 13.0.1 as a ride-along.
Comment 29•13 years ago
|
||
Comment on attachment 631390 [details] [diff] [review]
Patch v2 with regression test
[Triage Comment]
I think we should be safe enough to take this for a 13.0.1 as well. As the risk area is a small set of use cases.
Attachment #631390 -
Flags: approval-comm-release+
Comment 30•13 years ago
|
||
Fixed in 13.0.1:
https://hg.mozilla.org/releases/comm-release/rev/1085f63aa2f3
Comment 31•13 years ago
|
||
Although the bug as described in comment #7 is now fixed in TB 13.0.1, there are some variations of this bug still present, at least in TB 13.0.1 on Windows. I wasn't sure whether to file these as new bugs or not, but I have decided to describe the new variants in this bug as they are so similar to the original bug as described in comment #7. I think they are probably from the same original problem which was introduced into TB 13 as I don't recall seeing them prior to 13 or 13.0.1 (although I don't have an easy way to check earlier versions). If instead these should be reported as new bugs, let me know.
Steps to reproduce variant number 1:
In TB 13.0.1, from a brand new TB profile:
1, Click "Skip this and use my existing email" and setup access to email (in my case, IMAP to a MS Exchange server, but I don't think this matters)
2, Tools, Account Settings
3, Important: Leave the signature box blank
4, Composition & Addressing:
a, Untick "Compose messages in HTML format"
b, Tick "Automatically quote the original message when replying"
c, Choose "start my reply above the quote"
d, and place my signature "below my reply (above the quote)"
5, Click OK, then go into Tools, Account Settings again (important to do this rather than just clicking on the account name in the top-left, as you want the identity that you're just about to setup to have the same composition settings as the main account)
6, Click Manage Identities, then Add
7, Fill in a new name and email address (doesn't really matter what, but presumably must be different to the other email address)
8, Fill in "test" in the signature
9, OK, Close
10, Click Write
11, Change the From address to the one that you made in the additional identity
12, Tab into the message body and observe that there are no blanks lines above the signature, and that message body text placed above the signature is grey
The important thing about the above variant is that the problem only shows up if the main account has a blank signature, and an additional identity has a non-blank signature. (Incidentally, you will also notice while doing step 12 the appearance of bug 118050 - "Caret (cursor) is positioned after the signature if you change the identity (From: address)").
Steps to reproduce variant number 2:
In TB 13.0.1, from a brand new TB profile:
1, Click "Skip this and use my existing email" and setup access to email (in my case, IMAP to a MS Exchange server, but I don't think this matters)
2, Tools, Account Settings
3, Fill in "test" in the signature box, *or* leave blank (bug occurs with either)
4, Composition & Addressing:
a, Untick "Compose messages in HTML format"
b, Tick "Automatically quote the original message when replying"
c, Choose "start my reply above the quote"
d, and place my signature "below my reply (above the quote)"
5, Click OK, then go into Tools, Account Settings again (important to do this rather than just clicking on the account name in the top-left, as you want the identity that you're just about to setup to have the same composition settings as the main account)
6, Click Manage Identities, then Add
7, Fill in a new name and email address (doesn't really matter what, but presumably must be different to the other email address)
8, Fill in "test 2" in the signature box
9, OK, Close, OK
10, Find a message in your inbox or a folder that has been sent to the main email address (not to the identity email address)
11, Click Reply
12, Change the From address to the one that you made in the additional identity
13, Tab into the message body and observe that although there is one blank line above the signature, any message body text placed above the signature is grey
To re-iterate, I'm pretty sure variant 1 didn't exist in TB 12 or earlier so was probably new in TB 13 or TB 13.0.1. I'm more certain in thinking that variant 2 may have been introduced in TB 13.0.1, that is, was not present in TB 13.
Comment 32•13 years ago
|
||
Hi John, please can you file new bugs for your issues, commenting on closed bugs doesn't let us track your issues as well and they may get lost.
Comment 33•13 years ago
|
||
Thanks for letting me know the correct thing to do. I have filed these as bug 766520 and bug 766521.
You need to log in
before you can comment on or make changes to this bug.
Description
•