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)
Firefox
Theme
Tracking
()
VERIFIED
FIXED
Firefox 46
People
(Reporter: bbell, Assigned: markh)
References
Details
Attachments
(4 files, 2 obsolete files)
13.12 KB,
image/png
|
Details | |
332.67 KB,
image/png
|
Details | |
5.27 KB,
application/zip
|
Details | |
15.98 KB,
patch
|
Dolske
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Current icons is too light, needs to be darker.
Assignee | ||
Comment 2•9 years ago
|
||
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)
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.
Assignee | ||
Comment 6•8 years ago
|
||
The new icons initially landed in bug 1074937, which is on 44. I guess that will mean we want to uplift this.
Blocks: 1074937
status-firefox44:
--- → affected
Assignee | ||
Comment 8•8 years ago
|
||
Screencap of the awesomebar with the darker icons.
Assignee | ||
Comment 9•8 years ago
|
||
(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.
Comment 10•8 years ago
|
||
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)
Updated•8 years ago
|
Attachment #8692782 -
Flags: review?(dolske) → review-
Reporter | ||
Comment 11•8 years ago
|
||
example of the icon working on different backgrounds.
Flags: needinfo?(bbell)
Reporter | ||
Comment 12•8 years ago
|
||
Sorry about the background being white... this should fix that.
Attachment #8689246 -
Attachment is obsolete: true
Assignee | ||
Comment 13•8 years ago
|
||
I've verified these versions have a transparent background.
Attachment #8692782 -
Attachment is obsolete: true
Attachment #8701242 -
Flags: review?(dolske)
Comment 14•8 years ago
|
||
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+
Assignee | ||
Comment 16•8 years ago
|
||
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?
Comment 17•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2b2b7a649d50
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Comment 18•8 years ago
|
||
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 19•8 years ago
|
||
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+
Assignee | ||
Comment 20•8 years ago
|
||
(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 :)
Comment 21•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/c42506273272
Comment 22•8 years ago
|
||
(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)
Comment 23•8 years ago
|
||
(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]).
Assignee | ||
Comment 24•8 years ago
|
||
(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)
Comment 25•8 years ago
|
||
(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)
Comment 26•8 years ago
|
||
(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)
Comment 27•8 years ago
|
||
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]
Comment 28•8 years ago
|
||
Thanks Mohammad for verifying this issue!
You need to log in
before you can comment on or make changes to this bug.
Description
•