1419898 - Address bar border doesn't change on hover when using a lightweight theme

Status: RESOLVED FIXED

(Firefox :: Theme, defect, P2)

Description

[Dão Gottwald [:dao]]

Bug 1418603 seems to have regressed this.

Comment 1

[Tim Nguyen :ntim]

Attachment: Bug 1419898 - Fix address bar border hover state on lightweight themes.

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

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8931349 [details]
Bug 1419898 - Fix address bar border hover state on lightweight themes.

https://reviewboard.mozilla.org/r/202490/#review207818

::: browser/themes/shared/urlbar-searchbar.inc.css:55
(Diff revision 1)
>    border-color: var(--url-and-searchbar-border-color, hsla(240,5%,5%,.25));
>  }
>  
> +#urlbar:not([focused="true"]):-moz-lwtheme:hover,
> +#navigator-toolbox .searchbar-textbox:not([focused="true"]):-moz-lwtheme:hover {
> +  border-color: var(--url-and-searchbar-border-color, hsla(240,5%,5%,.35));

I'd prefer if this stylesheet didn't repeat hsla(240,5%,5%,.35) to set the same border for non-lwt and lwt. Maybe use %define for that?

::: browser/themes/shared/urlbar-searchbar.inc.css:55
(Diff revision 1)
>    border-color: var(--url-and-searchbar-border-color, hsla(240,5%,5%,.25));
>  }
>  
> +#urlbar:not([focused="true"]):-moz-lwtheme:hover,
> +#navigator-toolbox .searchbar-textbox:not([focused="true"]):-moz-lwtheme:hover {
> +  border-color: var(--url-and-searchbar-border-color, hsla(240,5%,5%,.35));

I think I already noted this elsewhere... I'd prefer if variables that exist only for lwt support used a common prefix.

Comment 3

[Andrew Overholt [:overholt]]

Tim, are you able to get back to this? (I'm asking because this is marked as a regression so it came up in our triage)

Comment 4

[Tim Nguyen :ntim]

Comment on attachment 8931349 [details]
Bug 1419898 - Fix address bar border hover state on lightweight themes.

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

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 8931349 [details]
Bug 1419898 - Fix address bar border hover state on lightweight themes.

https://reviewboard.mozilla.org/r/202490/#review211128

::: browser/themes/shared/urlbar-searchbar.inc.css:53
(Diff revision 2)
>    color: var(--url-and-searchbar-color, black);
>  }
>  
>  #urlbar:not([focused="true"]):-moz-lwtheme,
>  #navigator-toolbox .searchbar-textbox:not([focused="true"]):-moz-lwtheme {
> -  border-color: var(--url-and-searchbar-border-color, hsla(240,5%,5%,.25));
> +  border-color: var(--theme-toolbar-field-border-color, @fieldBorderColor@);

"theme" is ambiguous as the default theme is also a theme. This is a bit confusing and may make the prefix look pointless. Perhaps use "lwtheme"? That would be consistent with the pseudo classes.

Comment 6

[Tim Nguyen :ntim]

Comment on attachment 8931349 [details]
Bug 1419898 - Fix address bar border hover state on lightweight themes.

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

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/613018fe3f81
Fix address bar border hover state on lightweight themes. r=dao

Comment 8

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/613018fe3f81