Notifications preference pane doesn't observe locked status for show_alert and show_balloon

RESOLVED FIXED in seamonkey2.20

Status

SeaMonkey
Preferences
--
minor
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: rsx11m, Assigned: rsx11m)

Tracking

Trunk
seamonkey2.20

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

1.77 KB, patch
neil@parkwaycc.co.uk
: review+
Details | Diff | Splinter Review
(Assignee)

Description

4 years ago
After learning how to lock preferences (bug 861471 comment #43) I've revisited the logic for bug 856454. While the individual mail.biff.alert.show_* prefs are correctly disabled on locking (and remain disabled when the entire vbox is enabled again, so that part is taken care of by the prefwindow already), the logic for unchecking one alert when selecting the other is still active.

STR:
 1. lock "mail.biff.show_alert" to true
 2. verify that "Show an alert" box is checked but disabled
 3. check "Show a balloon" (with "Show an icon" checked)
 4. note that "Show an alert" becomes unchecked despite locking

Thus, the locked status has to be verified in both instances before changing the value in response to checking the other alert's box.
(Assignee)

Comment 1

4 years ago
Created attachment 742819 [details] [diff] [review]
Proposed patch

This patch assigns "false" only if the preference to be changed is not locked.

In theory, this allows now both show_alert and show_balloon to be selected at the same time (e.g., one is locked "true" and the other checked by the user). It is safe to assume that, if an administrator wants to prevent that case, he or she would lock both show_alert and show_balloon preferences.
Attachment #742819 - Flags: review?(neil)
(Assignee)

Comment 2

4 years ago
> individual mail.biff.alert.show_* prefs are correctly disabled on locking
> ([...] that part is taken care of by the prefwindow already),

Actually, that's done by EnableElement() as called from EnableElementById().

Comment 3

4 years ago
Comment on attachment 742819 [details] [diff] [review]
Proposed patch

Seems reasonable.
Attachment #742819 - Flags: review?(neil) → review+
(Assignee)

Comment 4

4 years ago
Thanks Neil. Push for comm-central, please.
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/87d846122a36
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.20
You need to log in before you can comment on or make changes to this bug.