The default bug view has changed. See this FAQ.

Fully fix onComposerFromChanged() after bug 337964

RESOLVED FIXED

Status

Thunderbird
Message Compose Window
--
major
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
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.
(Assignee)

Comment 1

6 years ago
Created attachment 502544 [details] [diff] [review]
(Av1) Do not abuse boolean vars with string values
Attachment #502544 - Flags: review?(mkmelin+mozilla)
(Assignee)

Comment 2

6 years ago
Created attachment 502545 [details] [diff] [review]
(Av1a) Do not abuse boolean vars with string values
[Checked in: Comment 5]
Attachment #502544 - Attachment is obsolete: true
Attachment #502545 - Flags: review?(mkmelin+mozilla)
Attachment #502544 - Flags: review?(mkmelin+mozilla)

Comment 3

6 years ago
(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

6 years ago
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+
(Assignee)

Comment 5

6 years ago
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]

Updated

4 years ago
Assignee: nobody → sgautherie.bz
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.