Page MenuHomePhabricator

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

Details

Reference
bz17791

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:33 PM
bzimport added a project: ProofreadPage.
bzimport set Reference to bz17791.

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 ?

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

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

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!

beau wrote:

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

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:

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

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.

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.

sumanah wrote:

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

beau wrote:

I have submitted gerrit #6078 for review.

patch merged. seems to be fixed.