Closed Bug 1173749 Opened 9 years ago Closed 9 years ago

URLs in the URL bar's autocomplete popup should use a lighter blue

Categories

(Firefox :: Theme, defect)

Unspecified
Windows 10
defect
Not set
normal

Tracking

()

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

People

(Reporter: dao, Assigned: dao)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

I believe we're using -moz-nativehyperlinktext at the moment. http://people.mozilla.org/~shorlander/mockups/Windows-10/Windows-10.html wants something lighter.
Attached patch patchSplinter Review
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #8621542 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8621542 [details] [diff] [review]
patch

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

I'm a little confused over the media query here; the only non-default themes right now are high-contrast ones, is there a reason not to do the same there?

Likewise, is there a particular reason not to apply the same styling on win8?
Attachment #8621542 - Flags: review?(gijskruitbosch+bugs) → review+
(In reply to :Gijs Kruitbosch from comment #2)
> I'm a little confused over the media query here; the only non-default themes
> right now are high-contrast ones, is there a reason not to do the same there?

Highlight is not a proper text color, it's a background color that's supposed to be used in conjunction with HighlightText, so there's no guarantee this will be legible with different high contrast themes.

> Likewise, is there a particular reason not to apply the same styling on win8?

dunno
(In reply to Dão Gottwald [:dao] from comment #3)
> (In reply to :Gijs Kruitbosch from comment #2)
> > I'm a little confused over the media query here; the only non-default themes
> > right now are high-contrast ones, is there a reason not to do the same there?
> 
> Highlight is not a proper text color, it's a background color that's
> supposed to be used in conjunction with HighlightText, so there's no
> guarantee this will be legible with different high contrast themes.

OK. Should we hardcode this to an rgb value instead of relying on this constant, in case win10 adds more default themes where this doesn't work?

> > Likewise, is there a particular reason not to apply the same styling on win8?
> 
> dunno

Mmk, let's leave that for now then.
(In reply to :Gijs Kruitbosch from comment #4)
> > Highlight is not a proper text color, it's a background color that's
> > supposed to be used in conjunction with HighlightText, so there's no
> > guarantee this will be legible with different high contrast themes.
> 
> OK. Should we hardcode this to an rgb value instead of relying on this
> constant, in case win10 adds more default themes where this doesn't work?

If Windows 10 adds another theme we don't know about yet, the rgb value could similarly be illegible, so we probably shouldn't consider that a default theme in the first place.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1fd07cd84f39
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Comment on attachment 8621542 [details] [diff] [review]
patch

Approval Request Comment
[Feature/regressing bug #]: windows 10
[User impact if declined]: just subtle aesthetics
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: trivial CSS patch, not risky
[String/UUID change made/needed]: none
Attachment #8621542 - Flags: approval-mozilla-beta?
Attachment #8621542 - Flags: approval-mozilla-aurora?
Attachment #8621542 - Flags: approval-mozilla-beta?
Flags: qe-verify+
Verified fixed on Nightly 41.0a1 (2015-06-15), using Windows 10 Pro Insider Preview (build 10130).

The mock available in Comment 0 shows #0096FF as a highlight color, while current Nightly shows #3399FF, which is really close as far as the eye can tell. I assume that the fact that they're not exactly the same is intended - Dão, please correct me if I'm wrong.

Spot-checked high-contrast themes as well, to make sure that no regressions were introduced by this change.
Status: RESOLVED → VERIFIED
Flags: qe-verify+ → needinfo?(dao)
(In reply to Andrei Vaida, QA [:avaida] from comment #9)
> The mock available in Comment 0 shows #0096FF as a highlight color, while
> current Nightly shows #3399FF, which is really close as far as the eye can
> tell. I assume that the fact that they're not exactly the same is intended -

Yes. The mockup uses the same color for the URLs as for the selected entry's background. We do the same here. The color being slightly different from the native Highlight color would be a mistake in the mockup.
Flags: needinfo?(dao)
Comment on attachment 8621542 [details] [diff] [review]
patch

Verified + windows 10, taking it for 40.
Attachment #8621542 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified fixed on Beta 40.0b2 (20150706172413) as well, using Windows 10 Pro x64 (Build 10158).
You need to log in before you can comment on or make changes to this bug.