Closed Bug 1287763 Opened 8 years ago Closed 8 years ago

[10.10+] update remaining OS X textbox focus styling

Categories

(Firefox :: Theme, defect, P3)

Product:
Firefox
Component:
Theme
Platform:
x86
macOS
Type:
defect
Points:
3

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox50 --- affected
firefox55 --- fixed

People

(Reporter: Gijs, Assigned: sfoster)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [qx:spec])

Attachments

(1 file)

Bug 1287763 - Use flat yosemite-style border-box for focused styling in OSX, in more places
59 bytes, text/x-review-board-request
Gijs
: review+
Details
+++ This bug was initially created as a clone of Bug #1084598 +++ Some text fields still have the pre-Yosemite glowy effect, and those should get the flatter, different blue. See bug 1084598 for examples - generally this will boil down to adding media-queried copies of these styles and updating the box-shadow appropriately. Alternatively, it /may/ be possible to use CSS variables to make the whole thing slightly less repetitive, but because of the wide range of selectors I'm not entirely sure if that's feasible in the general case. A list: the "edit bookmark" panel input boxes: http://searchfox.org/mozilla-central/rev/65bed54efcce67cf04a890f7fe253ccdfa6befdc/browser/themes/osx/browser.css#2096 the folder tree and tag selector in the same: http://searchfox.org/mozilla-central/rev/65bed54efcce67cf04a890f7fe253ccdfa6befdc/browser/themes/osx/browser.css#2229 the tabbrowser tab focus rings: http://searchfox.org/mozilla-central/rev/65bed54efcce67cf04a890f7fe253ccdfa6befdc/browser/themes/osx/browser.css#2558 In toolkit, XUL link styling: http://searchfox.org/mozilla-central/rev/65bed54efcce67cf04a890f7fe253ccdfa6befdc/toolkit/themes/osx/global/global.css#231 notification button styling: http://searchfox.org/mozilla-central/rev/65bed54efcce67cf04a890f7fe253ccdfa6befdc/toolkit/themes/osx/global/global.css#257,261 and a second time, the notification button styling: http://searchfox.org/mozilla-central/rev/65bed54efcce67cf04a890f7fe253ccdfa6befdc/toolkit/themes/osx/global/notification.css#136 and inputs in the editable tree: http://searchfox.org/mozilla-central/rev/65bed54efcce67cf04a890f7fe253ccdfa6befdc/toolkit/themes/osx/global/tree.css#286
I'm not sure how feasible it will be to further reduce repetition beyond using the @yosemiteFocusRingShadow@ variable, but I'll get an initial patch together and we can see.
Assignee: nobody → sfoster
Attachment 8844703 [details] is a first pass at the browser/ parts of this bug. I'm reusing the @yosemiteFocusRingShadow@ macro. I guess these preprocessor macros pre-date CSS variables? They are sorta convenient in that they can expand to multiple property/value pairs, but I'm not sure that outweighs the value of being clear/explicit and having valid CSS files. I think I'm going to try to refactor the focusRingShadow / yosemiteFocusRingShadow stuff to use CSS variables for the box-shadow values and see how that plays out. That could consolidate most of the @media blocks in one place in the theme's global.css where we define the variables.
(In reply to Sam Foster [:sfoster] from comment #3) > I guess these > preprocessor macros pre-date CSS variables? They are sorta convenient in > that they can expand to multiple property/value pairs, but I'm not sure that > outweighs the value of being clear/explicit and having valid CSS files. Yes, they pre-date CSS variables. The other difference beyond what you've already listed is that CSS variables sometimes have performance costs that build-time replacement/inclusion does not. I doubt we'll really run into this here though, as this is only about focus styles, but it's something to keep in mind.
Comment on attachment 8844703 [details] Bug 1287763 - Use flat yosemite-style border-box for focused styling in OSX, in more places https://reviewboard.mozilla.org/r/118014/#review120228 I have the variable in place for the box-shadow value list. I stopped short of replacing all the existing @focusRingShadow@ / @yosemiteFocusRingShadow usage with the new variable in this patch. I thought to do that in a follow-up?
(In reply to Sam Foster [:sfoster] from comment #6) > Comment on attachment 8844703 [details] > Bug 1287763 - Use flat yosemite-style border-box for focused styling in OSX, > in more places > > https://reviewboard.mozilla.org/r/118014/#review120228 > > I have the variable in place for the box-shadow value list. I stopped short > of replacing all the existing @focusRingShadow@ / @yosemiteFocusRingShadow > usage with the new variable in this patch. I thought to do that in a > follow-up? That works for me. Did you mean to request review on this patch and/or is it complete beyond what you just mentioned? :-)
Flags: needinfo?(sfoster)
Attachment #8844703 - Flags: review?(gijskruitbosch+bugs)
> That works for me. Did you mean to request review on this patch and/or is it > complete beyond what you just mentioned? :-) Still figuring out reviewboard :/ I had to click 3 times just now to actually actually submit the review request? I'm not sure if we have any reftests that will need updating with this change. Probably not?
Flags: needinfo?(sfoster)
Comment on attachment 8844703 [details] Bug 1287763 - Use flat yosemite-style border-box for focused styling in OSX, in more places https://reviewboard.mozilla.org/r/118014/#review120992 LGTM, thanks! Please remember to file a followup for the remaining ones (findbar, by the looks of it, and a few other ones in the OS X toolkit theme?)
Attachment #8844703 - Flags: review?(gijskruitbosch+bugs) → review+
Follow-up filed as bug 1346280.
Blocks: 1346280
Pushed by sfoster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/057d31114af0 Use flat yosemite-style border-box for focused styling in OSX, in more places r=Gijs
https://hg.mozilla.org/mozilla-central/rev/057d31114af0
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: