593217 - Detail view always shows full screenshot, not thumbnail

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Added in bug 562902, the details view is always showing the full screenshot, instead of the thumbnail.

See:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/content/extensions.js#1623

It should be using screenshots[0].thumbnailURL - not screenshots[0].thumbnail

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th)]

But if no thumbnail is provided we would still show the full screenshot, at least given by the else path of the condition. That would break the UI too.

Comment 2

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Yea, I made the same comment in the review of bug 562902. Dave was going to file a separate bug for that.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th)]

There hasn't been filed a follow-up yet. So should we stop displaying the full screenshot for now until a module is available which scales down larger screenshots?

Comment 4

[Dave Townsend [:mossop]]

Going to check with AMO guys but I believe that we will always get a thumbnail for a screenshot, the only case where we wouldn't have one is for themes where we haven't downloaded the AMO data yet in which case we only have their full-sized preview.png but these are generally small enough so I suspect we will be ok just by fixing this bug for now.

Comment 5

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Silly mistake, easily fixed and tested

Comment 6

[Blair McBride [:Unfocused] (UNAVAILABLE)]

https://hg.mozilla.org/mozilla-central/rev/1376f2e60e6b

Comment 7

[Aaron Train [:aaronmt]]

VERIFIED FIXED w/ Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7pre) Gecko/20100917 Firefox/4.0b7pre

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th)]

Do we have a max width set for thumbnails? Any theme from Firefox Cup has a screenshot/thumbnail which takes all the available width, i.e. https://addons.mozilla.org/en-US/firefox/addon/169343/

Comment 9

[Dave Townsend [:mossop]]

(In reply to comment #8)
> Do we have a max width set for thumbnails? Any theme from Firefox Cup has a
> screenshot/thumbnail which takes all the available width, i.e.
> https://addons.mozilla.org/en-US/firefox/addon/169343/

You're seeing those in the add-ons manager?

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th)]

Attachment: screenshot

(In reply to comment #9)
> You're seeing those in the add-ons manager?

Yes, see the attachment. New bug?

Comment 11

[Dave Townsend [:mossop]]

(In reply to comment #10)
> Created attachment 476556 [details]
> screenshot
> 
> (In reply to comment #9)
> > You're seeing those in the add-ons manager?
> 
> Yes, see the attachment. New bug?

Yes get it filed and we'll figure it out.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2] (away July 24th - August 16th)]

Filed as bug 598670.