Closed Bug 1419898 Opened 3 years ago Closed 3 years ago

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

Categories

(Firefox :: Theme, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- unaffected
firefox59 --- fixed

People

(Reporter: dao, Assigned: ntim)

References

Details

(Keywords: regression)

Attachments

(1 file)

Bug 1418603 seems to have regressed this.
Flags: needinfo?(ntim.bugs)
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.
Attachment #8931349 - Flags: review?(dao+bmo)
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)
Flags: needinfo?(ntim.bugs)
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.
Attachment #8931349 - Flags: review?(dao+bmo) → review+
Pushed by ntim.bugs@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/613018fe3f81
Fix address bar border hover state on lightweight themes. r=dao
https://hg.mozilla.org/mozilla-central/rev/613018fe3f81
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Depends on: 1423622
You need to log in before you can comment on or make changes to this bug.