499558 - New signature UI with inconsistent "deactive" state if box is checked without file specified

Status: RESOLVED FIXED

(MailNews Core :: Composition, defect)

Description

[u235898]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1pre) Gecko/20090620 Shiretoko/3.5pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.9.1pre) Gecko/20090616 Lightning/1.0pre Shredder/3.0b3pre

If use signature file is checked but no file selected, signature from (deactivated) free text field is used

Reproducible: Always

Steps to Reproduce:
1. open account setup
2. enter signature to new free text field 
3. select "attach a signature from a file instead" checkbox without choosing a file!
4. click "OK" in account setup mask
5. create a new mail
6. reopen account setup
Actual Results:  
1. -
2. -
3. -
4. -
5. signature entered in signature text field is used
6. text field is disabled but "use file" without choosen file is checked

Expected Results:  
1. -
2. -
3. -
4. (perhaps a warning "no file selected")
5. no signature is beeing used
OR
6. "use file instead" is no longer checked but free entered signature is active again

relates to Request 324495

Comment 1

[rsx11m]

Confirmed, in that case the entered signature should be ignored and just no signature attached at all. I'll have a look at it.

Comment 2

[rsx11m]

The problem is in the nsMsgCompose::ProcessSignature() backend, where the code uses a single variable useSigFile to indicate that the box is checked, and then later to indicate that a file is present and contains a signature. Thus, if the box is checked but no file name given, useSigFile becomes true first as the box is checked, but false later after a signature file couldn't be loaded.

The issue also applies if the signature file was deleted or renamed, therefore an invalid file name is given. While this worked fine when the signature pref didn't exist yet, now it is assumed that the user didn't check the "signature from file" box and the entered text is taken if present (though it shouldn't).

Introducing another variable to separate those two functions of "useSigFile" would resolve the issue.

Comment 3

[rsx11m]

Attachment: Proposed fix

This adds a variable attachFile for the attachSignature attribute (I've kept "File" on purpose to make it clear that it's not about the text pref). The old useSigFile can only be true if attachFile is true and that file exists. Thus, the checkbox status remains available through attachFile. Note that this does not change anything in the UI, it may be surprising if the selection doesn't "stick" when clicking Ok even though the user action eventually results in an empty signature.

Also, to be consistent, "attachFile" instead of "useSigFile" is used now in QuotingOutputStreamListener as well. There is an obsolete comment (line 4008) which I've updated too as it reflected an older variant of the signature pref.

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 384538 [details] [diff] [review]
Proposed fix

Thanks for fixing this. r=Standard8. I've already pushed it on your behalf:

http://hg.mozilla.org/comm-central/rev/4358a1356c8d