Page MenuHomePhabricator

Sizing of `framed` images
Closed, ResolvedPublic

Description

https://www.mediawiki.org/wiki/Help:Images#Size_and_frame says:

An image with frame always ignores the size specification, the original image
will be reduced if it exceeds the maximum size defined in user preferences.

However, as bug 53514 describes, the actual behavior seems to have regressed at some point. The code in includes/Linker.php to check the user preference for size (which seems to be the same as the thumbnail size preference) is unreachable. As presently written, framed images are always rendered at their "natural" size, *unless a height* is specified, in which case the height is honored (and the width, if any, ignored).

It's not entirely clear what the proper fix should be. There are three options:

  1. Leave the code as is, and update the spec on the wiki.
  2. If a height is specified, honor the height *and* width (conservative, should change few pages)
  3. Implement what I think was the original idea: *always* ignore the width and height, and limit the maximum image size with the user's thumbnail size preference.

Version: 1.23.0
Severity: normal

Details

Reference
bz62258

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 3:00 AM
bzimport added a project: MediaWiki-Parser.
bzimport set Reference to bz62258.
cscott created this task.Mar 5 2014, 3:04 PM

(In reply to C. Scott Ananian from comment #0)

https://www.mediawiki.org/wiki/Help:Images#Size_and_frame says:

An image with frame always ignores the size specification, the original image
will be reduced if it exceeds the maximum size defined in user preferences.

However, as bug 53514 describes, the actual behavior seems to have regressed
at some point. The code in includes/Linker.php to check the user preference
for size (which seems to be the same as the thumbnail size preference) is
unreachable. As presently written, framed images are always rendered at
their "natural" size, *unless a height* is specified, in which case the
height is honored (and the width, if any, ignored).

It's not entirely clear what the proper fix should be. There are three
options:

  1. Leave the code as is, and update the spec on the wiki.
  2. If a height is specified, honor the height *and* width (conservative,

should change few pages)

  1. Implement what I think was the original idea: *always* ignore the width

and height, and limit the maximum image size with the user's thumbnail size
preference.

I think option 2 is best; option 3 would be quite disruptive given that I think this is how it's been for a while (getting out git blame here might be interesting), so people aren't expecting this fundamental a thing to change much. We should talk to the Multimedia team a bit here to check with them, though.

However, MediaWiki's image system needs some serious review, and I don't think a bug is the place to do it. Some possible questions, just to illustrate:

*Why* does a frame image work like a thumb image except for the thumbnail and the magnifying glass icon?

*Why* does frameless not operate like frame with respect to size, but does for everything else? (Though that might be just this bug, I guess?)

*Why* are galleries fundamentally different in options and some of the new gallery options like hover-caption aren't implemented?

Given that the proposed changes to Vector (but not Monobook, and not MediaWiki core?) in Typography Update remove the frame from frame and thumb images, should those options be merged? Should the wikitext for "thumb" be killed off? Should |border=1 still work in these cases? Why?

… etc.

As you say, James, these are mostly questions for an RFC, and not really relevant here, though maybe blockers. A separate bug is called for.

For what it's worth I would err in the direction of following the spec, but without a complaining user it may just be that the spec has fallen out of favour with reality. I would suggest asking on VPT and a few other sites that use lots of images in their work - Wikibooks and Commons come to mind - and maybe getting some CL time to help you sort out the replies, and post to more languages than just English.

HTH...

Change 116995 had a related patch set uploaded by Cscott:
Ignore height as well as width for framed images.

https://gerrit.wikimedia.org/r/116995

Parsoid commit https://gerrit.wikimedia.org/r/#/c/116983/ makes Parsoid match the old (broken) behavior. We'll need to partly undo it once this bug is fixed.

cscott added a comment.Apr 3 2014, 4:10 PM

To be specific, the reason why framed images respect height (but not width) is because the linker uses:

$thumb = $file->getUnscaledThumb( $hp );

but includes/filerepo/file/File.php::getUnscaledThumb forces the requested width to be the same as the images natural width *but not the height*. So if there is a height specification, getUnscaledThumb actually returns a scaled thumb. Which is... a bug.

https://gerrit.wikimedia.org/r/116995 fixes this, but probably in the wrong way (unsets height for framed images). File::getUnscaledThumb should probably be fixed so that it ignores the specified height if present.

The patch attached to https://gerrit.wikimedia.org/r/116995 now fixes the bug in the "right way" -- that is, it fixes File::getUnscaledThumb.

This will change the size of all images which combine a height bound and a framed image.

In order to determine whether fixing this bug will affect this display of too many images, I wrote a script to grunge through all images and pages containing images on a wiki. See https://github.com/cscott/wp-image-info

One limitation is that it uses the wp allimages API query, so it does not include pages which include images *only from commons*.

I'm still collecting data for enwiki, but for frwiki:

43,014 total non-commons images
 8,839 of them are in portrait orientation

1,242,985 image links on pages, of which only

60 use the 'framed' option with a height bound

For dewiki:

170,701 total non-commons images
 55,208 of these are in portrait orientation

1,786,779 image links on pages, of which only

538 use the 'framed' option with a height bound

I fixed some of these:
https://de.wikipedia.org/w/index.php?title=Hauptkirche_Sankt_Michaelis_%28Hamburg%29&diff=129396939&oldid=129292349
https://de.wikipedia.org/w/index.php?title=Ceylor&diff=129396510&oldid=129332266
https://de.wikipedia.org/w/index.php?title=Ceylor&diff=129396510&oldid=129332266

Sorry, my stats were slightly wrong above:
frwiki:

60 image links using the "framed" option
 1 of which uses a height bound

dewiki:
538 image links using the "framed" option

20 of which use a height bound

So on dewiki/frwiki I've only been about to find 21 image links whose size would be changed by this bug fix. I'm still collecting data on enwiki, I'll update this bug with the numbers when I have them.

Full list for dewiki:
Ceylor Datei:Lochtest.png x206px
Ceylor Datei:Volumentest.png x206px
Fernsehwerbung Datei:Werbeanteile.png x200px
Fahrtanzeiger Datei:FSM_WT.png 500x300px
Friedrich Glauner Datei:Grafik_Wertetrichter_unterlegt.png 480x367px
Gliederung des Feldheeres (Bundeswehr, Heeresstruktur 4) Datei:II._Korps.svg x150px
Gliederung des Feldheeres (Bundeswehr, Heeresstruktur 4) Datei:III._Korps.svg x150px
Gliederung des Feldheeres (Bundeswehr, Heeresstruktur 4) Datei:I._Korps.svg x150px
Hauptkirche Sankt Michaelis (Hamburg) Datei:Oratorium_zur_Einweyhung_der_Michaeliskirche_1762.JPG 180x180px
Opernhaus vorm Salztor Datei:Salztor_NMB_1800.JPG 200x200px
Opernhaus vorm Salztor Datei:Karte_NB_Oper.jpg 300x300px
Raketenartilleriebataillon Datei:APP-6_MLRS.svg x80px
Reedschalter Datei:Form_A_-_Schließer_(Normally_Open).jpg x50px
Reedschalter Datei:Form_C_-Wechsler_bzw._Umschalter.jpg x50px
Rheintal-Express Datei:Rheintal-Express_RABe_511.jpg 260x260px
SV Salamander Kornwestheim Datei:Logo_TV_Kornwestheim_Kodiaks.gif x130px
STUDIOPARK KinderMedienZentrum Datei:STUDIOPARK_KinderMedienZentrum.png x100px
ToiToiToi Datei:Toitoitoi-symbole.svg x70px
Spotlight (Software) Datei:Spotlight-Suche_im_Finder.png x222px
Zwergsignal Datei:Zs-beispiel1.png 500x300px

(In reply to comment #8)
cscott had already fixed the pages "Gliederung des Feldheeres (Bundeswehr, Heeresstruktur 4)", "Hauptkirche Sankt Michaelis (Hamburg)", and "Ceylor" and I've fixed the rest now on de.wp.

Change 116995 merged by jenkins-bot:
Ignore height as well as width for framed images.

https://gerrit.wikimedia.org/r/116995

Change 125876 had a related patch set uploaded by Cscott:
Update release notes to describe user-visible change to framed images.

https://gerrit.wikimedia.org/r/125876

Change 125876 merged by jenkins-bot:
Update release notes to describe user-visible change to framed images.

https://gerrit.wikimedia.org/r/125876

Change 125885 had a related patch set uploaded by Cscott:
Match fixed PHP behavior for framed images with a height specification.

https://gerrit.wikimedia.org/r/125885

Change 125885 merged by jenkins-bot:
Match fixed PHP behavior for framed images with a height specification.

https://gerrit.wikimedia.org/r/125885

You have missed the localized version at least for dewiki ("gerahmt"), see [1]
There are also english aliase ("enframed" and "frame")

[1] https://de.wikipedia.org/w/index.php?title=Airbus_A350&oldid=129797202#Zweiter_Entwurf

We use the parsoid HTML for the page, so we do in fact handle localized option names. However, the caveat in comment 6 applies: "One limitation is that it uses the wp allimages API query, so it does not include pages which include images *only from commons*." That seems to have been the case on the page you link to.

I can't be bothered to read through and interpret the logic of height bound and width bound and square cropped box ponies.

But I'm confused by two things:

  1. I can understand why certain keywords should not result in a cropped image and thus for 'frame' to now only respect height of width. But ignoring both seems like a pointless change that is asking to break and upset existing content.

Existing usage like [[File:Example.jpg|frame]] would be unaffected, but existing content like [[File:Test wiki logo notext.png|frame|200x200px]] will not result in a gigantic 3000px image being framed on the page[1]

  1. Wait, we made this change because some old Help documentation page[2] on mediawiki.org says[3] so? That's not by any means a specification. Also, that same exact documentation page has an example of [[File:Example.jpg|frame|50px]] which is now broken[4] by being way too large.

[1] https://www.mediawiki.org/w/index.php?title=VisualEditor:Test&oldid=1011846
[2] https://www.mediawiki.org/w/index.php?title=Help:Images&oldid=945436
[3] "An image with frame always ignores the size specification, the original image will be reduced if it exceeds the maximum size defined in user preferences."
[4] https://www.mediawiki.org/w/index.php?title=Help:Images&oldid=945436#Size_and_frame

(In reply to Krinkle from comment #17)

I can't be bothered to read through and interpret the logic of height bound
and width bound and square cropped box ponies.

But I'm confused by two things:

  1. I can understand why certain keywords should not result in a cropped

image and thus for 'frame' to now only respect height of width. But ignoring
both seems like a pointless change that is asking to break and upset
existing content.

Existing usage like [[File:Example.jpg|frame]] would be unaffected, but
existing content like [[File:Test wiki logo notext.png|frame|200x200px]]
will not result in a gigantic 3000px image being framed on the page[1]

Note that only 200x200px was scaled so far, and is rare in combination with frame. 200px was *not* scaled with frame, and still isn't.

  1. Wait, we made this change because some old Help documentation page[2] on

mediawiki.org says[3] so? That's not by any means a specification. Also,
that same exact documentation page has an example of
[[File:Example.jpg|frame|50px]] which is now broken[4] by being way too
large.

There should be no change here at all. Framed images with only a width bbox specified were not scaled so far.

Gabriel

I checked image documentation on mw as well as enwiki and other language-specific wikis, as well as reading the code. It was clear that the intent was as documented -- framed images were original size. They even called getUnscaledThumb in the code. However, getUnscaledThumb had a bug so that it did not always return unscaled thumbs. This affected a miniscule number of pages, all of which (upon examination) were really trying to do something else (usually 'thumb' not 'frame'). We fixed most of those pages, and fixed the bug. This bug has been merged and closed for over a month now and is already live on all the wikis. (See https://en.wikipedia.org/wiki/User:Cscott/framed )

I think you are actually trying to review the 'square bounding box' RfC -- which is Not This Bug.