602059 - Large screenshots not handled well, and/or no scroll bars on details page

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, enhancement)

Description

[Erik Vold]

I'm working on a addon that adds External addons (user scripts) to the new addon manager. These userscripts are allowed to define screenshots to be used with a @screenshot header key.

So the problem is that if a user scripts includes a very large screenshot image, then that will end up taking all of the space on the addon's detail view page, and there are no scroll bars provided when the content overflows the little area it has. See attached screen shot.

I'm going to allow user script authors to provide thumbs for the screen shots, but there will be people that ignore that and use images that are too large anyway, and I think that this case ought to be handled by the Addon manger (by setting css max-width and max-height for the screenshots for example), instead of being handled by every extension that adds external addons to the addon manager.

Comment 1

[Erik Vold]

Attachment: screen shot of bug

Comment 2

[Dave Townsend [:mossop]]

bug 595317 will probably fix the scrollbar issues, otherwise we generally expect to see a thumbnail version of the screenshot to display

Comment 3

[Blair McBride [:Unfocused] (UNAVAILABLE)]

I still think we need max-width/height on the thumbnail. I actually thought that had been added awhile ago, but it wasn't.

Comment 4

[Dave Townsend [:mossop]]

Well if we do that we either need bug 585069 fixed or we need to switch to using xhtml:img tags there which can have bad XUL layout issues.

Comment 5

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Bug 585069 would help tremendously, but I'd rather have a mis-scaled thumbnail, than have a thumbnail that takes up all screen real-estate and forces scrolling to see anything else.

Comment 6

[Brian King [:kinger]]

Is there some argument against switching to html:img? Usually that + some clever CSS and/or constraining the image container does the trick in situations like this.

Comment 7

[Dave Townsend [:mossop]]

No argument against it, just a warning that it can layout problems and goodness knows we've had enough problems getting scrolling to work right here. When I was doing the initial prototype of the new implementation pane I discovered that in certain situations with a scaled xhtml:img inside XUL a strange effect would occur where the XUL would behave as if the image was still full size.

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

As given by bug 616634 comment 8 I would like to nominate for blocking. In case of Persona installed from AMO, the details pane will always show large screenshots instead of the previews.

Comment 9

[Blair McBride [:Unfocused] (UNAVAILABLE)]

Attachment: Patch v1

Restricts screenshots to a maximum of 300px in either direction. Also adjusts the min-width of the description (see 602062 comment 3).

And yes, using html:img resulted in a mess.

Comment 10

[Dave Townsend [:mossop]]

Landed: http://hg.mozilla.org/mozilla-central/rev/b32d9897178b

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

Erik, can you please check the latest nightly if this issue is now fixed for you? Thanks.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 10/03 - 10/13)]

As given by Erik on bug 602062 this has been fixed. Marking as verified fixed.