598670 - Personas preview image should be small (200px across)

Status: VERIFIED FIXED

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

Description

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

Mozilla/5.0 (Windows NT 5.1; rv:2.0b7pre) Gecko/20100922 Firefox/4.0b7pre

In some cases we display the full screenshot which breaks the details pane. I have already mentioned that on bug 593217 comment 1. 

By using a Firefox Cup theme the screenshot (or thumbnail) extends the normal width we allow for a thumbnail. All the text gets shifted out of the default view. See attachment 476556 [details].

Not sure if we should block because we do not know how many add-ons are affected by this bug. Lets Dave decide.

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

Dave, we are letting you decide! :)

Comment 2

[Dave Townsend [:mossop]]

The problem here is that AMO delivers us a preview image for personas but for some reason it is sending very large previews (680px across). The preview image is meant to be a small thumbnail of the persona, getpersonas.com only sends 200px previews.

Comment 3

[Wil Clouser [:clouserw]]

We've got 3 sizes:
- The size on search results pages (https://addons.mozilla.org/en-US/firefox/search/?q=asdf&cat=all&lver=any&pid=1&sort=&pp=20&lup=&advanced=)

- The full size (https://addons.mozilla.org/en-US/firefox/addon/29725/)

- And the small sizes at the bottom of the full size page.  Sounds like you'd like the small size at the bottom of the page?

How are we delivering this to you?

Comment 4

[Dave Townsend [:mossop]]

The url is passed in the previewURL property of the JSON blob in the data-browsertheme attribute of the theme div on the page (try saying that after a few drinks)

Comment 5

[Wil Clouser [:clouserw]]

Oh, it looks like the tiny images are just served off getpersonas and the search result size is us just trimming the image with CSS.

Comment 6

[Wil Clouser [:clouserw]]

This code hasn't changed in a long time.  Is this a change on your end?

Comment 7

[Dave Townsend [:mossop]]

We have changed where the image is displayed, but it wouldn't look good on 3.6 for it to be that large either.

Comment 8

[Wil Clouser [:clouserw]]

potch:  any idea what's going on here?  any ramifications if we change it?

Comment 9

[Wil Clouser [:clouserw]]

What is the priority on this bug for the client team?

Comment 10

[Dave Townsend [:mossop]]

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

Comment 12

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Any chance I can convince someone to make this a high priority? The workaround we have (bug 602059) is going to make the preview look a lot uglier than if it were just the smaller size (see bug 616634 for details).

Comment 13

[Jeff Balogh (:jbalogh)]

(In reply to bug 616634 comment #2)
> 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)

It seems like we could put preview.jpg into previewURL instead of thumbnailURL as (1) proposes. AMO doesn't care what's in previewURL, and getpersonas.com is using preview.jpg in previewURL, so I assume all clients will happily handle the smaller previewURL.

Comment 14

[Blair McBride [:Unfocused] (UNAVAILABLE)]

(In reply to comment #13)
> It seems like we could put preview.jpg into previewURL instead of thumbnailURL
> as (1) proposes. AMO doesn't care what's in previewURL, and getpersonas.com is
> using preview.jpg in previewURL, so I assume all clients will happily handle
> the smaller previewURL.

Oh, yes - that's the conclusion that was reached in later comments in that bug.

Comment 15

[Jeff Balogh (:jbalogh)]

http://github.com/jbalogh/zamboni/commit/e5ac3ef

Comment 16

[krupa raj[:krupa]]

whimboo, can you please check if this is fixed in allizom? Thanks!

Comment 17

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

Installing the given Persona from allizom, correctly shows up a smaller thumbnail, while installing it from AMO still shows the large version. Given that I assume we can call this bug as verified fixed.