Page MenuHomePhabricator

Gallery sometimes ignores image when using filename copied from URL
Closed, ResolvedPublic

Description

Author: brianna.laugher

Description:
MediaWiki image title: Image:Nunâ island Upernavik district 2007-08-09 3.jpg
URL "title": Image:Nun%C3%A2_island_Upernavik_district_2007-08-09_3.jpg

If the URL title is used in <gallery> syntax, it gets completely ignored. Not even any text prints out. However using it in a [[:link]] or just normally showing an image [[image:foo.jpg|thumb]] works fine.

This affects images with commas, single and double quotes and parentheses, as well as accented and non-Latin characters such as "â".

thanks.


Version: 1.12.x
Severity: minor
URL: http://commons.wikimedia.org/wiki/User:Pfctdayelise/gallery_character_bug

Details

Reference
bz11659

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 9:59 PM
bzimport set Reference to bz11659.
bzimport added a subscriber: Unknown Object (MLST).

brianna.laugher wrote:

The problem is possibily in Parser.php function renderImageGallery around line 4459:

<code>
$tp = Title::newFromText( $matches[1] );
$nt =& $tp;
if( is_null( $nt ) ) {

Bogus title. Ignore these so we don't bomb out later.

continue;
}
</code>

I'm not totally sure what constitutes a "bogus" title, since redlinked images appear in galleries... Ideally the image should show up, if not then it should be treated like a filename that doesn't exist, ie. the name just prints out as plain text.

De-URL-escaping in wiki links is done in the parser itself as part of double-bracket handling. Title::newFromText() currently doesn't do de-URL-escaping. It's not 100% clear whether it should or not.

Note that URL-style percent codes are forbidden in raw titles to protect against titles which can't be consistently typed or cut-n-pasted -- that is, ensuring that 'Blah%20blah' can only mean 'Blah blah' and not a title actually called 'Blah%20blah'. So running a percent-encoded string through Title::newFromText() will come back with null, indicating an invalid title.

brianna.laugher wrote:

OK, so if we assume for the moment that Title::newFromText() shouldn't do de-URL-escaping, how can we make URL-escaped titles behave the same as non-existent titles? ie. just print out the text.

My impression is that text just gets printed out when it fails this if statement: "if ( $title instanceof File )" in class ImageGallery functions add() and insert().

What if the is_null check in Parser.php is removed and functions add() and insert() are changed

old:

if ( $title instanceof File ) {

new:

if ( $title instanceof File && !is_null($title) ) {

Bryan.TongMinh wrote:

Fixed in r33978.

Reverted in r34064. The indiscriminate use of urldecode() here breaks images with a "+" character in their names.

Bryan.TongMinh wrote:

Hm should be rawurldecode() then?

Bryan.TongMinh wrote:

Fixed in r35683: Now first checks whether the link contains a %, similarly as done for normal links.