Closed Bug 624440 Opened 13 years ago Closed 11 years ago

Fully fix onComposerFromChanged() after bug 337964

Categories

(Thunderbird :: Message Compose Window, defect)

Product:
Thunderbird
Component:
Message Compose Window
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: sgautherie, Assigned: sgautherie)

References

(
URL
)

Details

Attachments

(1 file, 1 obsolete file)

(Av1a) Do not abuse boolean vars with string values [Checked in: Comment 5]
1.87 KB, patch
mkmelin
: review+
Details | Diff | Splinter Review 
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.

        Attachment #502544 -
        Flags: review?(mkmelin+mozilla)

    
    

    
  


  

        Attachment #502544 -
        Attachment is obsolete: true

        Attachment #502545 -
        Flags: review?(mkmelin+mozilla)

        Attachment #502544 -
        Flags: review?(mkmelin+mozilla)

    
    

    
  
(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 on attachment 502545 [details] [diff] [review]
(Av1a) Do not abuse boolean vars with string values
[Checked in: Comment 5]

Ok with me.

        Attachment #502545 -
        Flags: review?(mkmelin+mozilla) → review+

    
    

    
  
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

        Attachment #502545 -
        Attachment description: (Av1a) Do not abuse boolean vars with string values → (Av1a) Do not abuse boolean vars with string values
[Checked in: Comment 5]

    
  
Assignee: nobody → sgautherie.bz
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED

          You need to log in
          before you can comment on or make changes to this bug.