Closed Bug 1389640 Opened 3 years ago Closed 3 years ago

Location Bar Focus State Missing with Light and Dark Themes

Categories

(Firefox :: Theme, defect, P1)

57 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 57
Iteration:
57.2 - Aug 29
Tracking Status
firefox57 --- verified

People

(Reporter: shorlander, Assigned: daleharvey)

References

(Blocks 1 open bug)

Details

(Whiteboard: [reserve-photon-visual])

Attachments

(1 file)

After switching to the Light or Dark theme the location bar no longer has focus state styling.
Priority: P1 → --
Whiteboard: [photon-visual] → [photon-visual] [triage]
Assignee: nobody → dharvey
Status: NEW → ASSIGNED
Iteration: --- → 57.2 - Aug 29
Flags: qe-verify?
Priority: -- → P1
Whiteboard: [photon-visual] [triage] → [reserve-photon-visual]
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 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?
Attachment #8898231 - Flags: review?(dao+bmo)
Comment on attachment 8898231 [details]
Bug 1389640 - Restore focus states for url bar in light/dark themes.

https://reviewboard.mozilla.org/r/169590/#review174854
Attachment #8898231 - Flags: review?(dao+bmo) → review+
Pushed by dharvey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f4665a357f3a
Restore focus states for url bar in light/dark themes. r=dao
https://hg.mozilla.org/mozilla-central/rev/f4665a357f3a
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Duplicate of this bug: 1391399
Flags: qe-verify? → qe-verify+
QA Contact: ovidiu.boca
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.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.