252953 - check boxes don't show correctly for load images for originating site only

Status: RESOLVED WORKSFORME

(Firefox :: Settings UI, defect)

Description

[Michael Lefevre]

using Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7) Gecko/20040724
Firefox/0.9.1+

to reproduce:
1. in tools, options, web features, check both "load images" and "for
originating site only", and OK dialog
2. select tools, options, web features again.

actual result:
"load images" is unchecked, "for originating site only" is checked, but
disabled. (pref is actually set correctly, and images are loaded for the
originating site only)

expected result:
"load images" is checked, "originating site" checked and not disabled

Expected result needs to be the initial state of the dialog when
network.image.imageBehavior is 1.

Comment 1

[Michael Lefevre]

Seems to be this change to pref-features.js (from bug 241705) that caused it:

-  case "1": gImagesRestricted.checked=true;
-  case "0": gImagesEnabled.checked=true;
+  case "1": 
+    gImagesRestricted.checked = true;
+    break;
+  case "0": 
+    gImagesEnabled.checked = true;
+    break;

If the pref is set to 1, gImagesEnabled.checked should still be set to true, so
falling through was the right thing to do.

Either the break in case 1 should be removed, or gImagesEnabled.checked = true
should be added to the first case.

Comment 2

[Michael Lefevre]

Attachment: trivial patch

Patch to remove the first break and add a "fall through" comment to make it
clear why.  Afraid I don't have CVS or a tree set up, so patch doesn't have
full paths etc

Comment 3

[Mike Connor [:mconnor]]

I fixed this with a bunch of other stuff in bug 230462

Comment 4

[Michael Lefevre]

Fine, but how about adding a "fall-through" comment (which I was told on IRC is
the correct thing to do in terms of coding style) so that someone doesn't spot
the sloppy missing break and "tidy it up" when they're passing by?  Too late now
I suppose...

Comment 5

[:Gavin Sharp [email: [e-mail]]]

This was fixed, then rewritten :).

Comment 6

[Mike Connor [:mconnor]]

sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs,
filter on "beltznerLovesGoats" to get rid of this mass change