Closed Bug 1224653 Opened 9 years ago Closed 8 years ago

Replace existing globe icons for site missing a fav-icon with an new darker version.

Categories

(Firefox :: Theme, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 46
Tracking Status
firefox44 --- affected
firefox45 --- verified
firefox46 --- verified

People

(Reporter: bbell, Assigned: markh)

References

Details

Attachments

(4 files, 2 obsolete files)

Current icons is too light, needs to be darker.
Assignee: nobody → markh
And they need to invert on hover in the Awesome bar.
Bryan, can you please attach the version you want me to use? And if comment 1 is accurate, I guess I'll also need the inverted version.
Flags: needinfo?(bbell)
Attached file Globe Icon.zip (obsolete) —
Here is the replacement icons for the Globe. I worked hard to make them work well even on a dark background. So I don't think we need the inverted icon.
Flags: needinfo?(bbell)
I've noticed the currently the globe icon is not Retina on the Mac
Currently all "actions" icons (search for example) are inverted in the awesome bar on hover. I don't know why the globe icon should differ.
The new icons initially landed in bug 1074937, which is on 44. I guess that will mean we want to uplift this.
Darker icons
Attachment #8692782 - Flags: review?(dolske)
Attached image darker-icons.png
Screencap of the awesomebar with the darker icons.
(In reply to Guillaume C. [:ge3k0s] from comment #1)
> And they need to invert on hover in the Awesome bar.

I think we should open a new bug for that, blocking bug 1074937, so this bug can land the improved icons sooner.
The current icons are on a transparent background, but the icons in attachment 8689246 [details] are on a white background. I.E., on a dark background you're get a globe on a white square, which seems wrong.

I suspect Bryan just forgot to turn off a background layer for testing -- can get get a fixed asset with the transparent background? (I didn't check the actual patch, apologies if that got fixed outside this bug.)

I'm a bit ambivalent on having an inverted flavor of the icon... Maybe it should to match the search icon, but normal favicons don't ever invert. So it probably doesn't matter much either way. But if I had to pick I'd probably do it, just to avoid the slight glow that's there to accommodate dark backgrounds. Then we'd have nice flat/monochrome glyphs. Not something that needs to happen in this bug. :)
Flags: needinfo?(bbell)
Attachment #8692782 - Flags: review?(dolske) → review-
Attached image Example
example of the icon working on different backgrounds.
Flags: needinfo?(bbell)
Sorry about the background being white... this should fix that.
Attachment #8689246 - Attachment is obsolete: true
I've verified these versions have a transparent background.
Attachment #8692782 - Attachment is obsolete: true
Attachment #8701242 - Flags: review?(dolske)
Comment on attachment 8701242 [details] [diff] [review]
0001-Bug-1224653-Replace-existing-globe-icons-for-site-mi.patch

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

rs=me

I'm a little amused at the change in size and image format... It started life as a 445 byte 16-bit greyscale (!) PNG, and doubled in size to a 979 8-bit indexed-color PNG. As a 32-bit RGBA images + pngcrush, it sits in-between at 728 bytes. None of this matters, other than it being a sign it would be nice to have an automated image-optimizing process. :)
Attachment #8701242 - Flags: review?(dolske) → review+
Comment on attachment 8701242 [details] [diff] [review]
0001-Bug-1224653-Replace-existing-globe-icons-for-site-mi.patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1202712
[User impact if declined]: The new favicon which landed in 45 is too light in some contexts. Given this is a highly visible change we should get the better icon riding the trains to release.
[Describe test coverage new/current, TreeHerder]: N/A
[Risks and why]: Very low risk, only changes image assets.
[String/UUID change made/needed]: None.
Attachment #8701242 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/2b2b7a649d50
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Mark, in comment #6, you say that it landed in 44 (and marked it as affected) and in comment #16, you say 45 and only requested aurora. So, 44 or 45 ? :)
Comment on attachment 8701242 [details] [diff] [review]
0001-Bug-1224653-Replace-existing-globe-icons-for-site-mi.patch

45 is affected anyway :)
Attachment #8701242 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Sylvestre Ledru [:sylvestre] from comment #18)
> Mark, in comment #6, you say that it landed in 44 (and marked it as
> affected) and in comment #16, you say 45 and only requested aurora. So, 44
> or 45 ? :)

Yeah, sorry, the too-light favicon landed in 44 (and the correct bug is bug 1074937) - so -beta too :)
(In reply to Mark Hammond [:markh] from comment #8)
> Created attachment 8692783 [details]
> darker-icons.png
> 
> Screencap of the awesomebar with the darker icons.

Honestly that doesn't look great at all on that dark background. Similarly on Ubuntu where popups are dark by default, the globe is close to invisible. This outcome might be worse than the problem this bug tried to fix in the first place (which would mean that we should back this out). I'm not sure what that original problem was, though, since comment 0 doesn't say it.

Regardless of whether or not we back out, I think this needs more work. The darker and lighter portions of the icon should have more contrast such that the icon is visible on dark and light backgrounds.
Flags: needinfo?(markh)
Flags: needinfo?(dolske)
Flags: needinfo?(bbell)
(In reply to Dão Gottwald [:dao] from comment #22)
> (In reply to Mark Hammond [:markh] from comment #8)
> > Created attachment 8692783 [details]
> > darker-icons.png
> > 
> > Screencap of the awesomebar with the darker icons.
> 
> Honestly that doesn't look great at all on that dark background.

Ooops, I meant to reply to comment 11 (attachment 8699800 [details]).
(In reply to Dão Gottwald [:dao] from comment #22)
> Honestly that doesn't look great at all on that dark background. Similarly
> on Ubuntu where popups are dark by default, the globe is close to invisible.
> This outcome might be worse than the problem this bug tried to fix in the
> first place (which would mean that we should back this out). I'm not sure
> what that original problem was, though, since comment 0 doesn't say it.

I'm going to defer to Bryan's judgement here given he is part of UX.
Flags: needinfo?(markh)
(In reply to Dão Gottwald [:dao] from comment #22)

> Regardless of whether or not we back out, I think this needs more work. The
> darker and lighter portions of the icon should have more contrast such that
> the icon is visible on dark and light backgrounds.

I don't think it's bad enough to back out... And when it's shown in the URL bar (as in the devedition part of attachment 8699800 [details]), it seems like a higher-contrast improvement to me.

How about we do a followup (see comment 10 :) to add an inverted version of the icon? That would at least address the awesomebar dropdown case, and lets us fix this more definitively than tweaking shades of gray to work on all backgrounds.
Flags: needinfo?(dolske)
(In reply to Justin Dolske [:Dolske] from comment #25)
> I don't think it's bad enough to back out... And when it's shown in the URL
> bar (as in the devedition part of attachment 8699800 [details]), it seems
> like a higher-contrast improvement to me.
> 
> How about we do a followup (see comment 10 :) to add an inverted version of
> the icon? That would at least address the awesomebar dropdown case, and lets
> us fix this more definitively than tweaking shades of gray to work on all
> backgrounds.

Ok, filed bug 1238907. I think we need more contrast in this icon rather than a separate inverted icon, because backgrounds vary depending on the OS theme and Firefox theme and it's not always easy to switch to a different icon based on these factors.
Flags: needinfo?(bbell)
I have reproduced this bug with Firefox Nightly 45.0a1 (Build ID: 20151113030248) on 
windows 8.1 64-bit.

Verified as fixed with latest Firefox beta 45.0b3 (Build ID: 20160204142810)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0

Verified as fixed with latest Firefox aurora 46.0a2 (Build ID: 20160206004009)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
QA Whiteboard: [testday-20160205]
Thanks Mohammad for verifying this issue!
Status: RESOLVED → VERIFIED
Depends on: 1291770
You need to log in before you can comment on or make changes to this bug.