167319 - [RFE] "Don't add signature to replies and forwards" option in preferences

Status: RESOLVED FIXED

(MailNews Core :: Composition, enhancement)

Description

[Dimitrios]

A nice feature of Outlook Express: it has a "Don't add signatures to replies &
forwards" setting. Why don't we add a similar setting in our preferences ?
(together with the ability to add signature on request, which is bug 147854)

It seems like a partial alternative to the infamous bug 62429 or bug 141531,
which no one at mozilla.org is willing to fix. I am explaining why this rfe
might be useful:
1. When you know the person you answer, it isn't elegant to add that clutter in
your mail. Especially when there are more than one replies in the message.
2. International users might want to use signatures written in their local
languages. But, when replying in another (non-local) language, you obviously
must ommit the signature. Especially when replying above, since the signature
might be buried many lines below. It's too easy to forget erasing it (I fell
into this many times in my business mail replies). Nevertheless, erasing is time
consuming.
3. Newsgroup and mailing list users might want to preserve their anonymity. Why
forcing them to erase their signatures? Or exposing them in the risk of
forgetting to erase it?

Well, I'm not an optimist. This rfe will probably end wontfixed (or duped but I
didn't find something similar in Bugzilla).

Comment 1

[Jean-Francois Ducarroz]

don't be pessimist! sound to be a good idea. The only problem is time...
-->varada

Comment 2

[(not reading, please use [e-mail] instead)]

taking all of varada's bugs.

Comment 3

[Bruce Davidson]

*** Bug 215247 has been marked as a duplicate of this bug. ***

Comment 4

[Pedro Madeira]

I'm voting for this since this is very easy to do
and it makes all the sense in the world for corporate users.

Also I suggest that someone suficiently empowered should change
the OS to all since this happens in all mozilla mail clients.

Comment 5

[Malte Rücker (mbr)]

*** Bug 186511 has been marked as a duplicate of this bug. ***

Comment 6

[Robert Mohr]

*** Bug 226433 has been marked as a duplicate of this bug. ***

Comment 7

[Robert Mohr]

From the last dupe, this would be especially useful when replying to digests.

Comment 8

[Peep Chaintreuil]

I actually wouldn't mind getting options similar to those that solved the
"infamous" bugs that were mentioned in the original description.  I actually
want the signature, I just want it before the forwarded message.

Comment 9

[Martin Brunthaler]

Attachment: Proposed fix for this RFE

This patch adds a checkbox to the Account Prefs UI which allows the user
to choose whether to attach a signature to quoted messages or not.

Comment 10

[Mike Cowperthwaite]

*** Bug 214763 has been marked as a duplicate of this bug. ***

Comment 11

[Adam Guthrie]

*** Bug 314443 has been marked as a duplicate of this bug. ***

Comment 12

[Rob Hasselbaum]

A patch was provided for this over two years ago. Can this go in?

Comment 13

[David :Bienvenu]

Comment on attachment 151286 [details] [diff] [review]
Proposed fix for this RFE

reviews must be requested and completed before any patch can land...this patch languished because no one ever requested a review :-(

I'm not sure we want a UI for this, but the composition and addressing page does have plenty of space at the moment.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

*** Bug 362974 has been marked as a duplicate of this bug. ***

Comment 15

[frederico pflug]

a lot of amazing features coming into new versions. can't this come together with the TB release?

Comment 18

[Willie]

This is ridiculous.
<rant>This was first posted in 2002.  That was 5 years ago!  Someone even posted a patch in 2002 for it, but apparently it was never "reviewed".  How do I request it to be "reviewed"?  What's the point of having bugzilla or a similar system if it takes 5 years for something to happen (or in this case, nothing)?
This is like me hoping that Microsoft will improve their IMAP support in Outlook!</rant>
On the other hand, I do hold the highest respect for the developers.  They are working on all this without pay, so I can't really expect them to do anything that they don't feel like doing.  It would just be nice if we had a little more throughput in certain areas.
A similar RFE that has been waiting since 2004: Bug 273114

Comment 19

[Stefan]

I came here to request this feature also. Just fix it! I started using Thunderbird yesterday and this is an annoyance when you have been using Outlook Express for 7 years before.

Comment 20

[Willie]

(In reply to comment #18)
> This is ridiculous.

I'd like to edit or rescind my previous comment (#18).  I think it was just a bad day.  I understand the developers are under no obligation to do anything they do not want to.

I hope it didn't hurt this bug report.  Perhaps I just have to learn how to program more. 

Comment 21

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Moving from wanted‑thunderbird3.0a1? flag to wanted‑thunderbird3.0a2? flag since code for Thunderbird 3 Alpha 1 has been frozen.

Comment 22

[Tony]

Yeah why not add this into upcoming tb3?

Comment 23

[Wayne Mery (:wsmwk)]

adding bryan to the mix to comment. 

Comment 24

[Wayne Mery (:wsmwk)]

Comment on attachment 151286 [details] [diff] [review]
Proposed fix for this RFE

mscott no longer actively reviewing, so canceling review

once a direction is (re)decided someone can take up the torch on this request

Comment 25

[Worcester12345]

Can you re-request the review please?

Comment 26

[Wayne Mery (:wsmwk)]

need a patch owner first.
also need UI review.

Comment 28

[Bryan Clark (DevTools PM) [:clarkbw]]

This seems like a good idea to me, probably should default to being on.

Is anyone willing to work on the patch?

Comment 29

[Magnus Melin [:mkmelin]]

Taking.

Comment 30

[Ivica]

This is big problem for business use, I think feature should be implemented as soon as possible. Thanks.

Comment 31

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Comment on attachment 151286 [details] [diff] [review]
Proposed fix for this RFE

Patch has bitrotted.

$ patch  --dry-run < ~/Desktop/tbTestPatches/patch_for_167319.diff 
(Stripping trailing CRs from patch.)
patching file am-addressing.js
Hunk #1 succeeded at 95 with fuzz 1 (offset -8 lines).
(Stripping trailing CRs from patch.)
patching file am-addressing.xul
Hunk #1 FAILED at 89.
1 out of 1 hunk FAILED -- saving rejects to file am-addressing.xul.rej
(Stripping trailing CRs from patch.)
can't find file to patch at input line 84
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|Index: mailnews/base/public/nsIMsgIdentity.idl
|===================================================================
|RCS file: /cvsroot/mozilla/mailnews/base/public/nsIMsgIdentity.idl,v
|retrieving revision 1.33
|diff -u -8 -p -r1.33 nsIMsgIdentity.idl
|--- mailnews/base/public/nsIMsgIdentity.idl    17 Apr 2004 18:32:15 -0000      1.33
|+++ mailnews/base/public/nsIMsgIdentity.idl    20 Jun 2004 13:10:39 -0000
--------------------------
File to patch:

Comment 32

[David Ascher (:davida)]

This would not block a release, but work on the patch would be great.

Comment 34

[Magnus Melin [:mkmelin]]

Attachment: proposed fix

Defaulting to use signature for replies but not for forwards.
Screen shot coming up.

Comment 35

[Magnus Melin [:mkmelin]]

Attachment: screenshot

Comment 36

[[e-mail]]

Comment on attachment 371490 [details] [diff] [review]
proposed fix

Can you point me to the bug for redirect, so I can decide whether I agree with your signature decision?

If you change your identity in the Compose window then you'll always get a new signature; is that likely to be a problem?

Comment 37

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 371490 [details] [diff] [review]
proposed fix

looks good to me from the screenshot

Comment 38

[Magnus Melin [:mkmelin]]

The redirect implementation was bug 359226. (AFAIK, it's just an easy way for forward + set reply-to.)

Comment 39

[[e-mail]]

Comment on attachment 371490 [details] [diff] [review]
proposed fix

One thing I did notice was this patch doesn't disable the reply/forward preference when the user has no signature (compare with the "place my signature" dropdown).

>diff --git a/mailnews/base/prefs/resources/content/am-addressing.js b/mailnews/base/prefs/resources/content/am-addressing.js
Part of some other patch?

>-    
>+
Nit: lots of random whitespace cleanup

>+  PRBool includeSignature;
Nit: prefer to init to PR_TRUE here, and don't bother with the default:

>+
>+    default:
Nit: inconsistent spacing

>-    if (!aSignature.IsEmpty() )
>+    if (includeSignature && !aSignature.IsEmpty())
Is it possible to avoid fetching the signature if we won't need it?

> <!ENTITY place.label                          "and place my signature">
> <!ENTITY place.accesskey                      "s">
> <!ENTITY belowText.label                      "below the quote (recommended)">
> <!ENTITY aboveText.label                      "below my reply (above the quote)">
>+<!ENTITY includeSigOnReply.label "Include signature for replies">
>+<!ENTITY includeSigOnReply.accesskey "s">
>+<!ENTITY includeSigOnForward.label "Include signature for forwards">
>+<!ENTITY includeSigOnForward.accesskey "w">
Nit: strings should align

Comment 40

[Magnus Melin [:mkmelin]]

(In reply to comment #36)
> (From update of attachment 371490 [details] [diff] [review])
> Can you point me to the bug for redirect, so I can decide whether I agree with
> your signature decision?

Redirect is not an issue in the next iteration.

> If you change your identity in the Compose window then you'll always get a new
> signature; is that likely to be a problem?

Fixed.

Comment 41

[Magnus Melin [:mkmelin]]

(In reply to comment #39)
> (From update of attachment 371490 [details] [diff] [review])
> One thing I did notice was this patch doesn't disable the reply/forward
> preference when the user has no signature (compare with the "place my
> signature" dropdown).

Yeah, I think the disabling is confusing and should go, plan to attack bug 277187 at some point.

> >diff --git a/mailnews/base/prefs/resources/content/am-addressing.js b/mailnews/base/prefs/resources/content/am-addressing.js
> Part of some other patch?

Drive-by cleanup, have removed that from the patch...

> Nit: lots of random whitespace cleanup

Not a bug ;)

> Is it possible to avoid fetching the signature if we won't need it?

Fixed.

Comment 42

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v2

Carrying fwd ui-r=clarkbw

Comment 43

[[e-mail]]

Comment on attachment 371826 [details] [diff] [review]
proposed fix, v2

And to think I'd decided to go with your decision on signing redirects too...

Comment 44

[David :Bienvenu]

Comment on attachment 371826 [details] [diff] [review]
proposed fix, v2

this patch has bit-rotted. In general, it looks good, but I'd like to try running it. Do you have a non-bitrotted version?

Comment 45

[Magnus Melin [:mkmelin]]

Attachment: proposed fix, v3

Unbitrotted.

Comment 46

[David :Bienvenu]

Comment on attachment 373362 [details] [diff] [review]
proposed fix, v3

thx, Magnus.

Comment 47

[David :Bienvenu]

Comment on attachment 373362 [details] [diff] [review]
proposed fix, v3

one thought, however, is to use the doxygen /// comment. format for the one line comments. There's a possibility the comment needs to end with a "." for doxygen to pick it up, though I don't know if that was ultimately figured out or not.

Comment 48

[Magnus Melin [:mkmelin]]

I thought about that, but it looks a bit inconsistent in the file when some attributes still need more than one line for the comment, so it'd be mixed formats.

Comment 49

[Magnus Melin [:mkmelin]]

changeset:   2414:7f2485cc359a
http://hg.mozilla.org/comm-central/rev/7f2485cc359a

->FIXED