Closed Bug 731539 Opened 13 years ago Closed 12 years ago

Provide URL to 64px icon in API

Categories

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

Product:
addons.mozilla.org Graveyard
Component:
API
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
2012-05-17

People

(Reporter: darktrojan, Assigned: darktrojan)

References

Details

Attachments

(1 file)

WIP
(deleted), patch
clouserw
: feedback+
Details | Diff | Splinter Review
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.
+1
https://github.com/mozilla/zamboni/pull/317
Assignee: nobody → geoff
Status: NEW → ASSIGNED
(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.
Target Milestone: --- → 2012-05-17
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.
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?
(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.
You can add comments. {# blah #} comments are preferable as they are stripped before being sent client side.
Thanks. I landed this in https://github.com/mozilla/zamboni/commit/4b77bc889821f8d6449f0001273a2e3f4e6137c9
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
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.
Blocks: 760892
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch WIP (deleted) — — Splinter Review
Is there anything else I need to do here?
Attachment #632432 - Flags: feedback?(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.
Attachment #632432 - Flags: feedback?(clouserw) → feedback+
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. :-/
Re-closing this. I'm not working on it and a new bug can be opened later for the unfinished bits.
Status: REOPENED → RESOLVED
Closed: 13 years ago → 12 years ago
Resolution: --- → FIXED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: