Last modified: 2010-12-13 19:37:59 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
This could probably be accomplished by replacing the table with floated divs. I'll put it on my to-look-at list.
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.
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.
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.
(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.
We have a JavaScript solution working for some time now in commons.wikimedia.org. http://commons.wikimedia.org/wiki/MediaWiki:ResizeGalleries.js
*** Bug 18213 has been marked as a duplicate of this bug. ***
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
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
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
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.
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.
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.
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.
sry for the double post, forgot the link: http://dl.dropbox.com/u/442163/fluidGallery.html
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?
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
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?)
(Sorry for the slow response, by the way. I'll try to be more prompt for future iterations if I can manage it.)
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.
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.
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..)
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.
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.
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.
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.
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.
(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-	"-hack introduced in r42257. At least that was the problem with the similar bug 16108.
Reverted r42257 in r77410, and committed the patch (with style fixes, parser test fixes, fix from comment 27) in r77411.
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)
(In reply to comment #30) > Followup > > Here's a follow-up. It does three things: thx. committed.
committed in r77836
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 }
@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"
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.
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.
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.
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
Created attachment 7908 [details] Cite cite test patch as well
