229044 - Don't add "-- " divider if signature already includes one

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[Christian Eyrich]

It's extremely annoying to have the signature delimitier automatically placed
before a signature file like

bye
-- 
some text

Two remedies:
1. enable the user to turn off the automatic delimiter
2. look not only at the first four bytes for an existing delimiter in the sig
but at the whole sig.

The first one is simple (except the prefs get stuff) but makes it possible for
users to add none or the wrong delimiter (I don't mind - if the user wants to
disgrace himself, he should -, but not few other users do).
The second one is more costly (string search) but fool-safe.

That's at least true for non-HTML-sigs as the classic delimiter doesn't work in
HTML (because of the trailing <br>).

Comment 1

[Christian Eyrich]

Attachment: proposed patch with automatic decision

This is No 2 I wrote about.
Searching for "-- \r" and "-- \n" would work to (and make the existing
firstFourChars.Equals ifs obsolete), but wouldn't be that save.

Comment 2

[David :Bienvenu]

Comment on attachment 137755 [details] [diff] [review]
proposed patch with automatic decision

seems OK. Asking mscott for sr unless I'm missing some reason why this wouldn't
be a good idea. If he sr's it, then I can check it in for you.

Comment 3

[Scott MacGregor]

Does this patch make it possible to not insert the delimter? If so, then I would
say we don't want this. Not having the standard delimter breaks mail clients
when they try to figure out where the signature is in the message.  

Or is this patch just making it such that if the signature file already contains
the delimter, we don't add it a second time?

Comment 4

[Christian Eyrich]

It only doesn't add it if it's already present in the sig file. That simply
extends the current behaviour where we don't add if the sig starts with the
separator.

Comment 5

[Mike Cowperthwaite]

Dupe of bug 113684?  Sure looks like it.

Comment 6

[Christian Eyrich]

Oh, you're right, Mike (I only searched for "delimiter" and "terminator" but not
"-- \n" before filing this bug) and it contains some good thoughts. But
shouldn't bug 113684 marked as a dupe because this one has a working patch?

Comment 7

[Christian Eyrich]

Attachment: another alternative

This patch uses an own function findDelimiter() for searching the delimiter.
It's more code and not so easy to read as the other alternative but should be
faster if the delimiter is somewhere behind the start of the signature.
It's also more flexible as it also is capable of finding the delimiter at the
end of the sig (although this shouldn't occur in reality).

It's just a suggestion if anyone isn't comfortable with my previous patch.

Comment 8

[Scott MacGregor]

Comment on attachment 137755 [details] [diff] [review]
proposed patch with automatic decision

In both patches, we are only iterating over the signature right? I would expect
signatures to be relatively small, so performing multiple Finds on it is
probably not that expensive.

But I'm ok with either patch. sr=mscott, I'll let Ere decide which patch he
wants to use.

Comment 9

[David :Bienvenu]

fix checked in (the first, simpler one)

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

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