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

Status: RESOLVED FIXED

(Toolkit :: Find Toolbar, defect)

Description

[Richard Marti (:Paenglab)]

Bug 1084598 added the new 10.10 focus styling for the URL- and Search-bar. The findbar should use the same styling.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: findBar.patch

I removed also a semicolon in shared.inc which was too much.

Comment 2

[Mike de Boer [:mikedeboer]]

Comment on attachment 8769488 [details] [diff] [review]
findBar.patch

I'd like to review this, if you don't mind ;-)

Comment 3

[Mike de Boer [:mikedeboer]]

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/

Comment 4

[Markus Stange [:mstange]]

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 5

[Dão Gottwald [:dao]]

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.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/93f095b7b772
Use the new 10.10 focus styling for the findbar-textbox and -buttons. r=dao

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/93f095b7b772