1389640 - Location Bar Focus State Missing with Light and Dark Themes

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Stephen Horlander [:shorlander]]

After switching to the Light or Dark theme the location bar no longer has focus state styling.

Comment 1

[Dale Harvey (:daleharvey)]

Attachment: Bug 1389640 - Restore focus states for url bar in light/dark themes.

Review commit: https://reviewboard.mozilla.org/r/169590/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/169590/

Comment 2

[Dale Harvey (:daleharvey)]

So the spec here (http://design.firefox.com/people/shorlander/photon/Mockups/macOS.html) shows we want to share the box shadow and border focus / active states between compact / non compact and light / dark / default themes. 

Not entirely certain what requires the overrides here (possibly different requirements with the previous spec?), but seems like the best is to either remove the rules or at least the !important, or add new duplicated !important rules for :hover etc which seems bad. Also changed the specificity so the search box gets the active states.

Comment 3

[Dão Gottwald [:dao]]

Comment on attachment 8898231 [details]
Bug 1389640 - Restore focus states for url bar in light/dark themes.

https://reviewboard.mozilla.org/r/169590/#review174850

::: browser/themes/shared/compacttheme.inc.css:110
(Diff revision 1)
>    text-shadow: none !important;
>  }
>  
>  /* URL bar and search bar*/
>  #urlbar,
> -#navigator-toolbox .searchbar-textbox {
> +.searchbar-textbox {

I suspect #navigator-toolbox is still needed here for when teh search bar is moved to the overflow panel.

::: browser/themes/shared/compacttheme.inc.css:114
(Diff revision 1)
>  #urlbar,
> -#navigator-toolbox .searchbar-textbox {
> +.searchbar-textbox {
>    background-color: var(--url-and-searchbar-background-color) !important;
>    background-image: none !important;
>    color: inherit !important;
> -  border: 1px solid var(--chrome-nav-bar-controls-border-color) !important;
> +  border: 1px solid var(--chrome-nav-bar-controls-border-color);

Should probably just set border-color in a rule using :not([focused="true"])

::: browser/themes/shared/compacttheme.inc.css:115
(Diff revision 1)
> -#navigator-toolbox .searchbar-textbox {
> +.searchbar-textbox {
>    background-color: var(--url-and-searchbar-background-color) !important;
>    background-image: none !important;
>    color: inherit !important;
> -  border: 1px solid var(--chrome-nav-bar-controls-border-color) !important;
> -  box-shadow: none !important;
> +  border: 1px solid var(--chrome-nav-bar-controls-border-color);
> +  box-shadow: none;

Is this still useful at all?

Comment 4

[Dale Harvey (:daleharvey)]

Comment on attachment 8898231 [details]
Bug 1389640 - Restore focus states for url bar in light/dark themes.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/169590/diff/1-2/

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 8898231 [details]
Bug 1389640 - Restore focus states for url bar in light/dark themes.

https://reviewboard.mozilla.org/r/169590/#review174854

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4665a357f3a
Restore focus states for url bar in light/dark themes. r=dao

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f4665a357f3a

Comment 9

[ovidiu boca[:Ovidiu]]

I verified this on Mac OS X 10.10, Windows 10 and Ubuntu 16.04 and I can't reproduce the issue any more. I will mark as verified fixed.