116259 - Inactive Link Toolbar links use wrong icons

Status: VERIFIED FIXED

(SeaMonkey :: Themes, defect)

Description

[Joe McCabe]

The inactive links on the Link Toolbar are using the wrong icons (rather than a
greyed version of the active icon they are using images from the Bookmarks icons).

Using 2001122008 build.

Image to be attached.

Comment 1

[Joe McCabe]

Attachment: Image shows incorrect inactive icons in Link Toolbar

Comment 2

[Joe McCabe]

It should be mentioned that this /may/ be related to 113100.

Comment 3

[Joe Hewitt (gone)]

link toolbar == [e-mail]

Comment 4

[Boris Zbarsky [:bzbarsky]]

The icons used to be correct.  So someone broke them...  Reassigning regressions 
[e-mail] seems a little odd, even if they are in a part of the UI 
that's off by default.

Assigning to Stuart, ccing Tim

Comment 5

[Stuart Ballard]

I'm not seeing this in 2001122808 in the classic skin. Is it modern-only?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Almost certainly. The classic icons were never munged in the "fix too-large 
icons" checkin, iirc.  The modern ones were...

Comment 7

[Stuart Ballard]

Attachment: Patch to fix the problem

I don't know if this is the right fix, but it works...

Comment 8

[Boris Zbarsky [:bzbarsky]]

Um... Either there is a style system bug here or something is setting the 
inline-style list-style-image on those elements (bad idea as far as theming 
goes, no?).... ccing hyatt.

Stuart, could you use DOM inspector on a navigator window and see which style 
rules are applied to the elements in question and where these rules come from 
(without your fix)?

Comment 9

[Stuart Ballard]

This appears in skin/modern/communicator/bookmarks/bookmarksToolbar.css (sorry,
don't know what path in CVS that corresponds to, I only use patchmaker :) ) line
72.

toolbarbutton.bookmark-item[disabled="true"] {
  cursor: default !important;
  text-decoration: none !important;
  list-style-image:
url("chrome://communicator/skin/bookmarks/bookmark-item-dis.gif") !important;
}

My personal feeling is that it really is a bug for themes to be using !important
rules all over the place, but if a !important rule is being used for
bookmarktoolbar items in general, then I need to use a !important rule to
override that, right? :) Hewitt, care to give us some commentary on why
!important rules are used and whether they're kosher in this case?

Now that I've found this (thanks for the tip about the document inspector!) I'm
more convinced that this is indeed the right patch. Anyone want to review?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 63071 [details] [diff] [review]
Patch to fix the problem

Hmm... The classic version of bookmarksToolbar.css
(http://lxr.mozilla.org/seamonkey/source/themes/classic/communicator/bookmarks/
bookmarksToolbar.css) also uses the !important stuff all over...

The patch does indeed do the right thing given the bookmarksToolbar.css files,
but I'm confused as to why this bug is modern-only then.  :)

In any case, r=bzbarsky; get sr from hewitt, please

Comment 11

[Joe Hewitt (gone)]

Comment on attachment 63071 [details] [diff] [review]
Patch to fix the problem

sr=hewitt

Comment 12

[[e-mail] (gone - use [e-mail] instead)]

Thanks Stuart, I landed this for you.

Comment 13

[Patty Mac]

Verified on linux (2002-01-09-08-trunk)