Closed Bug 1144774 Opened 8 years ago Closed 7 years ago

Add to reading list button is blurry


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



(firefox38 verified, firefox39 fixed, firefox40 fixed)

Firefox 40
Tracking Status
firefox38 --- verified
firefox39 --- fixed
firefox40 --- fixed


(Reporter: ntim, Assigned: florian)


(Blocks 1 open bug)


(Whiteboard: [reader-ui])


(2 files)

Applying shape-rendering: crispEdges to the "plus" icon mask should fix things.
Assignee: nobody → ntim.bugs
Attached patch PatchSplinter Review
Also removed xml:space, since it's fairly useless if there's no text in the SVG.
Attachment #8579512 - Flags: review?(gijskruitbosch+bugs)
Attachment #8579512 - Flags: review?(gijskruitbosch+bugs) → review?(jaws)
Comment on attachment 8579512 [details] [diff] [review]

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

crispEdges is causing it the rectangles to get off-center on my local build,

Have we looked at using the + character (if possible)?
Attachment #8579512 - Flags: review?(jaws) → feedback-
Flags: qe-verify+
QA Contact: andrei.vaida
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.
(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.)
Priority: -- → P3
Whiteboard: [reader-ui]
(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.
Attached patch PatchSplinter Review
Screenshot with the patch:
Screenshot without the patch:
Attachment #8589638 - Flags: ui-review?(mmaslaney)
Attachment #8589638 - Flags: review?(jaws)
Attachment #8589638 - Flags: ui-review?(mmaslaney) → ui-review+
Attachment #8589638 - Flags: review?(jaws) → review+
Thanks for picking this up Florian !
Assignee: ntim.bugs → florian
Hi Florian, can you provide a point value.
Blocks: 1132074
Iteration: --- → 40.1 - 13 Apr
Flags: needinfo?(florian)
Flags: firefox-backlog+
Points: --- → 2
Flags: needinfo?(florian)
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 40
Comment on attachment 8589638 [details] [diff] [review]

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.
Attachment #8589638 - Flags: approval-mozilla-beta?
Attachment #8589638 - Flags: approval-mozilla-aurora?
Comment on attachment 8589638 [details] [diff] [review]

Should be 38 beta 3 (or 4)
Attachment #8589638 - Flags: approval-mozilla-beta?
Attachment #8589638 - Flags: approval-mozilla-beta+
Attachment #8589638 - Flags: approval-mozilla-aurora?
Attachment #8589638 - Flags: approval-mozilla-aurora+
Verified fixed on 38.0b3-build1 (20150409144858), using Ubuntu 14.04 (x64), Windows 8.1 (x64) and Mac OS X 10.9.5.
Removing qe-verify flag, as verification on 38 Beta should suffice here.
Flags: qe-verify+
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.