624440 - Fully fix onComposerFromChanged() after bug 337964

Status: RESOLVED FIXED

(Thunderbird :: Message Compose Window, defect)

Description

[Serge Gautherie (:sgautherie)]

I filed (SM port) bug 624432,
but I'm puzzled by the "changed" case(s) of bug 337964 fix.

I'm confused by the vars being named 'g*OptionChanged' (which seems correct to me) versus the comments about 'manually checked' [Couldn't the case be "manually unchecked" too?]
Shouldn't useEncryption/useSigning value be explicitly set in !=/!signMessage cases too?
Shouldn't gEncryptOptionChanged and gSignOptionChanged be reset to false in some case(s)?

If the code is correct as is, maybe just modifying/adding some more comments would clarify it.

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) Do not abuse boolean vars with string values

Comment 2

[Serge Gautherie (:sgautherie)]

Attachment: (Av1a) Do not abuse boolean vars with string values [Checked in: Comment 5]

Comment 3

[Magnus Melin [:mkmelin]]

(In reply to comment #0)
> I'm confused by the vars being named 'g*OptionChanged' (which seems correct to
> me) versus the comments about 'manually checked' [Couldn't the case be
> "manually unchecked" too?]

The code comments "checked" is really implying checked/unckeckecked.

> Shouldn't useEncryption/useSigning value be explicitly set in !=/!signMessage
> cases too?

Matter of taste i guess. This way it's the least amount of code.

> Shouldn't gEncryptOptionChanged and gSignOptionChanged be reset to false in
> some case(s)?

I suppose one could, but i don't really they should ever reset. If they changed the user made a conscious choise.

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 502545 [details] [diff] [review]
(Av1a) Do not abuse boolean vars with string values
[Checked in: Comment 5]

Ok with me.

Comment 5

[Serge Gautherie (:sgautherie)]

Comment on attachment 502545 [details] [diff] [review]
(Av1a) Do not abuse boolean vars with string values
[Checked in: Comment 5]

http://hg.mozilla.org/comm-central/rev/57a01a6867f8