Last Comment Bug 756500 - Remove extraneous newline after signature in compose window
: Remove extraneous newline after signature in compose window
Status: RESOLVED FIXED
: regression
Product: MailNews Core
Classification: Components
Component: Composition (show other bugs)
: unspecified
: All All
: -- normal (vote)
: Thunderbird 15.0
Assigned To: Mike Conley (:mconley) - (Needinfo me!)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-18 10:06 PDT by Mike Conley (:mconley) - (Needinfo me!)
Modified: 2012-07-21 00:27 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
fixed
fixed


Attachments
DOMI showing Editor with empty lines below signature (10.85 KB, image/png)
2012-05-18 23:11 PDT, Alfred Peters
no flags Details
Patch v1 (4.17 KB, patch)
2012-05-31 11:32 PDT, Mike Conley (:mconley) - (Needinfo me!)
mozilla: review+
mozilla: approval‑comm‑aurora+
Details | Diff | Review
Patch v2 (6.74 KB, patch)
2012-06-01 08:18 PDT, Mike Conley (:mconley) - (Needinfo me!)
mozilla: review+
mozilla: approval‑comm‑aurora+
Details | Diff | Review

Description Mike Conley (:mconley) - (Needinfo me!) 2012-05-18 10:06:29 PDT
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.
Comment 1 rsx11m 2012-05-18 15:27:59 PDT
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.
Comment 2 rsx11m 2012-05-18 15:29:09 PDT
(sorry, I didn't mean to remove you as the assignee, just to move that into MailNews Core where it belongs...)
Comment 3 Alfred Peters 2012-05-18 23:11:19 PDT
Created attachment 625354 [details]
DOMI showing Editor with empty lines below signature
Comment 4 Alfred Peters 2012-05-18 23:18:28 PDT
(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
Comment 5 Alfred Peters 2012-05-18 23:31:35 PDT
(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.
Comment 6 Hartmut Figge 2012-05-18 23:59:47 PDT
(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.
Comment 7 Alfred Peters 2012-05-19 02:13:16 PDT
(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.
Comment 8 rsx11m 2012-05-19 08:03:47 PDT
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.
Comment 9 Mike Conley (:mconley) - (Needinfo me!) 2012-05-31 11:32:18 PDT
Created attachment 628831 [details] [diff] [review]
Patch v1

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.
Comment 10 David :Bienvenu 2012-05-31 12:47:45 PDT
Comment on attachment 628831 [details] [diff] [review]
Patch v1

[Triage Comment]

yes, I like this better, for sure.
Comment 11 Mike Conley (:mconley) - (Needinfo me!) 2012-05-31 12:58:51 PDT
comm-central: https://hg.mozilla.org/comm-central/rev/a74d7096d8fd
comm-aurora: https://hg.mozilla.org/releases/comm-aurora/rev/0c9f25d8e8bb
Comment 12 David :Bienvenu 2012-05-31 12:59:51 PDT
do you think we want this for beta? I'm inclined to say no, but open to argument.
Comment 13 Mike Conley (:mconley) - (Needinfo me!) 2012-05-31 13:10:16 PDT
(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.
Comment 14 Mike Conley (:mconley) - (Needinfo me!) 2012-05-31 14:03:51 PDT
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.
Comment 15 Mike Conley (:mconley) - (Needinfo me!) 2012-06-01 08:18:29 PDT
Created attachment 629197 [details] [diff] [review]
Patch v2

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
Comment 16 David :Bienvenu 2012-06-01 13:03:26 PDT
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.
Comment 17 David :Bienvenu 2012-06-01 13:11:48 PDT
landed on trunk - http://hg.mozilla.org/comm-central/rev/ebe999a8371e
Comment 18 Mike Conley (:mconley) - (Needinfo me!) 2012-06-01 13:14:15 PDT
(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.
Comment 19 Mike Conley (:mconley) - (Needinfo me!) 2012-06-07 08:27:33 PDT
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
Comment 20 mgmead 2012-07-20 22:57:21 PDT
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.
Comment 21 Jim Porter (:squib) 2012-07-21 00:27:07 PDT
(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.

Note You need to log in before you can comment on or make changes to this bug.