722019 - Investigate status of cbox-dis.gif (and related)

Status: RESOLVED FIXED

(Toolkit :: Themes, defect)

Description

[Serge Gautherie (:sgautherie)]

(Noticed in bug 721357 comment 6.)

{
Chrome file doesn't exist: e:\buildbot\try-comm-cen-w32-dbg\build\obj-firefox\mozilla\dist\bin\chrome\toolkit\skin\classic\global\checkbox\cbox-dis.gif
}
NB: This was a SeaMonkey (Try) build, not a Firefox one.

***

Toolkit WinStripe and GnomeStripe seem to reference a nonexistent (anymore?) file.
(SeaMonkey Modern seems fine.)

http://mxr.mozilla.org/comm-central/search?string=cbox-dis.gif&case=on
{
/mozilla/toolkit/themes/winstripe/global/listbox.css
    * line 185 -- background-image: url("chrome://global/skin/checkbox/cbox-dis.gif");
/mozilla/toolkit/themes/gnomestripe/global/listbox.css
    * line 175 -- background-image: url("chrome://global/skin/checkbox/cbox-dis.gif");

/suite/themes/modern/global/checkbox.css
    * line 86 -- list-style-image: url("chrome://global/skin/checkbox/cbox-dis.gif") !important
/suite/themes/modern/global/listbox.css
    * line 105 -- list-style-image: url("chrome://global/skin/checkbox/cbox-dis.gif");
/suite/themes/modern/jar.mn
    * line 251 -- skin/modern/global/checkbox/cbox-dis.gif (global/checkbox/cbox-dis.gif)
}

http://mxr.mozilla.org/comm-central/find?text=&string=%2Fcbox-dis.gif
{
/suite/themes/modern/global/checkbox/cbox-dis.gif
}

helpwanted:
Need to investigate what happened wrt these rules and files.
Should Toolkit rules be merely removed? (Keeping only SM Modern ones.)
Should SM Modern be (somehow) updated (too)?

Comment 1

[Stefan [:stefanh]]

The checkbox is styled with '-moz-appearance: checkbox;', so I guess the toolkit icon rules are not needed.

Comment 2

[[e-mail]]

As far as I can see, this was part of the great hewitt theme cleanup of 2001.

I guess he must have copied and pasted too much from pinstripe.

Comment 3

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) Remove some useless/redundant listbox rules, in gnomestript and winstripe

Here is what I understood from previous comments.
Please double-check, as I know little here.

Comment 4

[[e-mail]]

Comment on attachment 592416 [details] [diff] [review]
(Av1) Remove some useless/redundant listbox rules, in gnomestript and winstripe

>Bug 722019. (Av1) Remove some useless/redundant listbox rules, in gnomestript and winstripe.
So, this patch should only have changes to listbox.css in gnomestripe and winstripe...

> .listcell-check[disabled="true"] {
>   border-color: GrayText;
As it happens, -moz-appearance also overrides the border.

Comment 5

[Stefan [:stefanh]]

Comment on attachment 592416 [details] [diff] [review]
(Av1) Remove some useless/redundant listbox rules, in gnomestript and winstripe

-  background-image: url("chrome://global/skin/checkbox/cbox-check.gif");

.
.
.
-  background-image: url("chrome://global/skin/checkbox/cbox-check-dis.gif");

Why remove these ones (the icons exists)?

Comment 6

[Jens Hatlak (:InvisibleSmiley)]

(In reply to [e-mail] from comment #2)
> As far as I can see, this was part of the great hewitt theme cleanup of 2001.
> 
> I guess he must have copied and pasted too much from pinstripe.

pinstripe existed in 2001? :-o

Comment 7

[Serge Gautherie (:sgautherie)]

Attachment: (Av2) Remove some useless/redundant listbox rules, in gnomestripe and winstripe [Checked in: Comment 12]

Av1, with comment 4 suggestion(s).


(In reply to Stefan [:stefanh] from comment #5)
> -  background-image: url("chrome://global/skin/checkbox/cbox-check.gif");
> -  background-image: url("chrome://global/skin/checkbox/cbox-check-dis.gif");
> 
> Why remove these ones (the icons exists)?

This is the way I understood your comment 1.
'checkbox.css' has similar (but not equal) rules: I assumed (as in guessed) those apply nonetheless.
If it is not the case, then I concur these 2 should be kept.

Please double-check, as I know little here.

Comment 8

[Stefan [:stefanh]]

(In reply to Serge Gautherie (:sgautherie) from comment #7)
> Created attachment 592713 [details] [diff] [review]
> (Av2) Remove some useless/redundant listbox rules, in gnomestripe and
> winstripe
> 
> Av1, with comment 4 suggestion(s).
> 
> 
> (In reply to Stefan [:stefanh] from comment #5)
> > -  background-image: url("chrome://global/skin/checkbox/cbox-check.gif");
> > -  background-image: url("chrome://global/skin/checkbox/cbox-check-dis.gif");
> > 
> > Why remove these ones (the icons exists)?
> 
> This is the way I understood your comment 1.
> 'checkbox.css' has similar (but not equal) rules: I assumed (as in guessed)
> those apply nonetheless.
> If it is not the case, then I concur these 2 should be kept.
> 
> Please double-check, as I know little here.

Hmm, it's a bit confusing, actually. There appear (if I understand correct) to be 2 -moz-appearance rules, one for the actual checkbox (checkbox-container) and one for the checkmark (checkbox). Now, listbox.css seem to only want the checkmark and in that case I think you're right (but maybe the icons should go too then). Anyway, Neil knows this more than me, so don't listen to me now ;-)

Comment 9

[[e-mail]]

(In reply to Stefan from comment #5)
> (From update of attachment 592416 [details] [diff] [review])
> >-  background-image: url("chrome://global/skin/checkbox/cbox-check.gif");
> > 
> >-  background-image: url("chrome://global/skin/checkbox/cbox-check-dis.gif");
> 
> Why remove these ones (the icons exists)?

We're not actually using them, because the appearance rules take precedence. (In fact, only the margin has any effect here.)

(In reply to Jens Hatlak from comment #6)
> (In reply to comment #2)
> > As far as I can see, this was part of the great hewitt theme cleanup of 2001.
> > 
> > I guess he must have copied and pasted too much from pinstripe.
> 
> pinstripe existed in 2001? :-o

Would you prefer "Mac Classic"? That sounds too much like OS9 to me ;-)

(In reply to Stefan from comment #8)
> Hmm, it's a bit confusing, actually. There appear (if I understand correct)
> to be 2 -moz-appearance rules, one for the actual checkbox
> (checkbox-container) and one for the checkmark (checkbox). Now, listbox.css
> seem to only want the checkmark and in that case I think you're right (but
> maybe the icons should go too then). Anyway, Neil knows this more than me,
> so don't listen to me now ;-)

Well, we would have to remove the icons from any other obsolete users (probably tree.css and checkbox.css but there may be others).

Comment 10

[Stefan [:stefanh]]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #6)

> pinstripe existed in 2001? :-o

I actually did :-P (the project started then)

Not in the tree, though - that happened a bit later on.

Comment 11

[Stefan [:stefanh]]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #6)

> pinstripe existed in 2001? :-o

It actually did :-P (the project started then)

Not in the tree, though - that happened a bit later on.

Comment 12

[Serge Gautherie (:sgautherie)]

Comment on attachment 592713 [details] [diff] [review]
(Av2) Remove some useless/redundant listbox rules, in gnomestripe and winstripe
[Checked in: Comment 12]

https://hg.mozilla.org/mozilla-central/rev/e61dcd6f9bd7

Comment 13

[Serge Gautherie (:sgautherie)]

Ftr, SeaMonkey Modern listbox.css has no -moz-appearance, so nothing to touch there.


(In reply to [e-mail] from comment #9)
> (In fact, only the margin has any effect here.)
> 
> Well, we would have to remove the icons from any other obsolete users
> (probably tree.css and checkbox.css but there may be others).

I would be willing to try to do more cleanup if there is.
Please file a bug (with details) as need be.