Closed Bug 229044 Opened 21 years ago Closed 21 years ago

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


(MailNews Core :: Composition, defect)

Not set


(Not tracked)



(Reporter: ch.ey, Assigned: ch.ey)




(2 files)

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

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>).
Severity: normal → minor
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.
Attachment #137755 - Flags: review?(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.
Attachment #137755 - Flags: superreview?(mscott)
Attachment #137755 - Flags: review?(bienvenu)
Attachment #137755 - Flags: review+
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?
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
Dupe of bug 113684?  Sure looks like it.
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?
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 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.
fix checked in (the first, simpler one)
Closed: 21 years ago
Resolution: --- → FIXED
*** Bug 113684 has been marked as a duplicate of this bug. ***
Attachment #137755 - Flags: superreview?(mscott)
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.