In the Junk settings pane of Account manager, the "Do not mark mail as junk if the sender is in:" addressbook whitelist is always enabled. If it is used only when the adaptive junk mail control is enabled, then it should be disabled otherwise. Alternatively, it should only be enabled if any of "adaptive junk mail" or "trust headers set by" is enabled. Can anybody test or confirm when this whitelist is consulted?

Also, if the whitelist is dependent on the adaptive junk mail control, make it indented so the dependence is seen visually. As in the mockup in this attachment: https://bugzilla.mozilla.org/attachment.cgi?id=612072

See also comments in bug 397197.

The indentation was already done in bug 397197.

So this could do it. However there is a glitch. Then the whilelist is disabled, the checkboxes in it do not appear disabled (just don't work). Is that some problem with the widget?

Comment on attachment 618796 [details] [diff] [review] patch Is that an universal problem with the list widget or is this some addressbook specific implementation?

I should also disable the list caption "Do not automatically mark..." .

Comment on attachment 618796 [details] [diff] [review] patch Yeah, that's a glitch. ;) Other than that, I think I like it, but that kinda needs to be fixed. (I took a quick look, and couldn't figure out why your disabling didn't work.)

Who can help us here if it is a problem with the widget?

So what about this? :) It came out that checkbox is actually just an image (defined in core listbox.css and such). It seems it doesn't have css rule for the [disabled] state. So I made a rule for our element (with opacity:0.5 which produces just the needed effect, at least on linux). But it should probably better be handled in the core css, can you contact the proper people?

Roc, bz, can you forward this to the correct people? The problem is we have an element like this: <listbox id="whiteListAbURI" rows="5"/> let abItem = document.createElement("listitem"); abItem.setAttribute("type", "checkbox"); abItem.setAttribute("class", "listitem-iconic"); abItem.setAttribute("label", ab.dirName); abItem.setAttribute("value", ab.URI); When we set disabled=true on it, most of the element turns gray, but the checkbox icon stays white (without change). I could override it in our theme (see patch v2), but there surely is a better way.

I don't know who maintains the core CSS. Neil Deakin maybe?

Comment on attachment 619441 [details] [diff] [review] patch v2 That's better, but I think we also want to grey-out the text, and the border, like in the text box for "Automatically delete junk mail after [14] days". And I kind of suspect that this is the wrong way to go, and we should figure out why setting the disabled attribute doesn't do the right thing here. So, given those, I have to say ui-r-.

Bwinton, can you attach a screenshot of what is not greyed out?

I hope http://dl.dropbox.com/u/2301433/Screenshots/GreyJunk.png shows what I mean…

Well, in your screenshot the whitelist background is white in the same way as the number in "Automatically delete junk mail after [14] days". Also, the "trust junk mail headers set by" should not be greyed out, it is not dependent on Adaptive junk setting. But I agree the whilelist entries' text should be grey (as it eh whitelist caption). I'll look into that. Maybe it is the same problem as the checkboxes not properly greying out (the whole widget disabling may be unimplemented in toolkit).

But for me on Linux, the whitelist is disabled fine, the text is grey and the background too. Just in case I attach an updated patch. Do you still see the problem? Can you try other platforms?

Alternative patch that does not just set .disabled=true on the listbox (that seems to behave differently in each theme) but also on individual list-rows (listitems?). That does work for me fine in gnomestripe without the need of any theme fixes (I had opacity 0.5 for the "checkbox" image in patch v3). So this patch has no theme changes. Does it work for you any better?

Comment on attachment 632884 [details] [diff] [review] patch v4 (alternative) Works for me on OS X! And on Windows 7! And on Ubuntu! Based on those, I like it. ui-r=me!

Great to hear that :)

Comment on attachment 632828 [details] [diff] [review] patch v3 The text is still black for me with this patch. Fortunately, I think V4 is a better way to go, so this f- is really only for consistency's sake. ;) Thanks, Blake.

Yes, I have no problem going with v4 if it works for everyone.

Comment on attachment 632884 [details] [diff] [review] patch v4 (alternative) >+ // Enabled/disable individual listbox rows Nit: full stop at end of sentence. >+ // Setting enable/disable on the parent listbox does not seem to work Nit: full stop at end of sentence. r=me with that fixed.

Thanks.

Comment on attachment 638223 [details] [diff] [review] patch v5 Review of attachment 638223 [details] [diff] [review]: ----------------------------------------------------------------- One super-tiny nit, otherwise, looks solid. Thanks aceman. ::: mailnews/base/prefs/content/am-junk.js @@ +167,4 @@ > } > > +/** > + * propagate changes to the server filter menu list back to Super tiny nit - propagate should be capitalised.

Thanks.