Last modified: 2011-07-04 21:36:43 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 27338 - Gallery in 1.17 breaks for audio/video + ogghandler
Gallery in 1.17 breaks for audio/video + ogghandler
Product: MediaWiki
Classification: Unclassified
Interface (Other open bugs)
All All
: Normal normal (vote)
: ---
Assigned To: Nobody - You can work on this!
Depends on:
Blocks: 27339
  Show dependency treegraph
Reported: 2011-02-11 20:45 UTC by Derk-Jan Hartman
Modified: 2011-07-04 21:36 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---

screenshot of broken layout (54.99 KB, image/png)
2011-02-11 20:45 UTC, Derk-Jan Hartman
Patch (1.53 KB, patch)
2011-02-12 16:07 UTC, DieBuche
screenshot of rendering with patch (221.73 KB, image/png)
2011-02-12 16:55 UTC, Derk-Jan Hartman
better patch (1.81 KB, patch)
2011-02-16 10:46 UTC, DieBuche
Better combined patch, fixes 27458 as well (1.92 KB, patch)
2011-02-16 15:48 UTC, DieBuche
screenshot of rendering with patch2 (134.89 KB, image/png)
2011-02-16 15:53 UTC, DieBuche

Description Derk-Jan Hartman 2011-02-11 20:45:11 UTC
The new gallery layout of 1.17 breaks when you have thumbnails for audio and video. The blocks have a fixed height, and this prevents dynamically loading elements like OggPlayer and mwEmbed from adapting the content and benefitting from auto sizing.

Perhaps min-height  instead of height ?
Comment 1 Derk-Jan Hartman 2011-02-11 20:45:51 UTC
Created attachment 8128 [details]
screenshot of broken layout
Comment 2 DieBuche 2011-02-12 01:57:22 UTC
min-heigth sounds good. I'll make a patch
Comment 3 Derk-Jan Hartman 2011-02-12 12:47:17 UTC
Min-height might cause the inline error messages to stretch a very larger area....  We should probably verify that.
Comment 4 DieBuche 2011-02-12 16:07:04 UTC
Created attachment 8134 [details]

No, the error messages are generated seperately, see ImageGallery,php L257-L272.
Here's the patch (had to create a new css rule to avoid regressing to a previous bug)
Comment 5 Derk-Jan Hartman 2011-02-12 16:54:54 UTC
ehm, mind explaining the css rule ? perhaps even with a comment ? Might be helpful if one done someone figure "what the f is this for".
Comment 6 Derk-Jan Hartman 2011-02-12 16:55:57 UTC
Created attachment 8135 [details]
screenshot of rendering with patch
Comment 7 DieBuche 2011-02-12 17:00:08 UTC
If the image height is lower than the line-height, the margin-top is applied to the top of the line.
a very short image will not follow for 3-4px laters, thus leading to a bigger distance from the top than it should be.
vertical-align:text-top moves the picture up, so this problem doesn't happen
Comment 8 Mark A. Hershberger 2011-02-15 19:41:41 UTC
Applied in r82181
Comment 9 Mark A. Hershberger 2011-02-16 03:41:27 UTC
Trevor said this didn't work on older browsers, that we have to support.  Could you test r82215 and see if it works for you?
Comment 10 DieBuche 2011-02-16 10:44:26 UTC
Comment 11 DieBuche 2011-02-16 10:46:49 UTC
Created attachment 8152 [details]
better patch

Nope, this leads to the first screenshot again.
Here's a better patch:

Remove the css rule again, don't set a fixed height at all, but fix the margin calculation for small images
Comment 12 DieBuche 2011-02-16 15:48:34 UTC
Created attachment 8153 [details]
Better combined patch, fixes 27458 as well
Comment 13 DieBuche 2011-02-16 15:53:03 UTC
Created attachment 8154 [details]
screenshot of rendering with patch2
Comment 14 Mark A. Hershberger 2011-02-16 23:22:24 UTC
see r82309
Comment 15 Ryan Kaldari 2011-07-04 21:36:43 UTC
The patch has two problems. First you can't rely on the line-height being 17px, since it's actually 1.5em. Secondly, rounding the margin gives you different div heights for even and odd pixel thumbnails.

I've attempted to fix the first problem with r91427 and the second problem with r91426. Could others test and verify that this works for all cases. Thanks!

Note You need to log in before you can comment on or make changes to this bug.