Last modified: 2012-08-13 12:37:32 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T19791, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 17791 - ProofreadPage broken for GIF images due to assuming it can always create thumbnails
ProofreadPage broken for GIF images due to assuming it can always create thum...
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
ProofreadPage (Other open bugs)
unspecified
All All
: Normal enhancement with 2 votes (vote)
: ---
Assigned To: Beau
http://en.wikisource.org/wiki/Page:Su...
: patch, patch-need-review
Depends on:
Blocks: Wikisource
  Show dependency treegraph
 
Reported: 2009-03-05 01:22 UTC by Brion Vibber
Modified: 2012-08-13 12:37 UTC (History)
9 users (show)

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


Attachments
Url of thumbnail is fetched using api. (17.86 KB, patch)
2011-08-26 20:52 UTC, Beau
Details
Reduced version of patch (10.94 KB, patch)
2011-08-28 14:44 UTC, Beau
Details

Description Brion Vibber 2009-03-05 01:22:59 UTC
The ProofreadPage extension seems to be manually constructing thumb URLs and assuming it can slap in any old number. This fails when, for instance, we have disabled scaling of GIF images:

var proofreadPageThumbURL = "http://upload.wikimedia.org/wikisource/en/thumb/1/14/Summary_for_B-23.gif/##WIDTH##px-Summary_for_B-23.gif";

Resulting files fail to render:
http://upload.wikimedia.org/wikisource/en/thumb/1/14/Summary_for_B-23.gif/475px-Summary_for_B-23.gif

and so you're left with a big blank spot.

ProofreadPage's JavaScript should use the API to look up thumb URLs if it's necessary to dynamically size them.
Comment 1 ThomasV 2009-03-08 09:01:10 UTC
the API allows to retrieve the URL of a given image, eg:
http://en.wikisource.org/w/api.php?action=query&titles=Image:Summary_for_B-23.gif&prop=imageinfo&iiprop=url|size

is there an API query that returns whether scaling is enabled/disabled for a particular image format ?
Comment 2 Brion Vibber 2009-03-16 18:35:52 UTC
Just ask for a thumb of a particular size:

http://en.wikisource.org/w/api.php?action=query&titles=Image:Summary_for_B-23.gif&prop=imageinfo&iiprop=url|size&iiurlwidth=320

It'll give you back thumbnail information, which will be scaled or not as appropriate:

<ii
size="36823"
width="850"
height="1099"
thumburl="http://upload.wikimedia.org/wikisource/en/1/14/Summary_for_B-23.gif"
thumbwidth="320"
thumbheight="414"
url="http://upload.wikimedia.org/wikisource/en/1/14/Summary_for_B-23.gif"
descriptionurl="http://en.wikisource.org/wiki/File:Summary_for_B-23.gif" />

You need only use the values returned to you and don't worry about trying to decide whether to scale anything yourself... The internal system already took care of that. :)
Comment 3 Beau 2011-08-26 20:52:29 UTC
Created attachment 8977 [details]
Url of thumbnail is fetched using api.

The Proofread script asks api for thumbnail url. If the image cannot be scaled it is shown in full size. This can mess up the layout if the image is large, but at least text on the scan should be visible.
Comment 4 John Du Hart 2011-08-27 17:41:30 UTC
r95602
Comment 5 Brion Vibber 2011-08-28 01:17:10 UTC
There are bugs described in the above comment, and the patch makes a lot of unrelated style changes that hide the functional code, making it harder to review and test.

John, can you back this out pending review and fixes? Thanks!
Comment 6 Beau 2011-08-28 13:45:13 UTC
Okay. I'll fix that, and send another version of patch.
Comment 7 Beau 2011-08-28 14:44:25 UTC
Created attachment 8982 [details]
Reduced version of patch

I have removed minor changes I did when browsing the code. I'll stick to changes related to fixing the bug only.

I have also fixed layout issue. The image is scaled down to desired size.
Comment 8 Brion Vibber 2011-08-29 18:44:06 UTC
Looks like fairly reasonablish code overall, but all this really needs a testing plan to confirm that it fixes the described bugs and doesn't cause regressions.


Are bits like this code necessary?

					if( !imageinfo.thumburl ) {

44 	

						// Unable to fetch a thumbnail, use the image without scaling and lie about its size

45 	

						// This works only for non-paged files though

46 	

						if ( mw.config.get( 'proofreadPageFilePage' ) == null ) {

47 	

							height = ( quantizedWidth / fullWidth ) * fullHeight;

48 	

							callback( imageinfo.url, quantizedWidth, height );

49 	

						}

50 	

					}

this seems to be redundant; per comments above, we should get back a thumburl entry whenever we've asked for a size, and we should always be asking for a size ("original size" may be unreasonably *huge* for some scans).
Comment 9 Derk-Jan Hartman 2011-08-29 19:11:02 UTC
You won't get back a thumburl for audio fragments, or any other file format that does not support thumb nailing (like ogg video if you don't have the scalers installed for instance).

Not sure how much you have to take that into account.
Comment 10 Beau 2011-08-30 14:40:52 UTC
(In reply to comment #8)
> this seems to be redundant; per comments above, we should get back a thumburl
> entry whenever we've asked for a size, and we should always be asking for a
> size ("original size" may be unreasonably *huge* for some scans).

The javascript asks for a specific size, which is based on the size of user screen during page load. If the software is unable to scale the image for whatever reason there is it tries to show the original image and let the browser do the scaling. If it is unwanted behaviour, we can get rid of that and do not show images.

(In reply to comment #9)
> You won't get back a thumburl for audio fragments, or any other file format
> that does not support thumb nailing (like ogg video if you don't have the
> scalers installed for instance).
> 
> Not sure how much you have to take that into account.

Proofread Page is meant for scans. If wikisource community want to write down dialogs from recordings they should use different extension (they don't need paging, but subtitles). The code could check if the file is an image, but I don't know how.
Comment 11 Sumana Harihareswara 2012-02-17 14:53:30 UTC
Adding Zaran to cc in case he can review this patch or otherwise update status.
Comment 12 Beau 2012-04-28 19:36:36 UTC
I have submitted Gerrit change #6078 for review.
Comment 13 matanya 2012-08-13 12:37:32 UTC
patch merged. seems to be fixed.

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


Navigation
Links