ProofreadPage broken for GIF images due to assuming it can always create thumbnails
Closed, ResolvedPublic

Description

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/WIDTHpx-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.


Version: unspecified
Severity: enhancement
URL: http://en.wikisource.org/wiki/Page:Summary_for_B-23.gif

bzimport set Reference to bz17791.
brion created this task.Via LegacyMar 5 2009, 1:22 AM
bzimport added a comment.Via ConduitMar 8 2009, 9:01 AM

thomasV1 wrote:

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 ?

brion added a comment.Via ConduitMar 16 2009, 6:35 PM

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. :)

bzimport added a comment.Via ConduitAug 26 2011, 8:52 PM

beau wrote:

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.

attachment proofread.patch ignored as obsolete

bzimport added a comment.Via ConduitAug 27 2011, 5:41 PM

john wrote:

r95602

brion added a comment.Via ConduitAug 28 2011, 1:17 AM

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!

bzimport added a comment.Via ConduitAug 28 2011, 1:45 PM

beau wrote:

Okay. I'll fix that, and send another version of patch.

bzimport added a comment.Via ConduitAug 28 2011, 2:44 PM

beau wrote:

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.

Attached: proofread.patch

brion added a comment.Via ConduitAug 29 2011, 6:44 PM

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).

TheDJ added a comment.Via ConduitAug 29 2011, 7:11 PM

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.

bzimport added a comment.Via ConduitAug 30 2011, 2:40 PM

beau wrote:

(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.

bzimport added a comment.Via ConduitFeb 17 2012, 2:53 PM

sumanah wrote:

Adding Zaran to cc in case he can review this patch or otherwise update status.

bzimport added a comment.Via ConduitApr 28 2012, 7:36 PM

beau wrote:

I have submitted gerrit #6078 for review.

Matanya added a comment.Via ConduitAug 13 2012, 12:37 PM

patch merged. seems to be fixed.

Add Comment

Column Prototype
This is a very early prototype of a persistent column. It is not expected to work yet, and leaving it open will activate other new features which will break things. Press "\" (backslash) on your keyboard to close it now.