642 bytes, patch
|Details | Diff | Splinter Review|
1.78 KB, patch
Ben Goodger (use ben at mozilla dot org for email): review+
Brian Ryner (not reading): superreview+
|Details | Diff | Splinter Review|
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.
Mike, is this still the case?
yes, this is still the case. it's quite annoying.
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...
We should really try to get this fixed for 1.6. It's highly visible and a terrible user experience.
who can come up with patch?
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
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.
Thanks for the quick response Chris and Kevin. Ben, can you review this patch? This is currently on our 1.6 blocker list.
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.
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.
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?
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
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...
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.
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...
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)
Comment on attachment 137540 [details] [diff] [review] possible patch email@example.com
Seems to improve the situation on Mac as well (I tested on the 1.5 branch tinderbox).
Comment on attachment 137540 [details] [diff] [review] possible patch a=asa (on behalf of drivers) for checkin to Mozilla 1.6
Fix checked in to trunk, 2003-12-17 16:13 -0800.