254553 - Preference items in "images" is not lockable

Status: RESOLVED FIXED

(SeaMonkey :: Preferences, defect)

Description

[Boying Lu]

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.

Comment 1

[Boying Lu]

Attachment: patch v0

Comment 2

[Boying Lu]

Comment on attachment 155350 [details] [diff] [review]
patch v0

Hi, BZ,
  Can you give r/sr?

Thanks

Comment 3

[Boris Zbarsky [: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...

Comment 4

[Boying Lu]

Comment on attachment 155350 [details] [diff] [review]
patch v0

Can you give r? Thanks

Comment 5

[[e-mail]]

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...

Comment 6

[Boying Lu]

Attachment: patch v1

Comment 7

[Boying Lu]

Comment on attachment 155534 [details] [diff] [review]
patch v1

Can you give r now?

Comment 8

[Boying Lu]

Comment on attachment 155534 [details] [diff] [review]
patch v1

Can you give sr?
Thanks a lot

Comment 9

[[e-mail]]

Comment on attachment 155534 [details] [diff] [review]
patch v1

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

Comment 10

[Boying Lu]

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

Comment 11

[[e-mail]]

No, it means you have to ask someone else for sr.

Comment 12

[Boying Lu]

Comment on attachment 155534 [details] [diff] [review]
patch v1

Can you give sr? Thanks

Comment 13

[jag (Peter Annema)]

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.

Comment 14

[Boying Lu]

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

Comment 15

[[e-mail]]

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

Comment 16

[Boying Lu]

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

Comment 17

[[e-mail]]

(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

[Pete Zha]

just checked in