Last modified: 2010-12-13 19:37:59 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 3276 - Give image <gallery>s fluid width
Give image <gallery>s fluid width
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Parser (Other open bugs)
unspecified
All All
: Normal enhancement with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
: patch
: 18213 (view as bug list)
Depends on:
Blocks: tidy 3770
  Show dependency treegraph
 
Reported: 2005-08-26 07:13 UTC by Nicholas
Modified: 2010-12-13 19:37 UTC (History)
9 users (show)

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


Attachments
Why floats won't work (2.63 KB, image/png)
2007-02-02 02:31 UTC, Aryeh Gregor (not reading bugmail, please e-mail directly)
Details
My abortive attempt at a patch, in case it's useful to anyone (6.63 KB, patch)
2007-02-02 02:34 UTC, Aryeh Gregor (not reading bugmail, please e-mail directly)
Details
Basic, working patch (2.29 KB, patch)
2010-11-05 15:23 UTC, DieBuche
Details
patch 2 (4.47 KB, patch)
2010-11-08 21:41 UTC, DieBuche
Details
patch (with line numbers) (4.45 KB, patch)
2010-11-19 18:43 UTC, DieBuche
Details
Screenshot with patch applied, incorrect results (131.20 KB, image/png)
2010-11-22 20:36 UTC, Aryeh Gregor (not reading bugmail, please e-mail directly)
Details
Before and after HTML output with patch applied (182.86 KB, application/x-php)
2010-11-26 00:05 UTC, Aryeh Gregor (not reading bugmail, please e-mail directly)
Details
Fix empty list elements (4.39 KB, patch)
2010-11-26 18:52 UTC, DieBuche
Details
Followup (2.75 KB, patch)
2010-11-30 19:34 UTC, DieBuche
Details
Tests (5.95 KB, patch)
2010-12-13 19:35 UTC, DieBuche
Details
Cite (778 bytes, patch)
2010-12-13 19:37 UTC, DieBuche
Details

Description Nicholas 2005-08-26 07:13:58 UTC
I use Opera 8.01 in Windows. The '''<gallery>''' tag creates 
a very annoying horizontal scrollbar at 800x600 resolution. 
Please could you reduce the width?

Mediawiki:Current
Comment 1 Aryeh Gregor (not reading bugmail, please e-mail directly) 2006-12-22 04:52:33 UTC
This could probably be accomplished by replacing the table with floated divs. 
I'll put it on my to-look-at list.
Comment 2 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-02-02 02:31:56 UTC
Created attachment 3169 [details]
Why floats won't work

I don't think the floated-div approach will work with captions.  If some images
in the gallery have captions and others don't, some boxes will be taller than
others, and the next row break might not go all the way to the left, so it will
look like the attachment.  I can't think of any way around this, offhand.
Comment 3 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-02-02 02:34:13 UTC
Created attachment 3170 [details]
My abortive attempt at a patch, in case it's useful to anyone

This partial patch has the flaw depicted in the previous attachment, but
otherwise is more or less working.
Comment 4 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-02-02 21:43:38 UTC
Some more possibilities:

1) Leave the table markup as-is, but translate using CSS into inline-block and -moz-inline-block.  
It will be difficult to do this with CSS alone without mucking up the display for people whose 
browsers partially implement the relevant attributes.  On the other hand, it will look as it does 
now for people with CSS1 or early CSS2 browsers, I suppose.  Would require quite a bit of testing.

2) Translate to divs (or spans to work around an IE bug) and use the inline-block approach.  I 
suspect this is more likely to work with IE.  As a fallback, perhaps display: inline could be used 
for the wrapping span, and block for the inside spans.  I'm not sure offhand how that works, or if 
it would work at all: it might be possible to do this without inline-block at all, if block boxes 
inside inline boxes work the way I think they do.  I'll look this up.

3) Use some kind of JavaScript hackery.  Icky, and also screws over the most likely beneficiaries -
- handheld users -- since many handhelds (I think?) tend to not execute JavaScript, unless perhaps 
specifically forced to.  Not really a useful option.

4) Workaround: have a user preference to specify the maximum width (in pixels) to display 
galleries, overriding the perrow setting even if that's specified in the content.
Comment 5 Aryeh Gregor (not reading bugmail, please e-mail directly) 2007-02-07 00:43:41 UTC
(In reply to bug 6987 comment #2)
> If you take a div as container, and put divs for the images inside of that and
> float these to the left, they'll automatically order themselves into a grid (or
> vertical strip if you make the container small enough). That way you don't have
> to keep browser width in mind.
> 
> You do have to make sure the blocks are all the same height though.

Yep, that's what I found out (see comment #2).

> For an even
> better solution I've tried to make the image divs behave like inline elements so
> they would also line up if they aren't all the same height, but I wasn't able to
> make that work.

Inline doesn't work, because the image and caption need to be laid out as block elements, and blocks 
inside inlines break out of the inline.  So you need inline-block.  Absolute positioning could be 
used, but that has the same problem as floats of knowing the height.  Inline-block seems to be the 
only realistic way to do this, although there may well be something I've missed.
Comment 6 Daniel Schwen 2008-02-22 22:55:08 UTC
We have a JavaScript solution working for some time now in commons.wikimedia.org.
http://commons.wikimedia.org/wiki/MediaWiki:ResizeGalleries.js
Comment 7 P.Copp 2009-03-30 10:59:49 UTC
*** Bug 18213 has been marked as a duplicate of this bug. ***
Comment 8 Subfader 2009-04-01 16:10:05 UTC
As of now ResizeGalleries.js breaks the manual Gallery's perrow="". If set manually it shouldn't be overridden. http://commons.wikimedia.org/wiki/MediaWiki_talk:ResizeGalleries.js#This_script_breaks_Gallery.27s_perrow.3D.22.22
Comment 9 DieBuche 2010-11-05 14:10:39 UTC
This is actually very easy:
Use divs with:
{
display:-moz-inline-stack;
display:inline-block;
zoom:1;
*display:inline;
}

add a vertical-align:top;
to cater for inconsistenst heights.

I'll add a patch later
Comment 10 DieBuche 2010-11-05 15:23:34 UTC
Created attachment 7792 [details]
Basic, working patch

here's a quick patch perrow is not yet refactored, best way imo would be to set a fixed width on the container div
Comment 11 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-11-07 17:33:25 UTC
Could you explain what exactly these CSS rules do, and say what browsers you've tested in?  Particularly, what does the "zoom: 1" do; and what browser are you hacking with the * there before "display: inline"?  And how does -moz-inline-stack differ from inline-block, for old Firefox that doesn't implement the latter?

Perrow should just be eliminated if the width is fluid.  There's no real need for it in that case.
Comment 12 DieBuche 2010-11-07 18:08:22 UTC
zoom: 1 makes an element get hasLayout in IE.
For some reasons a block element that hasLayout and has display:inline explicitely set, is treated as inline-block in IE6+7.

I'll have to investigate into the differences between -moz-inline-stack in FF2 and inline-block in FF3
Heres a more detailed description of the process: http://blog.mozilla.com/webdev/2009/02/20/cross-browser-inline-block/

Another thing, my patch creates <div class="gallerybox">. Using <li class="gallerybox"> is semantically better though.
Comment 13 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-11-07 18:57:28 UTC
If you're willing to provide a comprehensive test case and test it in all important browsers, I'd be okay with committing this.  The test case should include

* Small images
* Large images
* Different caption lengths
* Some exercise of various gallery options

If it looks right in at least IE6, IE7, IE8, Firefox 3.x, Opera 10.x, and recent Chrome/Safari, then I'd be willing to try checking it in.  I'm a little nervous because I don't know how all the crazy IE stuff works.  It would be nice if someone who knew more about old broken IE CSS handling could review this, like Trevor.

I'm not sure we have to worry about Firefox 2.x.  According to <http://stats.wikimedia.org/archive/squid_reports/2010-10/SquidReportClients.htm>, it's around 0.5% market share, so I guess we should make sure it doesn't totally break.
Comment 14 DieBuche 2010-11-07 23:06:56 UTC
Here's a testpage I build (using <li>'s now, I'll update the patch later..). Looks good for me in Webkit and Firefox and Opera. I'll test IE6-8 tomorrow at work.
Comment 15 DieBuche 2010-11-07 23:07:49 UTC
sry for the double post, forgot the link:
http://dl.dropbox.com/u/442163/fluidGallery.html
Comment 16 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-11-08 01:17:09 UTC
Looks good to me.  Trevor, could you (or someone else who knows how CSS works on old broken browsers) take a quick look at the CSS involved here and see if it seems to make sense?
Comment 17 DieBuche 2010-11-08 21:41:30 UTC
Created attachment 7803 [details]
patch 2

Here's the unified patch.

This time IE wasn't the troublemaker at all: IE6-8 behaved all as expected.
FF2 initally "stacked all images on top of each other", and after this was fixed, I needed to wrap the whole image in a div, because FF2 doesn't seem to accept display:block on a <li>

Anyway, it works now. Perrow is retained (It's not set by default though, but can be specified on individual gallerys)

It also happens to fix Bug 3770
Comment 18 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-11-19 00:36:55 UTC
Your patch is broken, the line numbers are missing:

@@ -%ld,%ld +%ld,%ld @@

Could you generate a correct patch using "svn diff"?  (Or is this broken one generated by that and it's a bug in SVN?)
Comment 19 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-11-19 00:45:21 UTC
(Sorry for the slow response, by the way.  I'll try to be more prompt for future iterations if I can manage it.)
Comment 20 DieBuche 2010-11-19 18:43:05 UTC
Created attachment 7839 [details]
patch (with line numbers)

This was weird. I used the normal svn diff. Too lazy to investigate though, a new one is attached.
Comment 21 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-11-22 20:36:38 UTC
Created attachment 7849 [details]
Screenshot with patch applied, incorrect results

Testing the patch on a simple gallery,

<gallery>
File:Test.gif|Foo
File:Test.gif|Bar
File:Test.gif
File:Test.gif
File:Test.gif
File:Test.gif
File:Test.gif
</gallery>

it doesn't seem to work as expected.  I did action=purge and Shift-refresh to ensure that everything was as it should be, but the boxes were stacked on top of each other in both Firefox 4b7 and Chrome dev, as illustrated in the attached screenshot.
Comment 22 DieBuche 2010-11-23 19:51:19 UTC
Weird, I can't reproduce that (in neither chrome nor ff4). You applied it to the trunk, right?
I just tried again with a clean install from svn & it behaves as expected. 
(I updated the dropbox link with a page generated by that patch, a perrow example is added as well)

Do you maybe have something weird in Mediawiki:common.js/.css (etc.)?

Otherwise, could you attach a zip of the whole page (firefox's save as..)
Comment 23 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-11-26 00:05:42 UTC
Created attachment 7869 [details]
Before and after HTML output with patch applied

I'm running trunk, and the patches applied cleanly.  It's not a clean trunk install, but if it breaks my dev install it will probably break other people's too, and I certainly can't commit it if I can't verify that it works.  I don't have anything in {Common,Monobook}.{css,js} as far as I can tell, and I don't see anything in LocalSettings.php that seems relevant.

before.html in the attached zip is what I get on regular trunk for the markup I gave in my last comment.  After applying your patch and doing action=purge and Shift-refresh, I get after.html.  I briefly tried to debug it, but I don't have time to look closely and didn't properly understand all of your changes in the first place.  Plus I get a sad tab in Chrome when trying to use Web Inspector here and Firebug doesn't work in Firefox 4.0 yet, so that makes it harder.
Comment 24 DieBuche 2010-11-26 09:13:23 UTC
weird, something in your install is causing the generation of empty <li> before each image <li> (<li style="list-style: none"></li>) These are by default display:block, so they add a newline before each image. I could fix that with css easily, but I'd still be interested where they come from. I'll have a look at that this evening.
Comment 25 DieBuche 2010-11-26 18:52:55 UTC
Created attachment 7871 [details]
Fix empty list elements

I greped through the trunk & all extensions, but couldn't find any code that could be responsible for those <li>s. Anyway, a new patch is attached, should work with those now.
Comment 26 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-11-28 19:20:00 UTC
It's caused by $wgUseTidy = true;.  It seems like a Tidy bug -- I don't know what objection it had to your original markup, it validates just fine.  A minimal test-case exhibiting the problem is

<ol>
	<li>x</li>
</ol>

(That's a tab for indentation, not spaces.)  It inserts a bogus <li> there, so the number displays as "2.", not "1.".  I worked around it by just removing all the tabs from your patch, so now Tidy doesn't mess it up so badly.  I should probably report the bug to Tidy's maintainers, but I don't really have time.

With the patch modified thus, it looks okay, but the display is changed a bit.  The old markup had a bit less space between the images, and a border around the whole thing.  It's not a big deal, but were you aiming for pixel-perfect identical rendering to the previous code here?  If so, I'm not seeing it.

Other than that, it looks fine now.  I'd prefer if you could tweak it to look just the same as before, but if you don't want to, I'm okay with committing it as-is, given how long it's taken me to get back to you all this time.
Comment 27 DieBuche 2010-11-28 23:19:10 UTC
Ah, ok, I'll file a bug for that later on.
Regarding the space between images: one margin got duplicated. remove "margin: 2px;" in shared.css, line 794 & it should look the same.
I removed the border attribute, because it tends to look weird if you have an odd number of images in the rows (4-4-2 etc.). Also, due to the box model, the border would always be on the very right side of the screen, even if the images have a linebreak before that.
Comment 28 P.Copp 2010-11-28 23:39:52 UTC
(In reply to comment #26)
> It's caused by $wgUseTidy = true;.  It seems like a Tidy bug -- I don't know
> what objection it had to your original markup, it validates just fine.  A
> minimal test-case exhibiting the problem is
> 
> <ol>
>     <li>x</li>
> </ol>
> 
> (That's a tab for indentation, not spaces.)  It inserts a bogus <li> there, so
> the number displays as "2.", not "1.".  I worked around it by just removing all
> the tabs from your patch, so now Tidy doesn't mess it up so badly.  I should
> probably report the bug to Tidy's maintainers, but I don't really have time.
> 
I'd think it's caused by the "tab-to-&#9;"-hack introduced in r42257. At least that was the problem with the similar bug 16108.
Comment 29 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-11-29 00:12:00 UTC
Reverted r42257 in r77410, and committed the patch (with style fixes, parser test fixes, fix from comment 27) in r77411.
Comment 30 DieBuche 2010-11-30 19:34:48 UTC
Created attachment 7877 [details]
Followup

Here's a follow-up. It does three things:

1. Adds a line css to actually fix Bug 3770 (Sorry, I had forgotten to paste it there as well)
2. Fixes a problem, where the box size would be slighty higher if the image thumb has a height of less than 8px
3. Normalize the calculation of the vertical padding to be exactly the same one as the horizontal padding. (This is only relevant for unusually large galleries, eg. previously it returned a not-exactly-square box if you specified 500px as both width and height)
Comment 31 Derk-Jan Hartman 2010-12-05 22:41:33 UTC
(In reply to comment #30)
> Followup
> 
> Here's a follow-up. It does three things:

thx. committed.
Comment 32 Derk-Jan Hartman 2010-12-05 22:42:04 UTC
committed in r77836
Comment 33 Fomafix 2010-12-07 21:15:00 UTC
r77411 has a problem on Firefox, when there is a floating object. It looks like a clear:both. The old table-based gallery hasn't this problem.

<div style="float:right; width:10em; height:10em; border: 1px solid red">Box</div>
<gallery>
Image:Köln Panorama.jpg
Image:Köln Panorama.jpg
Image:Köln Panorama.jpg
Image:Köln Panorama.jpg
Image:Köln Panorama.jpg
Image:Köln Panorama.jpg
Image:Köln Panorama.jpg
Image:Köln Panorama.jpg
Image:Köln Panorama.jpg
Image:Köln Panorama.jpg
Image:Köln Panorama.jpg
Image:Köln Panorama.jpg
</gallery>

r77411 defines
 ul.gallery { display: inline-block }

It should define
 ul.gallery { display: block }
Comment 34 Derk-Jan Hartman 2010-12-09 00:21:37 UTC
@Diebuche, can you fix the parser tests please ? I looked at them, but I can't figure out what the desired behavior should be.

@fomafix this behavior is expected. A gallery is now full width (and will thus clear floating objects), unless a perrow is specified. I note btw that this will cause some serious harm in articles most likely.... Perhaps we should only be fullwidth in the "file list" mode, and use a perrow=4 default in normal <gallery> tag mode, unless perrow="auto"
Comment 35 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-12-09 17:02:36 UTC
I think it should be full-width by default.  If it interferes with floats in particular cases, those can be fixed manually.  Surely it's pretty rare to have floats next to galleries?  Making it not flexible width by default more or less defeats the point.
Comment 36 Fomafix 2010-12-11 19:44:25 UTC
full-width as default is good, but a clear for floating objects is not good. With

 ul.gallery { display: block }

its possible to have full-width without a clear for floating objects. At least for Firefox.
Comment 37 Aryeh Gregor (not reading bugmail, please e-mail directly) 2010-12-12 00:31:36 UTC
Yeah, actually, floats should play nice with inline-block, right?  The line boxes should get shortened, just like with regular inline stuff.  Done in r78232.
Comment 38 DieBuche 2010-12-13 19:35:56 UTC
Created attachment 7907 [details]
Tests

I can't seem to remember why I made the ul inline-block, seems to work this way as well. Anyway parser test patch is attached
Comment 39 DieBuche 2010-12-13 19:37:59 UTC
Created attachment 7908 [details]
Cite

cite test patch as well

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


Navigation
Links