r91426 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r91425‎ | r91426 | r91427 >
Date:21:05, 4 July 2011
Author:kaldari
Status:reverted (Comments)
Tags:
Comment:
fixing rounding problem, per comment at r82309
Modified paths:
  • /trunk/phase3/includes/ImageGallery.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/ImageGallery.php
——@@ -284,7 +284,7 @@
285285 # We get layout problems with the margin, if the image is smaller
286286 # than the line-height (17), so we add less margin in these cases.
287287 $minThumbHeight = $thumb->height > 17 ? $thumb->height : 17;
288 - $vpad = floor( ( self::THUMB_PADDING + $this->mHeights - $minThumbHeight ) /2 );
 288+ $vpad = ( self::THUMB_PADDING + $this->mHeights - $minThumbHeight ) /2;
289289
290290 $imageParameters = array(
291291 'desc-link' => true,

Follow-up revisions

RevisionCommit summaryAuthorDate
r91427better fix for bug 27338 - doesnt rely on line-heightkaldari21:30, 4 July 2011
r91430using proper margin heights: 14 + 68 * 2 = 150kaldari23:43, 4 July 2011
r91557Revert r91426 and followups r91427, r91430: Breaks Gallery-related parser testsdemon16:23, 6 July 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r82309followup r82181 and r82215 to fix the FIXME and botched fix for FIXME....mah23:21, 16 February 2011

Comments

#Comment by Bawolff (talk | contribs)   21:39, 4 July 2011

This apparently breaks parser tests according to http://ci.tesla.usability.wikimedia.org/cruisecontrol/buildresults/mw?tab=testResults (you have to scroll and click stuff to get to the actual message... but it seems to be "CiteParserTests::testParserTest with data set #7 ('<references> after <gallery> ([https://bugzilla.wikimedia.org/show_bug.cgi?id=6164 bug 6164])', '<ref>one</ref>" that's causing the problem, along with the gallery related parser tests in core [4 in core+ the cite one])

#Comment by Kaldari (talk | contribs)   21:58, 4 July 2011

It looks like that test is written too strictly. It shouldn't care about the margin-size of the gallery thumbnail.

#Comment by Bawolff (talk | contribs)   22:01, 4 July 2011

All the parser tests are written the same - Wikicode in, html out, see if the html differs from the expected html. Its normal for them to break whenever the html output gets changed - they're more there from my understanding, to catch accidental changes in the html at the risk of false positives for intentional changes.

#Comment by Kaldari (talk | contribs)   22:06, 4 July 2011

Well, if you agree with the logic of this rev, I'll go ahead and change the parser test as well. The reasoning is presented in the comments at r82309.

#Comment by Kaldari (talk | contribs)   23:46, 4 July 2011

I went ahead and changed the parser tests in r91430. The values should definitely be 68 rather than 66 since the image is 14px tall and we want the div to be 150px high: 14 + 68 * 2 = 150.

#Comment by 😂 (talk | contribs)   14:59, 6 July 2011

Still not right. I'm still getting [[User:^demon/gallery failure r91426|a parser test failure]] as of HEAD.

#Comment by 😂 (talk | contribs)   15:01, 6 July 2011

Stupid CR parsing. Use this link

Status & tagging log