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

RESOLVED FIXED in Firefox 59

Status

()

defect
P2
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: dao, Assigned: ntim)

Tracking

({regression})

Trunk
Firefox 59
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox57 unaffected, firefox58 unaffected, firefox59 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

2 years ago
Bug 1418603 seems to have regressed this.
Flags: needinfo?(ntim.bugs)
Comment hidden (mozreview-request)
Reporter

Comment 2

2 years ago
mozreview-review
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)
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Flags: needinfo?(ntim.bugs)
Reporter

Comment 5

2 years ago
mozreview-review
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+
Comment hidden (mozreview-request)

Comment 7

2 years ago
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

Comment 8

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/613018fe3f81
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
Assignee

Updated

2 years ago
Depends on: 1423622
You need to log in before you can comment on or make changes to this bug.