731539 - Provide URL to 64px icon in API

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: API, defect)

Description

[Geoff Lankow (:darktrojan)]

We want to start using 64px icons for various things in the addons manager, so getting URLs for them from the API will be needed.

Comment 1

[Kohei Yoshino]

+1

Comment 2

[Geoff Lankow (:darktrojan)]

https://github.com/mozilla/zamboni/pull/317

Comment 3

[Wil Clouser [:clouserw]]

(In reply to Geoff Lankow (:darktrojan) from comment #2)
> https://github.com/mozilla/zamboni/pull/317

Thanks for the patch.  I added a comment on what I think is a more maintainable format.  Mind making the switch?  You can add size="32" to the other one too - it should continue working fine.

Comment 4

[Geoff Lankow (:darktrojan)]

That's a good idea, I was just copying our naming scheme client side. Before I make that change I'll check that a second <icon> isn't going to break stuff.

Comment 5

[Geoff Lankow (:darktrojan)]

Okay, I think I can make that change to the API without breaking existing products, if I put the 32px icon last - the parsing code will keep the value from the last <icon> specified, won't it, Blair?

http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/AddonRepository.jsm#1008

Wil, is it possible to put comments into the template?

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to Geoff Lankow (:darktrojan) from comment #5)
> the parsing code will keep the value
> from the last <icon> specified, won't it, Blair?

Yep.

Comment 7

[Wil Clouser [:clouserw]]

You can add comments.  {# blah #} comments are preferable as they are stripped before being sent client side.

Comment 8

[Wil Clouser [:clouserw]]

Thanks.  I landed this in https://github.com/mozilla/zamboni/commit/4b77bc889821f8d6449f0001273a2e3f4e6137c9

Comment 9

[Geoff Lankow (:darktrojan)]

Hmm, I wasn't expecting a value when there isn't a 64px icon. I think I'll change the behaviour to display all the icons we actually have.

Comment 10

[Geoff Lankow (:darktrojan)]

I'm reopening this bug as I don't think I fixed it properly. We should list all available icon sizes, but not where the image is just a copy of the image for a smaller size.

Comment 11

[Geoff Lankow (:darktrojan)]

Attachment: WIP

Is there anything else I need to do here?

Comment 12

[Wil Clouser [:clouserw]]

Comment on attachment 632432 [details] [diff] [review]
WIP

I think it's looking fine.  I'd like some more comments on has_icon() as that function doesn't really make sense to me.  Thanks for writing tests.  r+ for WIP.

Comment 13

[Geoff Lankow (:darktrojan)]

https://github.com/mozilla/zamboni/pull/342

Hopefully the commenting makes better sense now. It's difficult to explain with all the different edge cases. :-/

Comment 14

[Geoff Lankow (:darktrojan)]

Re-closing this. I'm not working on it and a new bug can be opened later for the unfinished bits.