614746 - [Edit page] User uploaded images are not saved

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Developer Pages, defect)

Description

[krupa raj[:krupa]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.6; en-US; rv:1.9.2.12) Gecko/20101026 Firefox/3.6.12

steps to reproduce:
1. Load https://addons.allizom.org/z/en-US/developers/addon/255387/edit
2. Click "edit" for Images and Videos
3. Upload an .png file and save changes

observed behavior:
User uploaded images are not saved

screencast: http://screencast.com/t/kPWuXJWj8

Comment 1

[Gregory Koberger (:gkoberger)]

The problem isn't that they aren't saving; they just aren't being served correctly.

Comment 2

[Wil Clouser [:clouserw]]

Alright, koberger says these aren't being saved in the right spot, but he'll take care of that (with the same algorithm as remora) today in bug 614450.

That changes this bug to be about serving the images.  Right now we're serving images directly from apache (eg. http://clousamo.khan.mozilla.org/en-US/firefox/images/addon_icon/3945/1291140244) which is hitting this rewrite (http://viewvc.svn.mozilla.org/vc/addons/trunk/site/app/webroot/.htaccess?view=markup#l33).

Our new site has 3 versions (32px, 48px, 64px) of every icon so the single icon won't work for us anymore.  The new filenames are <addon_id>_<size>.png, eg. 5326_32.png.

For this bug, please modify the rewrite above to serve the correctly sized images, making the default (the current URL) 48px.  So:

/images/addon_icon/3945/1291140244    => 3945_48.png
/images/addon_icon/3945/32/1291140244 => 3945_32.png
/images/addon_icon/3945/48/1291140244 => 3945_48.png
/images/addon_icon/3945/64/1291140244 => 3945_64.png

That trailing number is just a timestamp, we should continue to support it too.  If you want to support new, less ugly icon urls too (that actually end in .png), I wouldn't stop you. :)

Comment 3

[Wil Clouser [:clouserw]]

Oh, https://addons.allizom.org/media//img/amo2009/icons/default-addon.png is our default right now, and we should keep using that if they don't have one.  I'll file a bug to get the other sizes.

Comment 4

[Gregory Koberger (:gkoberger)]

I'm ccing Fligtar, to see which size (32/48/64) should be default.

Also, Andy- I already did the python that decides if it should show a default image, premade icon or a user uploaded one.  That's in the Addons model.

Comment 5

[Wil Clouser [:clouserw]]

Note that my awesome example in comment 2 isn't right, since the icons on AMO are 32x32 and not 48x48.  Anyway, do whatever fligtar says.

Comment 6

[Justin Scott [:fligtar]]

32 is the default. As we update designs of various AMO pages we'll try to take advantage of the larger sizes.

Comment 7

[Andy McKay]

https://github.com/andymckay/zamboni/commit/d146e8b9619333a615ca4fd6b2b697175751ab3c#

And Apache rule changes to come from Wil.

Comment 9

[Wil Clouser [:clouserw]]

(In reply to comment #7)
> https://github.com/andymckay/zamboni/commit/d146e8b9619333a615ca4fd6b2b697175751ab3c#
> 
> And Apache rule changes to come from Wil.

r79821

Comment 10

[krupa raj[:krupa]]

Add-on icon upload is not working anymore.

"Images are being processed" message persists forever and changes never get saved. STR is the same as comment 0.

Comment 11

[krupa raj[:krupa]]

Attachment: screenshot for reopening

Comment 12

[Andy McKay]

Is the problem that the message is persisting after you click edit a second time?

Comment 13

[Wil Clouser [:clouserw]]

I can't reproduce this problem.  Krupa, is this still something that you can see?

Comment 14

[Gregory Koberger (:gkoberger)]

Krupa will know better than me, but I think the issue is that celery is crazy slow on allizom.  So, it works, it just takes 10 minutes.

Comment 15

[krupa raj[:krupa]]

(In reply to comment #13)
> I can't reproduce this problem.  Krupa, is this still something that you can
> see?

wfm now. I think cvan's commit fixed this too.

Comment 16

[krupa raj[:krupa]]

verified @ https://addons.allizom.org/z/en-US/developers/addon/amazonassist-test-addon3011/edit

Icon upload worked when I tried.