Closed
Bug 1044595
Opened 7 years ago
Closed 7 years ago
[10.10] default buttons in dialogs etc. have black text instead of white
Categories
(Core :: Widget: Cocoa, defect)
Core
Widget: Cocoa
Tracking
()
VERIFIED
FIXED
mozilla35
People
(Reporter: danne.da, Assigned: Gijs)
References
(Blocks 1 open bug)
Details
Attachments
(3 files, 2 obsolete files)
177.38 KB,
image/png
|
Details | |
10.55 KB,
image/png
|
Details | |
12.14 KB,
patch
|
Gijs
:
review+
lmandel
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:34.0) Gecko/20100101 Firefox/34.0 (Beta/Release) Build ID: 20140727030204 Steps to reproduce: Currently any button element is Firefox Nightly doesn't match the look in OS X Yosemite. Most noticeable are the "default" button and active state where the text color is black when really should be white. Attached are screenshots of how the buttons should look and how it currently looks in Firefox.
Updated•7 years ago
|
Component: General → Widget: Cocoa
Product: Toolkit → Core
Updated•7 years ago
|
Depends on: theme-yosemite
Updated•7 years ago
|
Blocks: theme-yosemite
No longer depends on: theme-yosemite
Assignee | ||
Comment 2•7 years ago
|
||
(In reply to d.a. from comment #0) > Currently any button element is Firefox Nightly doesn't match the look in OS > X Yosemite. Most noticeable are the "default" button and active state where > the text color is black when really should be white. Forgive me for not being the most visually oriented person, but you wrote "most noticeable" - what else, if anything, besides the text color on the default/focused/active button are we not getting right wrt the button style on 10.10?
Flags: needinfo?(danne.da)
Assignee | ||
Updated•7 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to :Gijs Kruitbosch from comment #2) > (In reply to d.a. from comment #0) > > Currently any button element is Firefox Nightly doesn't match the look in OS > > X Yosemite. Most noticeable are the "default" button and active state where > > the text color is black when really should be white. > > Forgive me for not being the most visually oriented person, but you wrote > "most noticeable" - what else, if anything, besides the text color on the > default/focused/active button are we not getting right wrt the button style > on 10.10? I should have used a period instead of the the word and, as in it is most noticeable in the "default" button. Should be noted that the black text only appears on buttons. All other controls (selects, radiobuttons and checkboxes) have white colored symbols.
Flags: needinfo?(danne.da)
Assignee | ||
Updated•7 years ago
|
Summary: OS X Yosemite Update buttons to match new look → [10.10] default buttons in dialogs etc. have black text instead of white
Comment 4•7 years ago
|
||
I belive the easiest way to fix this is to add style rules in button.css so (when on Yosemite) we change the label color to white when the button is pressed. I see that we have -moz-buttonhover, so I guess one could also add a -moz-buttonactive color and (if it exists) use a system NSColor (if we're lucky it'll stay black on < 10.10)- not sure if it's worth it, though.
Comment 5•7 years ago
|
||
(In reply to Stefan [:stefanh] from comment #4) > I see that we have -moz-buttonhover Oops, it's "-moz-buttonhovertext" so it should be -moz-buttonactivetext.
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Stefan [:stefanh] from comment #5) > (In reply to Stefan [:stefanh] from comment #4) > > I see that we have -moz-buttonhover > > Oops, it's "-moz-buttonhovertext" so it should be -moz-buttonactivetext. But it's not the active state - which is a "pressed" button - but the 'default' state. Otherwise, I agree that we need another -moz-* thing here.
Comment 7•7 years ago
|
||
Ah, I missed the "default". But from the report it looks like it's both "default" and "pressed", though.
Some clarification: It's both the "default" and pressed/active state that have the wrong text-color (as in anytime the button turns blue). But unless you keep the button pressed for long you're not that likely to notice that the text color is wrong in the active/pressed state.
Assignee | ||
Comment 9•7 years ago
|
||
This WFM, although it took a little bit of poking to get the CSS right. Markus, Jared, what do you think? :-)
Attachment #8488329 -
Flags: review?(jaws)
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Assignee | ||
Updated•7 years ago
|
Attachment #8488329 -
Flags: review?(mstange)
Assignee | ||
Comment 10•7 years ago
|
||
For the record, this was based on some IRC discussion with Markus ( http://logs.glob.uno/?c=mozilla%23fx-team&s=11+Sep+2014&e=11+Sep+2014#c165962 and later) where he suggested having two constants - even if they are currently implemented the same way, it makes sense to me to define them separately. Likewise, it seemed best to evil-hardcode the values in nsLookAndFeel instead of having a media query in toolkit, but one could argue differently, I suppose, if we're comfortable sticking this in CSS instead...
Updated•7 years ago
|
Attachment #8488329 -
Flags: review?(mstange) → review+
Comment 11•7 years ago
|
||
Comment on attachment 8488329 [details] [diff] [review] fix default/active button text on OS X 10.10, Review of attachment 8488329 [details] [diff] [review]: ----------------------------------------------------------------- Markus, in test_platform_colors.xul there is a check for colors[c].length == 8. Why should that cause a test failure? ::: widget/tests/test_platform_colors.xul @@ +65,5 @@ > "-moz-dragtargetzone": ["rgb(199, 208, 218)", "rgb(198, 198, 198)", "rgb(180, 213, 255)", "rgb(250, 236, 115)", "rgb(255, 176, 139)", "rgb(255, 209, 129)", "rgb(194, 249, 144)", "rgb(232, 184, 255)"], > "-moz-hyperlinktext": ["rgb(0, 0, 238)"], > "-moz-html-cellhighlight": ["rgb(212, 212, 212)"], > "-moz-html-cellhighlighttext": ["rgb(0, 0, 0)"], > + "-moz-mac-buttonactivetext": ["rgb(0, 0, 0)", "rgb(255, 255, 255)"], It would be nice if this could only be #fff when on Yosemite and later, and #000 when earlier than Yosemite.
Attachment #8488329 -
Flags: review?(jaws) → review+
Assignee | ||
Comment 12•7 years ago
|
||
remote: https://tbpl.mozilla.org/?tree=Try&rev=0d01038c6240 remote: Alternatively, view them on Treeherder (experimental): remote: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=0d01038c6240
Keywords: checkin-needed
Assignee | ||
Comment 13•7 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/3646cd944abe
Keywords: checkin-needed
Comment 14•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/3646cd944abe
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Reporter | ||
Comment 15•7 years ago
|
||
I can confirm that the patch working on OS X Yosemite, the text is now white where it should be.
Updated•7 years ago
|
Status: RESOLVED → VERIFIED
Comment 16•7 years ago
|
||
(In reply to d.a. from comment #15) > the text is now white where it should be. … and white where it should not be white (alerts).
Comment 17•7 years ago
|
||
I filed bug 1068913.
Assignee | ||
Comment 18•7 years ago
|
||
Assignee | ||
Comment 19•7 years ago
|
||
Comment on attachment 8498102 [details] [diff] [review] Patch for Aurora So I've pulled together this patch, and those for bug 1072992 and bug 1068913, and would like to uplift the lot to 34. Approval Request Comment [Feature/regressing bug #]: Yosemite [User impact if declined]: button colors will look odd on OS X Yosemite when released (rumored for October) [Describe test coverage new/current, TBPL]: little, mostly styling only patch (some minor widget additions which have parsing tests) [Risks and why]: we might still have missed buttons somewhere in the UI that override the button styling but don't override the foreground color (like the issues from bug 1068913, bug 1072992). Fixing those kinds of issues is pretty trivial, however, and this has been in Nightly for 2 weeks now, so I expect this to be OK. [String/UUID change made/needed]: nope
Attachment #8498102 -
Flags: review+
Attachment #8498102 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 20•7 years ago
|
||
Comment on attachment 8498102 [details] [diff] [review] Patch for Aurora Found another small issue, namely disabled buttons...
Attachment #8498102 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 21•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8488329 -
Attachment is obsolete: true
Assignee | ||
Comment 22•7 years ago
|
||
Comment on attachment 8501083 [details] [diff] [review] Patch for aurora Once more: I've pulled together this patch, and those for bug 1076252, bug 1072992 and bug 1068913, and would like to uplift the lot to 34. Approval Request Comment [Feature/regressing bug #]: Yosemite [User impact if declined]: button colors will look odd on OS X Yosemite when released (rumored to be in the next two weeks) [Describe test coverage new/current, TBPL]: little, mostly styling only patch (some minor widget additions which have parsing tests) [Risks and why]: we might still have missed buttons somewhere in the UI that override the button styling but don't override the foreground color (like the issues from bug 1068913, bug 1072992, bug 1076252). Fixing those kinds of issues is pretty trivial, however, and this has been in Nightly for 2 weeks now, so I expect this to be OK. 2 out of the 3 regressions were also 10.10-only, so to a certain degree, the people most affected by this are also where the risk is, and I think that "with this patch" is a better state than "without this patch", even if there are more not-correctly-styled buttons lurking in odd places in the UI, because it'd still be fixing the large majority. [String/UUID change made/needed]: nope
Attachment #8501083 -
Flags: review+
Attachment #8501083 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•7 years ago
|
Attachment #8498102 -
Attachment is obsolete: true
Comment 23•7 years ago
|
||
Comment on attachment 8501083 [details] [diff] [review] Patch for aurora Aurora+
Attachment #8501083 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•7 years ago
|
status-firefox33:
--- → wontfix
status-firefox34:
--- → affected
status-firefox35:
--- → fixed
tracking-firefox34:
--- → +
Comment 25•7 years ago
|
||
Marking as Verified on Firefox 35 based on comment 15 and comment 16.
You need to log in
before you can comment on or make changes to this bug.
Description
•