Closed Bug 1176252 Opened 4 years ago Closed 4 years ago

Use a white magnifying glass icon in the awesomebar when an entry is highlighted

Categories

(Firefox :: Theme, defect)

Unspecified
Windows 10
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 41
Tracking Status
firefox40 --- verified
firefox41 --- verified

People

(Reporter: phlsa, Assigned: jaws)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 2 obsolete files)

Attached image current ui
As part of the Windows 10 theme update, selected rows in the awesomebar dropdown now have a blue background. In those cases, the magnifying glass icon that indicates searches should change its color to white (as it does on OS X).
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Attached patch Patch (obsolete) — Splinter Review
autocomplete.css sets the custom gradient background color for windows-vista and windows7, so this rule only inverts the search icon when on the highlight background (using highlighttext).
Attachment #8624825 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8624825 [details] [diff] [review]
Patch

Review of attachment 8624825 [details] [diff] [review]:
-----------------------------------------------------------------

r=me for a different patch:

::: browser/themes/windows/browser.css
@@ +1474,5 @@
>    }
>  }
>  
> +@media not all and (-moz-os-version: windows-vista) and (-moz-windows-default-theme),
> +       not all and (-moz-os-version: windows-win7) and (-moz-windows-default-theme) {

Replace the media query here:

http://hg.mozilla.org/mozilla-central/annotate/2694ff2ace6a/browser/themes/windows/browser.css#l1468

With this one instead.
Attachment #8624825 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs Kruitbosch from comment #2)
> Comment on attachment 8624825 [details] [diff] [review]
> Patch
> 
> Review of attachment 8624825 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me for a different patch:
> 
> ::: browser/themes/windows/browser.css
> @@ +1474,5 @@
> >    }
> >  }
> >  
> > +@media not all and (-moz-os-version: windows-vista) and (-moz-windows-default-theme),
> > +       not all and (-moz-os-version: windows-win7) and (-moz-windows-default-theme) {
> 
> Replace the media query here:
> 
> http://hg.mozilla.org/mozilla-central/annotate/2694ff2ace6a/browser/themes/
> windows/browser.css#l1468
> 
> With this one instead.

Wait, in fact, how does this media query work? Doesn't this say:

if it's not the case that (we're on vista and default theme)

OR

if it's not the case that (we're on 7 and default theme)


which is always true?
Attached patch Patch v2 (obsolete) — Splinter Review
Okay, tweaking the media query...
Attachment #8624825 - Attachment is obsolete: true
Attachment #8624836 - Flags: review?(gijskruitbosch+bugs)
Attached patch Patch v2.01Splinter Review
Attachment #8624836 - Attachment is obsolete: true
Attachment #8624836 - Flags: review?(gijskruitbosch+bugs)
Attachment #8624837 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8624837 [details] [diff] [review]
Patch v2.01

Review of attachment 8624837 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/windows/browser.css
@@ +1467,5 @@
>  
> +@media not all and (-moz-os-version: windows-vista) and (-moz-windows-default-theme) {
> +  @media not all and (-moz-os-version: windows-win7) and (-moz-windows-default-theme) {
> +    .ac-result-type-keyword[selected="true"],
> +    .autocomplete-treebody::-moz-tree-image(keyword, treecolAutoCompleteImage, selected),

And we're sure we want these 2 things as well, right? Only realized afterwards that this came with extra selector baggage. Sorry. :-\
Attachment #8624837 - Flags: review?(gijskruitbosch+bugs) → review+
Yeah, we want the other ones too because those need to be inverted as well. See http://screencast.com/t/kdLsp6qE for an example.
https://hg.mozilla.org/mozilla-central/rev/bdb753f10694
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Flags: qe-verify+
Reproduced the issue on Nightly 41.0a1 (2015-06-22).

This is verified fixed as of Nightly 41.0a1 (2015-06-23), using Windows 10 Pro Insider Preview (build 10130, 64-bit architecture).
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Attached patch Patch for 40Splinter Review
Approval Request Comment
[Feature/regressing bug #]: Windows 10 HiDPI
[User impact if declined]: HiDPI icons on Windows are blurry
[Describe test coverage new/current, TreeHerder]: on mozilla-central for much of 41-nightly
[Risks and why]: none expected 
[String/UUID change made/needed]: none

https://hg.mozilla.org/try/pushloghtml?changeset=71084a9edf1e
Attachment #8627210 - Flags: approval-mozilla-beta?
Comment on attachment 8627210 [details] [diff] [review]
Patch for 40

Verified visual fix for Windows 10. Beta+
Attachment #8627210 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Verified fixed on Beta 40.0b2 (20150706172413) as well, using Windows 10 Pro x64 (Build 10158) with "browser.urlbar.unifiedcomplete" set to "true".

The magnifying glass turns white when a search entry from the awesomebar is highlighted.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.