181294 - Text colors wrong on personal toolbar (classic skin)

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect, P2)

Description

[Mike Pinkerton (not reading bugmail)]

2002112103 build osx 10.2.2, classic skin

in previous versions, rollover on the personal toolbar was blue and underlined.
clicking kept the text blue.

now, the text is black, and clicking the item turns the text white so it's
unreadable.

something regressed between now and 10/29 where it works ok.

Comment 1

[Andreas Kunz]

Mike, is this still the case?

Comment 2

[Mike Pinkerton (not reading bugmail)]

yes, this is still the case. it's quite annoying.

Comment 3

[Andreas Kunz]

Ok, Mike, I can only test on Windows, but there
http://lxr.mozilla.org/mozilla/source/themes/classic/global/win/toolbarbutton.css#44
determines the color of the regular text (-moz-DialogText), lines 62 and 72
determine hovered and hovered active (clicked) appearance. The latter ones are
set to #0000FF.
This makes the links look black (usually) and blue when hovered and clicked.

For Mac the colors in the corresponding file are: -moz-DialogText for normal,
[no color] for hovered and #FFFFFF for active. This pretty much explains what
you see (black-black-white, is that right?). 
This file was last changed *January* 2002 (by yourself, btw...), so the colors
you saw until November 2002 must have been defined somewhere else and then
removed there.

And the cause seems to be bug 174242. The patch took away the blue that was
defined cross-platform in bookmarksToolbar.css for the the hovered and active
bookmarks in the personal toolbar.
This change was intended *for Linux* as requested from dbaron; Windows was
seemingly unaffected because the bookmarks now take the colors from the
platform*specific* toolbarbutton.css where it is still defined as blue (as I
showed at the top).
But regular Mac toolbar buttons are not blue, so the bookmarks have now other
colors on Mac.

For fixing this on Mac either the font color for toolbarbuttons in general on
Mac have to be changed or toolbarbutton.bookmark-item:hover etc. have to be
colored blue at a *platformspecific* place, not where they are currently defined
(because bug 174242 would be reopened that way).
Mike, I think you know best how to fix this...

Comment 4

[Asa Dotzler [:asa]]

We should really try to get this fixed for 1.6. It's highly visible and a
terrible user experience.

Comment 5

[chris hofmann]

who can come up with patch?

Comment 6

[Chris Neale]

is this a simple matter of adding style rules to 
http://lxr.mozilla.org/mozilla/source/themes/classic/global/mac/toolbarbutton.css
?

i.e. toolbarbutton.bookmark-item:hover, toolbarbutton.bookmark-item:hover:active

as comment 3

Comment 7

[Kevin Gerich]

Attachment: Set color to -moz-DialogText on :hover:active

Since this css applies to all platforms, does this mess things up on platforms
other than Mac? I've only tested on my Powerbook.

Comment 8

[Asa Dotzler [:asa]]

Thanks for the quick response Chris and Kevin. Ben, can you review this patch?
This is currently on our 1.6 blocker list.

Comment 9

[Andreas Kunz]

On Windows this patch makes the labels of bookmarks in the bookmarks toolbar
appear black while they are pressed (without the patch they stay blue, just like
when only hovered).

I'm not sure what the intended behaviour on Mac is; should they get blue again
as they were earlier? From the patch it appears that they should be black (with
default colors) in all states now...

I guess on Linux this patch does not change anything (bug 174242 already removed
the blue color), but cannot test.

Kevin, just out of curiosity: why did you choose not to change the color in the
platform specific file
http://lxr.mozilla.org/mozilla/source/themes/classic/global/mac/toolbarbutton.css
? What about adding the desired colors (no matter if blue or -moz-DialogText)
for :hover and :active there? Does that not work?
As I pointed out in comment #3, second paragraph, this file seems to explain
pretty much the current (wrong) colors.

Comment 10

[Kevin Gerich]

The default :hover:active color is different across platforms for
toolbarbuttons. On Mac it should be white for all(?) toolbarbuttons except the
bookmarks toolbar items, so putting the rule in the global toolbarbutton.css is
not ideal.

We can take my patch and have the black color across platforms which would fix
this bug, back out the fix for 174242 (or restore the rule it removed) which
caused this bug, put the non-global rule in the global toolbarbutton.css, or
create a platform specific bookmarkToolbar.css for Mac.  

Comment 11

[Andreas Kunz]

As a recapitulation of the last comment:
Because there is only one (platform independent) bookmarkToolbar.css, you
basically have the choice between:
a) regressing Windows to change from blue to black on clicking (sounds bad to me)
b) regressing Linux by reopening bug 174242 (sounds equally bad)
c) creating platform dependent bookmarkToolbar.css files (would be cleanest, but
probably not make 1.6)
d) put the rule for Mac into the more general but platform dependent
toolbarbutton.css (is "not ideal")

That last mentioned file already contains selectors like
toolbarbutton[type="menu-button"]:hover:active which refer to special kinds of
toolbar buttons and to me it seems like overkill to create platform dependent
files just because of one style rule. This would be what I prefer, but I'm not
the one who decides.
"One who decides" should look at this and decide very soon or nothing will
happen at all (at least before 1.6).

Mike Pinkerton, what do you think?

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Perhaps the problem is here:  ?
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/themes/classic/global/mac/toolbarbutton.css&rev=1.4&mark=73#64

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Or, rather, that the previous URL clashes with:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/themes/classic/communicator/bookmarks/bookmarksToolbar.css&rev=1.21&mark=57#49

Removing the line above would make these look like other buttons.  That line and
the entire rule following should probably be removed...

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

OK, so after having looked at what's happening on Mac a little more, I'm leaning
in the other direction.

My problem with what used to happen is that the things on the personal toolbar
looked half like buttons and half like links.  On Mac, they manage not to look
very much like buttons, but the Windows/Linux button styles have borders in the
:hover state, so they really look a lot like buttons.  I'm somewhat curious as
to why the Mac personal toolbar buttons don't have borders on :hover.

I'd be quite happy with backing out bug 174242 if the buttons on the personal
toolbar didn't get borders on :hover, since then they'd look like links and not
like buttons.

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: possible patch

This backs out bug 174242 and adds two changes:
 * -moz-appearance: none for the non-container buttons
 * restrict the blue / underline to only non-container buttons (rather than
restricting the underline but keeping the blue on :hover)

I think this should be reasonable on Mac, but I haven't tested it there yet...

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: possible patch

a little more cleanup (this stylesheet was clearly written before we supported
:not(), and :not() makes it a good bit simpler)

Comment 17

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 137540 [details] [diff] [review]
possible patch

r=[e-mail]

Comment 18

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

taking

Comment 19

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Seems to improve the situation on Mac as well (I tested on the 1.5 branch
tinderbox).

Comment 20

[Asa Dotzler [:asa]]

Comment on attachment 137540 [details] [diff] [review]
possible patch

a=asa (on behalf of drivers) for checkin to Mozilla 1.6

Comment 21

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in to trunk, 2003-12-17 16:13 -0800.