1046693 - Gear on new tab page blurry

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect)

Description

[Manish Goregaokar [:manishearth]]

Attachment: Icon_Cog_Static.svg

Moved from bug 1044537

Aaron has already provided assets for this on that bug, copying them over.

Comment 1

[Manish Goregaokar [:manishearth]]

Attachment: Icon_Cog_Rollover.svg

Copying over other asset

Comment 2

[Manish Goregaokar [:manishearth]]

Attachment: Update with better assets

Comment 3

[Manish Goregaokar [:manishearth]]

Attachment: Patch

Whoops, wrong commit message

Comment 4

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 8465378 [details] [diff] [review]
Patch

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

::: browser/themes/shared/newtab/cog_rollover.svg
@@ +4,5 @@
> +<svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" x="0px" y="0px"
> +	 viewBox="0 0 32 32" enable-background="new 0 0 32 32" xml:space="preserve">
> +<g>
> +	
> +		<image overflow="visible" opacity="0.35" width="142" height="142" xlink:href="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAJIAAACSCAYAAACue5OOAAAACXBIWXMAAC4jAAAuIwF4pT92AAAA

Dear Adobe Illustrator: vector graphics, you're doing it wrong!

Illustrator has an awful habit of rasterizing shadows in particular :( Which rather defeats the purpose of SVG. 

Could we get these SVG files cleaned up so that:
* Shadow is done via CSS
* All the superfluous attributes/comments are removed
* DOCTYPE is removed (see bug 1023655)

::: browser/themes/shared/newtab/newTab.inc.css
@@ +60,5 @@
>  
>  /* CUSTOMIZE */
>  #newtab-customize-button {
>    background-color: transparent;
> +  background-image: url(chrome://browser/skin/newtab/cog_static.svg);

Nit: Quote URLs.

Comment 5

[Manish Goregaokar [:manishearth]]

Aaron, could you fix the shadow as listed above? Thanks.

Comment 6

[Manish Goregaokar [:manishearth]]

(I can fix the rest of the nits, just need to remove the rasterized shadow)

Comment 7

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Attachment: Icon_cog_hover.svg (blue)

Here's a blue graphic of the cog without a shadow as suggested in the other bug.
Aaron, what do you think of using this as the hover/active graphic?

Comment 8

[Aaron]

Attachment: Icon_Cog_Rollover.svg

New SVG without shadow attached. The only effect now will be a highlight, no more shadow since it adds little value and has been problematic.

Both Static and Rollover versions should render at 22 x 22 pixels.

Comment 9

[Aaron]

P.S. If it's too subtle, then I'm fine with the blue cog Phillip provided.

Comment 10

[Manish Goregaokar [:manishearth]]

Attachment: Blue patch

This is the patch with the blue cog

Comment 11

[Manish Goregaokar [:manishearth]]

Attachment: Gray patch (with gradient)

This is the one with the new gray cog provided by Aaron.

Both look pretty good, thoughts on which one we should pick?

Comment 12

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

Given that we're moving away from gradients more and more, let's use the blue version.

However, with the patch, the icon grows when hovering which is not intended. It should stay at the same size as in normal state.

Comment 13

[Manish Goregaokar [:manishearth]]

Attachment: Patch: blue, same size

Blue of the same size.

Created with the svg editor commonly known as vim ;)

Comment 14

[Ed Lee :Mardak]

Shouldn't we be able to just take the original svg file:
http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/newtab/controls.svg?raw=1

Remove the style glyphShape-style-hover-gear-dropshadow

And make the glyphShape-style-hover-gear style be a fill of #4A90E2 or whatever blue is desired?

Comment 15

[Manish Goregaokar [:manishearth]]

(In reply to Ed Lee :Mardak from comment #14)
> Shouldn't we be able to just take the original svg file:
> http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/newtab/
> controls.svg?raw=1
> 
> Remove the style glyphShape-style-hover-gear-dropshadow
> 
> And make the glyphShape-style-hover-gear style be a fill of #4A90E2 or
> whatever blue is desired?

I took the svg file attached above and changed the path fill to the blue from the other file. I'm not sure if we're supposed to apply hover effects to the SVG directly, though I'm open to doing that.

Comment 16

[Ed Lee :Mardak]

The existing svg contains the hover and active states of other controls. This also doesn't require adding additional svg files for static/rollover as those already exist in controls.svg.

The patch to update the cog icon hover/unhover should only need to touch that one controls.svg file, so no jar.mn nor added files.

Comment 17

[Manish Goregaokar [:manishearth]]

Okay, will do.

Comment 18

[Manish Goregaokar [:manishearth]]

Attachment: Blue on hover, uses same svg file

Incorporated Ed's comments.

Comment 19

[Ed Lee :Mardak]

Attachment: Blue on hover, uses same svg file screenshot

Seems to work for me!

Comment 20

[Ed Lee :Mardak]

Comment on attachment 8469866 [details] [diff] [review]
Blue on hover, uses same svg file

>+++ b/browser/themes/shared/newtab/controls.svg
>@@ -14,22 +14,17 @@
>       .glyphShape-style {
>         fill: #7a7a7a;
>       }
> 
>       .glyphShape-style-hover-gear {
>+        fill: #0092E5;
nit: the other colors are lowercase, so probably also match with #0092e5.

Although I believe the blue dcrobot provided is #4a90e2

Comment 21

[Ed Lee :Mardak]

dcrobot, just making sure which blue you were expecting for the hovered gear.

Comment 22

[Manish Goregaokar [:manishearth]]

(In reply to Ed Lee :Mardak from comment #21)
> dcrobot, just making sure which blue you were expecting for the hovered gear.

I took Phillip's blue (https://bug1046693.bugzilla.mozilla.org/attachment.cgi?id=8466173).

Comment 23

[Aaron]

Attachment: Icon_Cog_Rollover.svg

Ed, this is the new rollover cog icon. The blue is #4A90E2

Comment 24

[Manish Goregaokar [:manishearth]]

Attachment: Blue on hover, uses same svg file (different blue)

This is the "new" blue :)

Comment 25

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 8470125 [details] [diff] [review]
Blue on hover, uses same svg file (different blue)

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

r+ on the assumption that the blue for the gear should be different from the blue for the pin. But I'd like explicit confirmation on that before landing.

Comment 26

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Aaron, could you confirm comment 25? ie, that we want the blue used for the gear to be different from the blue used for the pin? Or should the hover color of the pin be updated to #4A90E2?

Comment 27

[Aaron]

Blair, I don't know why the pin was different. Please update the pin to #4A90E2 so that all "link blue" instances are consistent. Thanks!

Comment 28

[Manish Goregaokar [:manishearth]]

Attachment: Blue on hover, uses same svg file, updates pin blue for consistency

Comment 29

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Comment on attachment 8470906 [details] [diff] [review]
Blue on hover, uses same svg file, updates pin blue for consistency

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

The system works!

Comment 30

[Manish Goregaokar [:manishearth]]

(Does this need a try push? It's just a change to an image.)

Comment 31

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Nope.

Comment 32

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/7f1c6919a3ea

Comment 33

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7f1c6919a3ea

Comment 34

[Liz Henry (:lizzard) (relman/hg->git project)]

I can see the blurry gear on the 34.0a1 (2014-08-12) build, and on today's build, 34.0a1 (2014-08-13) it is not blurry and it's the new blue, as is the pin. Huzzah!

Comment 35

[Sylvestre Ledru [:Sylvestre]]

Uplift has been managed in bug 1057602

Comment 36

[Ed Lee :Mardak]

https://hg.mozilla.org/releases/mozilla-aurora/rev/24b8334087d6

Comment 37

[Cornel Ionce [:noni] [Hubs QA]]

Mozilla/5.0 (X11; Linux x86_64; rv:33.0) Gecko/20100101 Firefox/33.0

Verified fixed on latest Aurora, build ID: 20140828004002.

Comment 38

[Thomas Stache]

Attachment: Screenshot of Gear icon on Windows 7

I don't know, it appears blurry to me (on Windows 7). Has that maybe regressed? I expect being an SVG the icon should be crisp at all sizes, so is there a shadow looking like blur?

Comment 39

[Manish Goregaokar [:manishearth]]

(In reply to Thomas Stache from comment #38)
> Created attachment 8502409 [details]
> Screenshot of Gear icon on Windows 7
> 
> I don't know, it appears blurry to me (on Windows 7). Has that maybe
> regressed? I expect being an SVG the icon should be crisp at all sizes, so
> is there a shadow looking like blur?

What version are you running? I expect that this hasn't reached the release version of Firefox yet.

(The new icon is blue and has no shadow)

Comment 40

[Thomas Stache]

(In reply to Thomas Stache from comment #38)
> Created attachment 8502409 [details]
> Screenshot of Gear icon on Windows 7
> 
> I don't know, it appears blurry to me (on Windows 7). Has that maybe
> regressed? I expect being an SVG the icon should be crisp at all sizes, so
> is there a shadow looking like blur?

Oh, it's the "background-size: 28px auto;" rule that causes this for me!

Comment 41

[Thomas Stache]

(In reply to Manish Goregaokar [:manishearth] from comment #39)
> What version are you running? I expect that this hasn't reached the release
> version of Firefox yet.
> 
> (The new icon is blue and has no shadow)

Nightly and Aurora, doesn't matter. The icon is blue on hover.

Comment 42

[Thomas Stache]

See bug 955800

Comment 43

[Thomas Stache]

Filed bug 1081206 for investigation, as this one is already closed.

Comment 44

[Ed Lee :Mardak]

(In reply to Thomas Stache from comment #43)
> Filed bug 1081206 for investigation, as this one is already closed.
Thanks. To be clear, the issue this bug addressed was that the gear icon used to have a shadow when "active." This shadow at low resolutions just made the icon look blurry.