All users were logged out of Bugzilla on October 13th, 2018

Use the new Yosemite focus styling for the findbar-textbox and -buttons

RESOLVED FIXED in Firefox 50

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Paenglab, Assigned: Paenglab)

Tracking

unspecified
mozilla50
All
Mac OS X
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Bug 1084598 added the new 10.10 focus styling for the URL- and Search-bar. The findbar should use the same styling.
(Assignee)

Comment 1

2 years ago
Created attachment 8769488 [details] [diff] [review]
findBar.patch

I removed also a semicolon in shared.inc which was too much.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8769488 - Flags: review?(mstange)
Comment on attachment 8769488 [details] [diff] [review]
findBar.patch

I'd like to review this, if you don't mind ;-)
Attachment #8769488 - Flags: review?(mstange) → review?(mdeboer)
Comment on attachment 8769488 [details] [diff] [review]
findBar.patch

Review of attachment 8769488 [details] [diff] [review]:
-----------------------------------------------------------------

I think this will look good, but I don't have time to check it because I'm on vacation right now. Dão, can you do that for me please?

::: toolkit/themes/osx/global/shared.inc
@@ +1,5 @@
>  %filter substitution
>  
>  %define loweredShadow 0 1px rgba(255, 255, 255, .4)
>  %define focusRingShadow 0 0 1px -moz-mac-focusring inset, 0 0 4px 1px -moz-mac-focusring, 0 0 1.5px 1px -moz-mac-focusring
> +%define yosemiteFocusRingShadow 0 0 0 0.5px -moz-mac-focusring inset, 0 0 0 2px -moz-mac-focusring

s/yosemiteFocusRingShadow/focusRingShadowYosemite/
s/0.5px/.5px/
Attachment #8769488 - Flags: review?(mdeboer) → review?(dao+bmo)
Thanks Richard!

(In reply to Mike de Boer [:mikedeboer] (On vacation until 25-07, please ask for help at #fx-team) from comment #3)
> Comment on attachment 8769488 [details] [diff] [review]
> findBar.patch
> 
> Review of attachment 8769488 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think this will look good, but I don't have time to check it because I'm
> on vacation right now. Dão, can you do that for me please?

I've tested it, it works. The only unfortunate thing is that the focus ring corners aren't rounded at the corners where the text field / button itself isn't rounded. But I don't see a way to fix that using box-shadows.

> ::: toolkit/themes/osx/global/shared.inc
> @@ +1,5 @@
> >  %filter substitution
> >  
> >  %define loweredShadow 0 1px rgba(255, 255, 255, .4)
> >  %define focusRingShadow 0 0 1px -moz-mac-focusring inset, 0 0 4px 1px -moz-mac-focusring, 0 0 1.5px 1px -moz-mac-focusring
> > +%define yosemiteFocusRingShadow 0 0 0 0.5px -moz-mac-focusring inset, 0 0 0 2px -moz-mac-focusring
> 
> s/yosemiteFocusRingShadow/focusRingShadowYosemite/

Do you want him to fix the other existing users of this %define, too? (He's not adding this line; all he's doing is removing the semicolon at the end.)
Comment on attachment 8769488 [details] [diff] [review]
findBar.patch

In retrospect we probably should have made yosemite focus ring the default and use a pre-yosemite version check for the old focus ring. Makes it easier to phase out the old stuff when dropping support for those OS versions.
Attachment #8769488 - Flags: review?(dao+bmo) → review+
(Assignee)

Updated

2 years ago
Keywords: checkin-needed

Comment 6

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/93f095b7b772
Use the new 10.10 focus styling for the findbar-textbox and -buttons. r=dao
Keywords: checkin-needed

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/93f095b7b772
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.