Text colors wrong on personal toolbar (classic skin)

RESOLVED FIXED in mozilla1.6final

Status

SeaMonkey
Themes
P2
major
RESOLVED FIXED
15 years ago
9 years ago

People

(Reporter: Mike Pinkerton (not reading bugmail), Assigned: dbaron)

Tracking

({regression})

Trunk
mozilla1.6final
PowerPC
Mac OS X
regression
Bug Flags:
blocking1.6 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [patch])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

15 years ago
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

15 years ago
Mike, is this still the case?
(Reporter)

Comment 2

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

Comment 3

15 years ago
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

14 years ago
We should really try to get this fixed for 1.6. It's highly visible and a
terrible user experience.
Flags: blocking1.6+

Comment 5

14 years ago
who can come up with patch?

Comment 6

14 years ago
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

14 years ago
Created attachment 137194 [details] [diff] [review]
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

14 years ago
Thanks for the quick response Chris and Kevin. Ben, can you review this patch?
This is currently on our 1.6 blocker list.
Assignee: shliang → bugs

Comment 9

14 years ago
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

14 years ago
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

14 years ago
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?
(Assignee)

Comment 12

14 years ago
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
(Assignee)

Comment 13

14 years ago
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...
(Assignee)

Comment 14

14 years ago
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.
(Assignee)

Comment 15

14 years ago
Created attachment 137538 [details] [diff] [review]
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...
(Assignee)

Comment 16

14 years ago
Created attachment 137540 [details] [diff] [review]
possible patch

a little more cleanup (this stylesheet was clearly written before we supported
:not(), and :not() makes it a good bit simpler)
Attachment #137538 - Attachment is obsolete: true
Comment on attachment 137540 [details] [diff] [review]
possible patch

r=ben@mozilla.org
Attachment #137540 - Flags: review+
(Assignee)

Comment 18

14 years ago
taking
Assignee: bugs → dbaron
Priority: -- → P2
Whiteboard: [patch]
Target Milestone: --- → mozilla1.6final
(Assignee)

Comment 19

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

Updated

14 years ago
Attachment #137540 - Flags: superreview?(bryner)
Attachment #137540 - Flags: approval1.6?

Comment 20

14 years ago
Comment on attachment 137540 [details] [diff] [review]
possible patch

a=asa (on behalf of drivers) for checkin to Mozilla 1.6
Attachment #137540 - Flags: approval1.6? → approval1.6+
(Assignee)

Updated

14 years ago
Attachment #137540 - Flags: superreview?(bryner) → superreview?(roc)
Attachment #137540 - Flags: superreview?(roc) → superreview+
(Assignee)

Comment 21

14 years ago
Fix checked in to trunk, 2003-12-17 16:13 -0800.
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.