Preference items in "images" is not lockable

RESOLVED FIXED

Status

SeaMonkey
Preferences
RESOLVED FIXED
14 years ago
14 years ago

People

(Reporter: Boying Lu, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

939 bytes, patch
neil@parkwaycc.co.uk
: review+
jag (Peter Annema)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

14 years ago
If the preference of the checkbox in "Privacy and Security" -->Images is locked
by autoconfig.jsc. The corresponding checkbox is NOT actually locked. i.e. user
can still check ( or uncheck) this item.
(Reporter)

Comment 1

14 years ago
Created attachment 155350 [details] [diff] [review]
patch v0
(Reporter)

Comment 2

14 years ago
Comment on attachment 155350 [details] [diff] [review]
patch v0

Hi, BZ,
  Can you give r/sr?

Thanks
Attachment #155350 - Flags: review?(bzbarsky)
Comment on attachment 155350 [details] [diff] [review]
patch v0

I'm not going to get to this anytime soon, unfortunately.

In any case, it needs review from an XPFE peer and I'm generally not doing srs
in modules I'm not a peer for right now...
Attachment #155350 - Flags: review?(bzbarsky)
Summary: Preference items in "images" is lockable → Preference items in "images" is not lockable
(Reporter)

Comment 4

14 years ago
Comment on attachment 155350 [details] [diff] [review]
patch v0

Can you give r? Thanks
Attachment #155350 - Flags: review?(neil.parkwaycc.co.uk)

Comment 5

14 years ago
Comment on attachment 155350 [details] [diff] [review]
patch v0

>     // if mailnews is installed then we will have networkImageDisableImagesInMailNews checkbox
>     var networkImageDisableImagesInMailNews = document.getElementById("networkImageDisableImagesInMailNews");
>+    var prefString = networkImageDisableImagesInMailNews.getAttribute("prefstring");
>     if (networkImageDisableImagesInMailNews)
Getting the attribute before you check that the element exists looks like a bad
idea to me...
Attachment #155350 - Flags: review?(neil.parkwaycc.co.uk) → review-
(Reporter)

Comment 6

14 years ago
Created attachment 155534 [details] [diff] [review]
patch v1
(Reporter)

Comment 7

14 years ago
Comment on attachment 155534 [details] [diff] [review]
patch v1

Can you give r now?
Attachment #155534 - Flags: review?(neil.parkwaycc.co.uk)

Updated

14 years ago
Attachment #155534 - Flags: review?(neil.parkwaycc.co.uk) → review+
(Reporter)

Comment 8

14 years ago
Comment on attachment 155534 [details] [diff] [review]
patch v1

Can you give sr?
Thanks a lot
Attachment #155534 - Flags: superreview?(neil.parkwaycc.co.uk)

Comment 9

14 years ago
Comment on attachment 155534 [details] [diff] [review]
patch v1

Sorry, there's no r+sr rule for xpfe.
(Reporter)

Comment 10

14 years ago
Do you mean that the patch can be checked in now?

Comment 11

14 years ago
No, it means you have to ask someone else for sr.
(Reporter)

Updated

14 years ago
Attachment #155350 - Attachment is obsolete: true
(Reporter)

Comment 12

14 years ago
Comment on attachment 155534 [details] [diff] [review]
patch v1

Can you give sr? Thanks
Attachment #155534 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview?(jag)

Comment 13

14 years ago
Comment on attachment 155534 [details] [diff] [review]
patch v1

I suggest you switch the order of getPrefIsLocked and the value checked. To me
it makes more sense to say

"it's disabled if the pref is locked, or if it's not locked but the value is 2"

than

"it's disabled if the value is 2, or if the value is something other than 2 but
the pref is locked".

Put differently: "we don't care about the value if the pref is locked" is more
clear if you switch the order.

Anyway, just a suggestion, sr=jag either way.
Attachment #155534 - Flags: superreview?(jag) → superreview+
(Reporter)

Comment 14

14 years ago
Neil, Would you please help me to checkin this patch? Because I have no
permission  to do so. Thanks 

Comment 15

14 years ago
The tree is currently closed for the 1.8a3 freeze. Please remind me later.

http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey&nocrap=1#status
(Reporter)

Comment 16

14 years ago
Hi, Neil, 
  Can you checkin the patch now? The tree is open. Thanks a lot.

Comment 17

14 years ago
(In reply to comment #16)
> Hi, Neil, 
>   Can you checkin the patch now? The tree is open. Thanks a lot.

As you did not read the link I gave you, here is what it says:

We're *FROZEN* for 1.8a3. The tree is *open* for driver-*approved* checkins.

Comment 18

14 years ago
just checked in
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.