Closed Bug 756500 Opened 8 years ago Closed 8 years ago

Remove extraneous newline after signature in compose window

Categories

(MailNews Core :: Composition, defect)

defect
Not set

Tracking

(thunderbird14 fixed, thunderbird15 fixed)

RESOLVED FIXED
Thunderbird 15.0
Tracking Status
thunderbird14 --- fixed
thunderbird15 --- fixed

People

(Reporter: mconley, Assigned: mconley)

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Alfred pointed this out in https://bugzilla.mozilla.org/show_bug.cgi?id=698925#c250

We're adding a newline after the signature in the compose window here: http://hg.mozilla.org/comm-central/rev/43b791fd9ea4?revcount=100#l8.45

This doesn't appear to be necessary, and can be excised.
Does this affect HTML messages only? A plain-text message with a plain-text signature doesn't show any unnecessary blank line after the signature, comparing 12.0.1 vs. pre-12.0 versions.
Assignee: mconley → nobody
Component: Message Compose Window → Composition
Product: Thunderbird → MailNews Core
QA Contact: message-compose → composition
Hardware: x86 → All
(sorry, I didn't mean to remove you as the assignee, just to move that into MailNews Core where it belongs...)
Assignee: nobody → mconley
(In reply to rsx11m from comment #1)
> Does this affect HTML messages only? A plain-text message with a plain-text
> signature doesn't show any unnecessary blank line after the signature,

No, only plain-text messages.

attachment 625354 [details] shows news article 
<0LSdnXkefZ_8qirSnZ2dnUVZ_t-dnZ2d@mozilla.org> in mozilla.test
E-Mail are also effected.

> comparing 12.0.1 vs. pre-12.0 versions.

Mozilla/5.0 (Windows NT 5.1; rv:15.0) Gecko/15.0 Thunderbird/15.0a1
Application Build ID: 20120518101913
(In reply to Alfred Peters from comment #4)
> Mozilla/5.0 (Windows NT 5.1; rv:15.0) Gecko/15.0 Thunderbird/15.0a1
> Application Build ID: 20120518101913

A private Build without line <http://hg.mozilla.org/comm-central/rev/43b791fd9ea4?revcount=100#l8.45> doesn't show those 3 BR-Tags and so no empty lines at the end.
(In reply to Alfred Peters from comment #5)

> A private Build without line
> <http://hg.mozilla.org/comm-central/rev/43b791fd9ea4?revcount=100#l8.45>
> doesn't show those 3 BR-Tags and so no empty lines at the end.

I can confirm this for my own private build of a trunk SM.
(In reply to rsx11m from comment #1)
> signature doesn't show any unnecessary blank line after the signature,
> comparing 12.0.1 vs. pre-12.0 versions.

My last good is:
Mozilla/5.0 (Windows NT 5.1; rv:13.0) Gecko/2012031219 Thunderbird/13.0a1

My next version shows the effect of Bug 737014 and has the empty lines.
But after the Fix for Bug 737014 the empty lines still remain.
Stupid me, the filelink code didn't land before 13.0, thus 12.0 won't show it...

I can reproduce with plain-text composition in current 14.0a2 nightly builds, there's one additional line after the signature as reported.
Attached patch Patch v1 (obsolete) — Splinter Review
We needed the linebreak insertion in the case where the user was replying below a quote, with no signature, on an otherwise empty email.

I guess I was a bit too overzealous when I put that in, since we don't need it in the other cases. I've removed it, and put the special-case logic into cloudAttachmentLinkManager.js, where it probably belongs. I've also adjusted our test cases accordingly, since they were testing that we were adhering to this wrongness.
Attachment #628831 - Flags: review?(dbienvenu)
Comment on attachment 628831 [details] [diff] [review]
Patch v1

[Triage Comment]

yes, I like this better, for sure.
Attachment #628831 - Flags: review?(dbienvenu)
Attachment #628831 - Flags: review+
Attachment #628831 - Flags: approval-comm-aurora+
comm-central: https://hg.mozilla.org/comm-central/rev/a74d7096d8fd
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/0c9f25d8e8bb
Status: NEW → RESOLVED
Closed: 8 years ago
Keywords: regression
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 15.0
do you think we want this for beta? I'm inclined to say no, but open to argument.
(In reply to David :Bienvenu from comment #12)
> do you think we want this for beta? I'm inclined to say no, but open to
> argument.

I'm also going to say no, simply because I don't think we're rolling another beta. 

The bug is strange, but not debilitating. If it's really super annoying for people, and they can't wait for the fix, they can switch to EarlyBird (or beta once we merge over).

My 2c, anyhow.
Shoot - I think I just introduced a permanent orange:

https://tbpl.mozilla.org/php/getParsedLog.php?id=12247363&tree=Thunderbird-Trunk

Should have pushed to try. :/ Gonna revert on comm-central and comm-aurora and fix it tomorrow.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch Patch v2Splinter Review
Not only did I have to fix a test, but the signature-switching test was failing because we need a <br> node inserted *before* the signature in the plain-text editor.

Going to push this to try as well to avoid another orange snafu:  https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=6a2ccf42261c
Attachment #628831 - Attachment is obsolete: true
Attachment #629197 - Flags: review?(dbienvenu)
Comment on attachment 629197 [details] [diff] [review]
Patch v2

ok, mozmill composition and cloudfile tests pass, except for the .eml one, which is a known failure.
Attachment #629197 - Flags: review?(dbienvenu) → review+
Attachment #629197 - Flags: approval-comm-aurora?
Attachment #629197 - Flags: approval-comm-aurora? → approval-comm-aurora+
(In reply to David :Bienvenu from comment #17)
> landed on trunk - http://hg.mozilla.org/comm-central/rev/ebe999a8371e

Wrong bug for that one.  Gonna repost that in bug 760624.
Argh, this never got marked as closed.

This landed in comm-central / comm-aurora a day or so before the merge.

You can find this patch in:

https://hg.mozilla.org/comm-central/rev/f75feab874a5
https://hg.mozilla.org/releases/comm-aurora/rev/9185ea47b710
https://hg.mozilla.org/releases/comm-beta/rev/9185ea47b710
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
UGH.  This is exactly NOT the aesthetic that I (just one user) would want imposed.

I begin reply ABOVE quoted message and have no signature.

So when I start a reply, it used to look like this:

I'm going to type here and say neato things...

On 7/20/12 10:42PM, bugzilla-daemon@mozilla.org wrote:


That was great because then, since I don't have a signature, I could type:

Here is the thing I'm writing.

Thanks.

Malcolm<HIT SEND NOW>


This immediate return after one's name is a natural rhythm ... I type something, I say FROM ME! and send.  There was a natural whitespace between the end of the written message and the beginning of the included message ... it naturally created an easy break on the eyes... so if I was writing an email and walked off and came back I wouldn't see this:

I was writing this and went off to get coffee.
On 7/20/12 10:42PM, bugzilla-daemon@mozilla.org wrote:
blahblah blah and more blah.

What an ugly thing to potentially send to a customer by accident.  Or if you're using email quickly (as in during a fast back and forth)

Yes.

On 7/20/12 10:42PM, bugzilla-daemon@mozilla.org wrote:
blah blah blah

Works much better than:

Yes.
On 7/20/12 10:42PM, bugzilla-daemon@mozilla.org wrote:
blah blah blah and some more blah
because I wrote you before and made a long thing and then you replied with a long thing
and so forth.

Could we at LEAST expose this in a way as an option for users?

I tried putting a newline AS my signature ... but then I get:

I wrote this here.

--

On 7/20/12 10:42PM, bugzilla-daemon@mozilla.org wrote:
blahblah blah.


===================
I recognize that this is trivial -- but you've made a major aesthetic change with no user discussion ... I TRIED to find this thread and only got it when I looked at the bug fix list for version 14.

Blech and arg.  It's horrible.
(In reply to mgmead from comment #20)
> UGH.  This is exactly NOT the aesthetic that I (just one user) would want
> imposed.

This works fine for me in 14.0 and in nightly.
You need to log in before you can comment on or make changes to this bug.