r86749 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r86748‎ | r86749 | r86750 >
Date:07:27, 23 April 2011
Author:diebuche
Status:reverted (Comments)
Tags:
Comment:
Add ability to use add alt texts for images in galleries ( Bug 18682 ). Patch by Jan Paul Posma. Also cleaned up some comments and var names
Modified paths:
  • /trunk/phase3/includes/ImageGallery.php (modified) (history)
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)
  • /trunk/phase3/tests/parser/parserTests.txt (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/parser/parserTests.txt
——@@ -7312,6 +7312,7 @@
73137313 File:Nonexistant.jpg
73147314 image:foobar.jpg|some '''caption''' [[Main Page]]
73157315 image:foobar.jpg
 7316+image:foobar.jpg|Blabla|alt= This is a foo-bar.|blabla.
73167317 </gallery>
73177318 !! result
73187319 <ul class="gallery" style="max-width: 202px;_width: 202px;">
——@@ -7340,6 +7341,13 @@
73417342 <div class="gallerytext">
73427343 </div>
73437344 </div></li>
 7345+ <li class="gallerybox" style="width: 95px"><div style="width: 95px">
 7346+ <div class="thumb" style="width: 90px;"><div style="margin:26px auto;"><a href="https://www.mediawiki.org/wiki/File:Foobar.jpg" class="image"><img alt="This is a foo-bar." src="http://example.com/images/3/3a/Foobar.jpg" width="60" height="7" /></a></div></div>
 7347+ <div class="gallerytext">
 7348+<p>Blabla|blabla.
 7349+</p>
 7350+ </div>
 7351+ </div></li>
73447352 </ul>
73457353
73467354 !! end
Index: trunk/phase3/includes/parser/Parser.php
——@@ -4733,21 +4733,37 @@
47344734 if ( strpos( $matches[0], '%' ) !== false ) {
47354735 $matches[1] = rawurldecode( $matches[1] );
47364736 }
4737 - $tp = Title::newFromText( $matches[1], NS_FILE );
4738 - $nt =& $tp;
4739 - if ( is_null( $nt ) ) {
 4737+ $title = Title::newFromText( $matches[1], NS_FILE );
 4738+ if ( is_null( $title ) ) {
47404739 # Bogus title. Ignore these so we don't bomb out later.
47414740 continue;
47424741 }
 4742+
 4743+ $label = '';
 4744+ $alt = '';
47434745 if ( isset( $matches[3] ) ) {
4744 - $label = $matches[3];
4745 - } else {
4746 - $label = '';
 4746+ // look for an |alt= definition while trying not to break existing
 4747+ // captions with multiple pipes (|) in it, until a more sensible grammar
 4748+ // is defined for images in galleries
 4749+
 4750+ $altmatches = StringUtils::explode('|', $matches[3]);
 4751+
 4752+ foreach ( $altmatches as $altmatch ) {
 4753+ if ( substr( $altmatch, 0, 4 ) === 'alt=' ) {
 4754+ $alt = $this->stripAltText( trim( substr( $altmatch, 4 ) ), false );
 4755+ }
 4756+ else {
 4757+ // concatenate all other pipes
 4758+ $label .= '|' . $altmatch;
 4759+ }
 4760+ }
 4761+ // remove the first pipe
 4762+ $label = substr( $label, 1 );
47474763 }
47484764
47494765 $html = $this->recursiveTagParse( trim( $label ) );
47504766
4751 - $ig->add( $nt, $html );
 4767+ $ig->add( $title, $html, $alt );
47524768 }
47534769 return $ig->toHTML();
47544770 }
Index: trunk/phase3/includes/ImageGallery.php
——@@ -136,14 +136,15 @@
137137 * Add an image to the gallery.
138138 *
139139 * @param $title Title object of the image that is added to the gallery
140 - * @param $html String: additional HTML text to be shown. The name and size of the image are always shown.
 140+ * @param $html String: Additional HTML text to be shown. The name and size of the image are always shown.
 141+ * @param $alt String: Alt text for the image
141142 */
142 - function add( $title, $html='' ) {
 143+ function add( $title, $html = '', $alt = '' ) {
143144 if ( $title instanceof File ) {
144145 // Old calling convention
145146 $title = $title->getTitle();
146147 }
147 - $this->mImages[] = array( $title, $html );
 148+ $this->mImages[] = array( $title, $html, $alt );
148149 wfDebug( "ImageGallery::add " . $title->getText() . "\n" );
149150 }
150151
——@@ -151,14 +152,15 @@
152153 * Add an image at the beginning of the gallery.
153154 *
154155 * @param $title Title object of the image that is added to the gallery
155 - * @param $html String: Additional HTML text to be shown. The name and size of the image are always shown.
 156+ * @param $html String: Additional HTML text to be shown. The name and size of the image are always shown.
 157+ * @param $alt String: Alt text for the image
156158 */
157 - function insert( $title, $html='' ) {
 159+ function insert( $title, $html='', $alt='' ) {
158160 if ( $title instanceof File ) {
159161 // Old calling convention
160162 $title = $title->getTitle();
161163 }
162 - array_unshift( $this->mImages, array( &$title, $html ) );
 164+ array_unshift( $this->mImages, array( &$title, $html, $alt ) );
163165 }
164166
165167
——@@ -218,15 +220,16 @@
219221 if ( $this->mPerRow > 0 ) {
220222 $maxwidth = $this->mPerRow * ( $this->mWidths + self::THUMB_PADDING + self::GB_PADDING + self::GB_BORDERS );
221223 $oldStyle = isset( $this->mAttribs['style'] ) ? $this->mAttribs['style'] : "";
 224+ # _width is ignored by any sane browser. IE6 doesn't know max-width so it uses _width instead
222225 $this->mAttribs['style'] = "max-width: {$maxwidth}px;_width: {$maxwidth}px;" . $oldStyle;
223226 }
224227
225228 $attribs = Sanitizer::mergeAttributes(
226229 array( 'class' => 'gallery' ), $this->mAttribs );
227230
228 - $s = Xml::openElement( 'ul', $attribs );
 231+ $output = Xml::openElement( 'ul', $attribs );
229232 if ( $this->mCaption ) {
230 - $s .= "\n\t<li class='gallerycaption'>{$this->mCaption}</li>";
 233+ $output .= "\n\t<li class='gallerycaption'>{$this->mCaption}</li>";
231234 }
232235
233236 $params = array( 'width' => $this->mWidths, 'height' => $this->mHeights );
——@@ -234,6 +237,7 @@
235238 foreach ( $this->mImages as $pair ) {
236239 $nt = $pair[0];
237240 $text = $pair[1]; # "text" means "caption" here
 241+ $alt = $pair[2];
238242
239243 $descQuery = false;
240244 if ( $nt->getNamespace() == NS_FILE ) {
——@@ -272,18 +276,19 @@
273277 $thumbhtml = "\n\t\t\t".'<div style="height: '.(self::THUMB_PADDING + $this->mHeights).'px;">'
274278 . htmlspecialchars( $img->getLastError() ) . '</div>';
275279 } else {
276 - //We get layout problems with the margin, if the image is smaller
277 - //than the line-height, so we less margin in these cases.
 280+ # We get layout problems with the margin, if the image is smaller
 281+ # than the line-height (17), so we add less margin in these cases.
278282 $minThumbHeight = $thumb->height > 17 ? $thumb->height : 17;
279283 $vpad = floor(( self::THUMB_PADDING + $this->mHeights - $minThumbHeight ) /2);
280284
281285
282286 $imageParameters = array(
283287 'desc-link' => true,
284 - 'desc-query' => $descQuery
 288+ 'desc-query' => $descQuery,
 289+ 'alt' => $alt,
285290 );
286 - # In the absence of a caption, fall back on providing screen readers with the filename as alt text
287 - if ( $text == '' ) {
 291+ # In the absence of both alt text and caption, fall back on providing screen readers with the filename as alt text
 292+ if ( $alt == '' && $text == '' ) {
288293 $imageParameters['alt'] = $nt->getText();
289294 }
290295
——@@ -308,14 +313,14 @@
309314
310315 if( $this->mShowBytes ) {
311316 if( $img ) {
312 - $nb = wfMsgExt( 'nbytes', array( 'parsemag', 'escape'),
 317+ $fileSize = wfMsgExt( 'nbytes', array( 'parsemag', 'escape'),
313318 $wgLang->formatNum( $img->getSize() ) );
314319 } else {
315 - $nb = wfMsgHtml( 'filemissing' );
 320+ $fileSize = wfMsgHtml( 'filemissing' );
316321 }
317 - $nb = "$nb<br />\n";
 322+ $fileSize = "$fileSize<br />\n";
318323 } else {
319 - $nb = '';
 324+ $fileSize = '';
320325 }
321326
322327 $textlink = $this->mShowFilename ?
——@@ -332,20 +337,20 @@
333338 # in version 4.8.6 generated crackpot html in its absence, see:
334339 # http://bugzilla.wikimedia.org/show_bug.cgi?id=1765 -Ævar
335340
336 - # Weird double wrapping in div needed due to FF2 bug
 341+ # Weird double wrapping (the extra div inside the li) needed due to FF2 bug
337342 # Can be safely removed if FF2 falls completely out of existance
338 - $s .=
 343+ $output .=
339344 "\n\t\t" . '<li class="gallerybox" style="width: ' . ( $this->mWidths + self::THUMB_PADDING + self::GB_PADDING ) . 'px">'
340345 . '<div style="width: ' . ( $this->mWidths + self::THUMB_PADDING + self::GB_PADDING ) . 'px">'
341346 . $thumbhtml
342347 . "\n\t\t\t" . '<div class="gallerytext">' . "\n"
343 - . $textlink . $text . $nb
 348+ . $textlink . $text . $fileSize
344349 . "\n\t\t\t</div>"
345350 . "\n\t\t</div></li>";
346351 }
347 - $s .= "\n</ul>";
 352+ $output .= "\n</ul>";
348353
349 - return $s;
 354+ return $output;
350355 }
351356
352357 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r86752Reverting r86749: The alt stuff is far too simplistic. One way to fix it woul...diebuche09:55, 23 April 2011
r86859Add |alt= option for galleries ( Bug 18682 ). Recommit of r86749, with nested...diebuche13:51, 25 April 2011

Comments

#Comment by Duplicatebug (talk | contribs)   09:07, 23 April 2011

Hardcoded "alt=" is bad. Use the MagicWord "img_alt", which is the same as the normal filelinks. 

$magicWordAlt = MagicWord::get( 'img_alt' );
foreach
 $match = $magicWordAlt->matchVariableStartToEnd( $altmatch );
 if ( $match ) {
  $alt = $this->stripAltText( $match, false );
 }
#Comment by Duplicatebug (talk | contribs)   09:17, 23 April 2011

Please test also for other wikitext: 

<gallery>
File:Wiki.png|[[File:Wiki.png|20px|desc|alt=inneralt]]|alt=galleryalt
File:Wiki.png|{{tl|test|alt=param}}|alt=galleryalt
</gallery>

Status & tagging log