Page MenuHomePhabricator

Media dialog in VisualEditor uses invalid imageinfo API parameter
Open, Needs TriagePublic

Description

  1. https://www.mediawiki.org/wiki/Project:Sandbox?veaction=edit
  2. (in DevTools) Open the "Network" tab
  3. (in VisualEditor) Click "Insert" > "Images and media"

Actual

I noticed three issues:

  • Ambigious parameters sent to the API.
  • Invalid paramaters sent to the API.
  • Results are low resolution on modern Retina/HiDPI screens.

The API request loooks ambigious:

https://commons.wikimedia.org/w/api.php?action=query&format=json&origin=*&generator=allimages&gaisort=timestamp&gaidir=descending&iiurlheight=200&iiprop=dimensions|url|mediatype|extmetadata|timestamp|user&prop=imageinfo&gaiuser=Krinkle&iiurlwidth[width]=320&iiurlwidth[height]=240&gailimit=15

Note how it sets:

  • iiurlheight=200
  • iiurlwidth[width]=320
  • iiurlwidth[height]=240

There seems to some mixing of intentions and it is unclear which one is right or which one is meant to win. There are two possible heights, and two possilbe widths.

The latter two are invalid, and the response indicates this. The iiurlwidth accepts a single number, not a key-value structure.

{"batchcomplete":"","warnings":{"main":{"*":"Parameter \"iiurlwidth\" uses unsupported PHP array syntax."}},

The actual images in the results seem to render at 300x200 with the internal bitmap at 330px (this happens automatically via MediaWiki thumbnail steps). However, we seem to be ignoring the responsiveUrls.2 property, instead of feeding it to <img srcset=>.

{ "pageid": 4242400,
  "ns": 6,
  "title": "File:Scotiapiper.jpg",
  "imageinfo": [
    {"thumburl": "https://upload.wikimedia.org/wikipedia/commons/thumb/d/dc/Scotiapiper.jpg/330px-Scotiapiper.jpg",
      "thumbwidth": 299,
      "thumbheight": 200,
      "responsiveUrls": {
        "2": "https://upload.wikimedia.org/wikipedia/commons/thumb/d/dc/Scotiapiper.jpg/960px-Scotiapiper.jpg"
      },

Screenshot 2026-05-22 at 14.23.47.png (1,896×1,056 px, 531 KB)

Other information

The warning to detect invalid iiwidth parameters was introduced in 2014 via T66057: Using incorrect array parameter syntax does not raise an error/warning

Event Timeline

On quick examination, it's not anything inside the VisualEditor extension, this is a request made by mw.widgets.MediaSearchWidget in core. When the VE media dialog opens, it makes one of those search widgets and requests that it fetch the user-upload queue (via this.search.queryMediaQueue()), and that results in this request with malformed parameters.

Tracing that through, we find mw.widgets.MediaResourceProvider setting iiurlwidth to whatever getStandardWidth says:

mw.widgets.MediaResourceProvider.prototype.getStandardWidth = function () {
	return ( this.thumbSizes && this.thumbSizes[ this.thumbSizes.length - 1 ] ) ||
		( this.imageSizes && this.imageSizes[ 0 ] ) ||
		// Fall back on a number
		300;
};

It fetches thumbSizes and imageSizes from query siteinfo using the thumblimits and imagelimits props: https://www.mediawiki.org/w/api.php?action=query&format=json&meta=siteinfo

The problem is that those values are:

"thumblimits": {
  "2": 180,
  "5": 250,
  "7": 400
},
"imagelimits": [
  {
    "width": 320,
    "height": 240
  },
  {
    "width": 640,
    "height": 480
  },
  {
    "width": 800,
    "height": 600
  },
  {
    "width": 1024,
    "height": 768
  },
  {
    "width": 1280,
    "height": 1024
  },
  {
    "width": 2560,
    "height": 2048
  }
],

So, the check for thumbSizes fails because it's not an array and doesn't have a length property. This seems to be because of T426328's patch on Tuesday (cc @Jdlrobson) which altered the config from something that would serialize to an array to something that would serialize to an object. I don't know enough about this config var to say why it wasn't just changed to [180, 250, 400] -- are the indexes actually significant?

(The check for imageSizes fails because it returns that {width:320, height:240} object entirely rather than just its width property. This seems to have always been the behavior of imageSizes, so I think this might have always been broken and just masked by thumbSizes always having been set before Tuesday.)

I could absolutely patch the widget, but I'm not sure whether or not "siteinfo returned an object for this value" should be considered the new ongoing behavior or just a bug.

Change #1292173 had a related patch set uploaded (by DLynch; author: DLynch):

[mediawiki/core@master] mw.widgets.MediaResourceProvider: invalid return from getStandardWidth

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

^ that fixes the imageSizes access, which was definitely wrong, and will stop these errors from occurring. Should probably also work out what's going on with thumblimits though.

I am not sure this will fully help things, but I added this endpoint during the hackathon for these types of checks for what thumb sizes exist for an image in a wiki instance based on the new thumb limits config: /v1/file/{title}/thumbnails https://www.mediawiki.org/wiki/Special:RestSandbox#/default/get_v1_file__title__thumbnails

that might also be helpful here?

Also, I've noted in T425221: Separate display-size selection from normalized thumbnail-size selection in media API responses that we had a couple of endpoints that returned the correct thumb URLS but the wrong thumb sizes in a couple of endpoints -- if this is the same case here, we should fix that for imageinfo and/or siteinfo.

Change #1292272 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Site info should output thumblimits as array

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

Since it's always been an array I wonder if we should use array_values for consistency with other consumers of this API ? https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1292272
If clients were relying on the index of the array to correspond with the thumbsize preference that was error prone (particularly in the case of when the preference was global) and not something I think we should be expected to continue supporting, but I consider switching from an array to an object breaking the API contract here.

Note: With the work in T426328: [1.48] 3rd parties must update ThumbLimits config we might end up doing away with these index values going forward in favor of separate config values: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1286535/ - I'll make sure any change there doesn't resurface the issues in this ticket.

We always had plans to make the thumb limits be like this: T106640#10750371 As Jon said, we might end up with different config values in the future but yeah, in the mean time just switching to array_values is better. Maybe adding a regression test would be nice though.

Change #1292173 merged by jenkins-bot:

[mediawiki/core@master] mw.widgets.MediaResourceProvider: invalid return from getStandardWidth

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

(I'll +2 the Jon's patch once CI gods are happy with us)

Change #1292272 merged by jenkins-bot:

[mediawiki/core@master] Site info should output thumblimits as array

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

Is this causing enough of a problem that we should backport the API fix on monday?

Change #1293710 had a related patch set uploaded (by Ladsgroup; author: Jdlrobson):

[mediawiki/core@wmf/1.47.0-wmf.3] Site info should output thumblimits as array

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

Change #1293710 merged by jenkins-bot:

[mediawiki/core@wmf/1.47.0-wmf.3] Site info should output thumblimits as array

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

Mentioned in SAL (#wikimedia-operations) [2026-05-26T14:07:01Z] <ladsgroup@deploy1003> Started scap sync-world: Backport for [[gerrit:1293710|Site info should output thumblimits as array (T427066)]]

Mentioned in SAL (#wikimedia-operations) [2026-05-26T14:08:54Z] <ladsgroup@deploy1003> ladsgroup: Backport for [[gerrit:1293710|Site info should output thumblimits as array (T427066)]] synced to the testservers (see https://wikitech.wikimedia.org/wiki/Mwdebug). Changes can now be verified there.

Mentioned in SAL (#wikimedia-operations) [2026-05-26T14:13:41Z] <ladsgroup@deploy1003> Finished scap sync-world: Backport for [[gerrit:1293710|Site info should output thumblimits as array (T427066)]] (duration: 06m 40s)

...
are the indexes actually significant?

$wgDefaultUserOptions['thumbsize'] indexes into the array. Just taking the array_values results in an undefined array key for the default option. wmgThumbsizeIndex is 5.

See https://www.mediawiki.org/w/api.php?action=query&format=json&meta=siteinfo&siprop=general|defaultoptions

Change #1294437 had a related patch set uploaded (by Arlolra; author: Arlolra):

[mediawiki/services/parsoid@master] Temporary fix for default thumbsize index

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

Change #1295515 had a related patch set uploaded (by C. Scott Ananian; author: C. Scott Ananian):

[mediawiki/core@master] Ensure siteinfo query returns consistent values for thumbsize and thumblimits

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

cscott subscribed.

This caused a regression in Parsoid; two different patches above to resolve it.

Change #1294437 abandoned by Arlolra:

[mediawiki/services/parsoid@master] Temporary fix for default thumbsize index

Reason:

In favour of I2650d80d62e6971ed42d20235df99868cc54fbdf

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

Change #1295515 merged by jenkins-bot:

[mediawiki/core@master] Ensure siteinfo query returns consistent values for thumbsize and thumblimits

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