1144774 - Add to reading list button is blurry

Status: VERIFIED FIXED

(Firefox Graveyard :: Reading List, defect, P3)

Description

[Tim Nguyen :ntim]

Applying shape-rendering: crispEdges to the "plus" icon mask should fix things.

Comment 1

[Tim Nguyen :ntim]

Attachment: Patch

Also removed xml:space, since it's fairly useless if there's no text in the SVG.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

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

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

crispEdges is causing it the rectangles to get off-center on my local build, http://screencast.com/t/tEd0t1WuxjDE

Have we looked at using the + character (if possible)?

Comment 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

This is because of the half pixels specified in the SVG - it needs to be pixel aligned by increasing the circle radius or by using either thinner or thicker lines (I didn't get a chance to follow up on that).

I didn't look into using a "+" or "-" character - IIRC, we've had trouble doing that in the past with small things, because we can't reliably get it centered and looking right (different OSes drawing differently, different fonts imposed), and any deviation is really obvious at that small scale.

Comment 4

[Justin Dolske [:Dolske]]

(Hard for me to judge with the fuzzy upscaling on my Retina display, but let's tag this as P3. Could be a P4/P5 if it's more subtle that I'm thinking.)

Comment 5

[Florian Quèze [:florian]]

(In reply to Justin Dolske [:Dolske] from comment #4)
> (Hard for me to judge with the fuzzy upscaling on my Retina display

The icon is an SVG file, so I don't expect any fuzzy upscaling for retina mac displays.

Comment 6

[Florian Quèze [:florian]]

Attachment: Patch

Screenshot with the patch: http://i.imgur.com/njxNwbq.png
Screenshot without the patch: http://i.imgur.com/mBT1isa.png

Comment 7

[Tim Nguyen :ntim]

Thanks for picking this up Florian !

Comment 8

[Marco Mucci [:MarcoM]]

Hi Florian, can you provide a point value.

Comment 9

[Florian Quèze [:florian]]

https://hg.mozilla.org/integration/fx-team/rev/821e8ecf946f

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/821e8ecf946f

Comment 11

[Florian Quèze [:florian]]

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

Approval Request Comment
[Feature/regressing bug #]: ReadingList
[User impact if declined]: Blurry icon in the URL bar
[Describe test coverage new/current, TreeHerder]: QA will verify.
[Risks and why]: Low, only tweaking an SVG file.
[String/UUID change made/needed]: none.

Comment 12

[Sylvestre Ledru [:Sylvestre]]

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

Should be 38 beta 3 (or 4)

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1314e0b7a687

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f377c6831282

Comment 15

[Andrei Vaida [:avaida]]

Verified fixed on 38.0b3-build1 (20150409144858), using Ubuntu 14.04 (x64), Windows 8.1 (x64) and Mac OS X 10.9.5.

Comment 16

[[SV Manager] Florin Mezei, QA (:FlorinMezei)]

Removing qe-verify flag, as verification on 38 Beta should suffice here.