Last Comment Bug 842780 - about:newaddon doesn't restrict size of the add-on's icon
: about:newaddon doesn't restrict size of the add-on's icon
Status: RESOLVED FIXED
:
Product: Toolkit
Classification: Components
Component: Add-ons Manager (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla23
Assigned To: Sachin Hosmani [:sachin]
:
: Andy McKay [:andym]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-19 14:43 PST by Blair McBride [:Unfocused] (UNAVAILABLE)
Modified: 2013-04-25 10:51 PDT (History)
2 users (show)
blair: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Screenshot (232.33 KB, image/png)
2013-02-19 14:43 PST, Blair McBride [:Unfocused] (UNAVAILABLE)
no flags Details
Sets the image's width to 64px and height to 'auto' (1.27 KB, patch)
2013-04-17 10:32 PDT, Sachin Hosmani [:sachin]
blair: feedback-
Details | Diff | Splinter Review
Sets the max-height and max-weight CSS properties. (3.22 KB, patch)
2013-04-22 23:49 PDT, Sachin Hosmani [:sachin]
blair: review+
Details | Diff | Splinter Review

Description Blair McBride [:Unfocused] (UNAVAILABLE) 2013-02-19 14:43:46 PST
Created attachment 715729 [details]
Screenshot

Seems about:newaddon doesn't restrict the size of the add-on's icon, and blindly assumes the image is the right size. So if the add-on supplies a huge image for it's icon, it is shown at that huge size. See the attached screenshot for an example.
Comment 1 Sachin Hosmani [:sachin] 2013-04-17 10:30:05 PDT
I would like to take this bug up.
So, what exactly is the right size for an addon's icon?
Comment 2 Sachin Hosmani [:sachin] 2013-04-17 10:32:48 PDT
Created attachment 738606 [details] [diff] [review]
Sets the image's width to 64px and height to 'auto'

It doesn't check the present size however, since I don't know what is a right size.
Please tell me if this is what I should be doing.
Comment 3 Blair McBride [:Unfocused] (UNAVAILABLE) 2013-04-22 18:20:30 PDT
Comment on attachment 738606 [details] [diff] [review]
Sets the image's width to 64px and height to 'auto'

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

We support 32x32 and 64x64 sized icons for this. The way other code handles this is to set max-width:64px; max-height:64px; in the CSS file - that way the image will automatically show as its original size (without scaling) if it's not too big, or be scaled down to 64x64 if it's too big.
Comment 4 Blair McBride [:Unfocused] (UNAVAILABLE) 2013-04-22 18:22:23 PDT
(Btw, thanks for picking up all these bugs. Sorry for any delays in replying - I've been off sick.)
Comment 5 Sachin Hosmani [:sachin] 2013-04-22 23:49:20 PDT
Created attachment 740674 [details] [diff] [review]
Sets the max-height and max-weight CSS properties.
Comment 6 Sachin Hosmani [:sachin] 2013-04-22 23:54:04 PDT
(In reply to Blair McBride [:Unfocused] (Back from the dead. Mostly.) from comment #4)
> (Btw, thanks for picking up all these bugs. Sorry for any delays in replying
> - I've been off sick.)

No problem. Thank you :)
Comment 7 Blair McBride [:Unfocused] (UNAVAILABLE) 2013-04-23 22:58:45 PDT
https://hg.mozilla.org/integration/fx-team/rev/6b6e79e65b37
Comment 8 Ryan VanderMeulen [:RyanVM] 2013-04-25 10:51:12 PDT
https://hg.mozilla.org/mozilla-central/rev/6b6e79e65b37

Note You need to log in before you can comment on or make changes to this bug.