616634 - Installing a Persona from AMO will show a too large preview in the details pane

Status: VERIFIED DUPLICATE of bug 598670

(Toolkit :: Add-ons Manager, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: screenshot

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b8pre) Gecko/20101203 Firefox/4.0b8pre ID:20101203030309

When you install a Persona and open its detail view, the full image is shown and the description and buttons are pushed out of the view on the right side. See the screenshot.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: addons.sqlite

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

We had been holding off until bug 585069 was fixed (so the image keeps its aspect radio), but doesn't look like that will happen for 4.0.

However, AMO does have a preview image that's the right size - it just doesn't give it to us.

Here's the data we get for a lightweight theme:

{
  "iconURL": "https://www.getpersonas.com/static/3/4/91934/preview_small.jpg",
  "description": "Green Rings",
  "accentcolor": "#3b3b3b",
  "header": "http://www.getpersonas.com/static/3/4/91934/ffheader.png",
  "footerURL": "http://www.getpersonas.com/static/3/4/91934/ffbottom.png",
  "textcolor": "#000000",
  "id": "91934",
  "category": null,
  "headerURL": "http://www.getpersonas.com/static/3/4/91934/ffheader.png",
  "name": "Simple Xbox Green Rings",
  "author": "Seizonsha",
  "footer": "http://www.getpersonas.com/static/3/4/91934/ffbottom.png",
  "updateURL": "https://www.getpersonas.com/update_check/91934",
  "previewURL": "https://www.getpersonas.com/static/3/4/91934/preview_large.jpg"
}

(Note: we don't actually store all of that.)

In addition to preview_large.jpg (680x100 - too big) and preview_small.jpg (32x32 - too small), there's also preview.jpg (200x67 - just right). Using preview.jpg turns out to be better than just scaling preview_large.jpg, since it cuts of the left side of the image (the right side is the important area).

So there's a few options:
1. Get AMO to send us the location of preview.jpg (in a thumbnailURL property)
2. Assume preview.jpg will always be in the same path as preview_large.jpg, and alter the URL we have for preview_large.jpg
3. Resize preview_large.jpg, without care for the aspect ratio (won't look that good)

Option 1 would require either ignoring the problem for lightweight themes already in the addons manager, or using option 2 or 3 as a fallback.

Justin / Dave - thoughts?

Comment 3

[Justin Scott [:fligtar]]

Are you actually getting the data from AMO or from getpersonas.com? If it's AMO, it should be easy for us to add the field needed. If it's getpersonas, it will be a bit harder.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Indeed, that's only happening when installing a Persona from AMO. Using getpersonas.com instead, we get the correct preview image.

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #3)
> Are you actually getting the data from AMO or from getpersonas.com? If it's
> AMO, it should be easy for us to add the field needed. If it's getpersonas, it
> will be a bit harder.

Oh, hmm - I had only tested with AMO (I assumed they'd be the same). The data comes from whatever site the user visits. As Henrik said, if you install from getpersonas.com, we get the medium-sized preview.jpg that we want. So maybe the solution is to change what image AMO gives in the previewURL property?

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

I'll also add a max-width in bug 602059 for already installed personas that haven't updated (and too-large images in general).

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Dave, as you have mentioned on bug 598670:

> I think we're going to end up having to work around this separately anyway
> so I'd put it at a low priority from our side now

Where is this bug to implement the workaround? As long as we do not have a bug in our AOM component it can be easily forgotten. Can we use this bug?

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #8)
> Where is this bug to implement the workaround? As long as we do not have a bug
> in our AOM component it can be easily forgotten. Can we use this bug?

Bug 602059.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Blair.