Page MenuHomePhabricator

New images don't show up in popups
Closed, ResolvedPublic

Description

Hi, as mentioned here, I just upgraded to MediaWiki 1.28.0 and now newly uploaded images don't show up in popups anymore (old images still appear perfectly though).

My logfile is filled with entries like this:

2016/11/30 23:14:58 [error] 35809#0: *1234567 FastCGI sent in stderr: "PHP message: PHP Warning:  array_flip() expects parameter 1 to be array, integer given in /path/to/wiki/extensions/PageImages/includes/ParserFileProcessingHookHandlers.php on line 121" while reading response header from upstream, client: 123.123.123.123, server: en.domain.com, request: "GET /wiki/Article_name HTTP/1.1", upstream: "fastcgi://123.123.123.123:1234", host: "en.domain.com"

My setup:
OpenBSD 6.0, nginx 1.10.1, PHP-FPM 7.0.8, MariaDB 10.0.25 and MediaWiki 1.28.0 with REL1_28 extensions.

Thanks and cheers!

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptNov 30 2016, 10:38 PM
FoXFTW added a subscriber: FoXFTW.Dec 1 2016, 12:13 AM

Getting the same error.

PHP message: PHP Warning:  array_flip() expects parameter 1 to be array, integer given in /<path>/extensions/PageImages/includes/ParserFileProcessingHookHandlers.php on line 121" while reading response header from upstream, client: <ip>, server: <server>, request: "GET /index.php?title=<title> HTTP/1.1", upstream: "fastcgi://unix:/var/run/php/php7.0-fpm.sock:", host: "<host>"

Debian 8.1, nginx 1.10.2, PHP-FPM 7.0.13, MariaDB 10.1.19, Mediawiki 1.28.0, REL1_28 extensions

What's the value of $wgPageImagesNamespaces?

FoXFTW added a comment.EditedDec 1 2016, 12:56 PM
$wgPageImagesNamespaces

does not exist in my case.

After setting it to

$wgPageImagesNamespaces = array(0);

the warnings are gone, but still no images are shown

$wgPageImagesNamespaces wasn't set here either. Setting it to $wgPageImagesNamespaces = array(0); in LocalSettings.php fixed it for me. The warnings are gone and all images show up now. Thanks and cheers!

FoXFTW added a comment.Dec 1 2016, 2:39 PM

After running PageImages/maintenance/initImageData.php and adding said Namespace everything works again.

Kghbln added a subscriber: Kghbln.Dec 1 2016, 2:44 PM

The docu page of the extension (still) claims that the namespace constant has to be used for $wgPageImagesNamespaces and is set to NS_MAIN by default. So I guess along the way a breaking change was introduced switching to the namespace number and and the same time emptying the array?

Kghbln added a comment.EditedDec 1 2016, 2:49 PM

So I guess along the way a breaking change was introduced switching to the namespace number and and the same time emptying the array?

Indeed done with rEPIMd52f94205fab12b2b3e4f9c6c607c6e53f444b3e which is a bit frustrating. Also the default is 0 so I wonder why $wgPageImagesNamespaces has to be set again in "LocalSettings.php".

@Till_Kraemer @FoXFTW Did you change from classing extension invocation to extension registration after the upgrade?

FoXFTW added a comment.EditedDec 1 2016, 2:53 PM

@Kghbln indeed, switched to extension registration after upgrading

Kghbln added a comment.Dec 1 2016, 2:58 PM

indeed, switched to extension registration after upgrading

Pooh, I never liked extension registration but it appears to be worse than ever expected. That's another story though.

So to resolve this issue somebody should update the extension's page and add the info about the findings here, i.e. $wgPageImagesNamespaces has to be set manually in any case and that one should run "initImageData.php" after the upgrade.

FoXFTW added a comment.Dec 1 2016, 3:39 PM

indeed, switched to extension registration after upgrading

Pooh, I never liked extension registration but it appears to be worse than ever expected. That's another story though.

So to resolve this issue somebody should update the extension's page and add the info about the findings here, i.e. $wgPageImagesNamespaces has to be set manually in any case and that one should run "initImageData.php" after the upgrade.

Exactly. :>

bmansurov added a comment.EditedDec 1 2016, 5:44 PM

$wgPageImagesNamespaces is already present in extension.json. I wonder why it got removed for you all.

Related change: https://gerrit.wikimedia.org/r/#/c/311184/, but didn't break anything. T152052#2838272

cc @Legoktm
I think the issue here is if you don't use wfLoadExtension and instead use require_once config values are not loaded?

It seems like we could add a check for $wgPageImagesNamespaces being undefined but I'm not sure how I feel about that.

Change 324773 had a related patch set uploaded (by Legoktm):
$wgPageImagesNamespaces should be an array

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

Legoktm claimed this task.Dec 1 2016, 7:21 PM

There was nothing wrong with extension registration or wfLoadExtension(). The REL1_28 extension.json had $wgPageImagesNamespaces as an integer, instead of an array. Calling array_filp on that is going to fail.

indeed, switched to extension registration after upgrading

Pooh, I never liked extension registration but it appears to be worse than ever expected. That's another story though.

[off-topic] I would appreciate it if you could explain the story in another task or elsewhere.

Change 324773 merged by jenkins-bot:
$wgPageImagesNamespaces should be an array

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

Legoktm closed this task as Resolved.Dec 1 2016, 7:27 PM
Till_Kraemer added a comment.EditedDec 1 2016, 8:25 PM

Did you change from classing extension invocation to extension registration after the upgrade?

Actually, I was using require_once. I'm switching to wfLoadExtension now.

Kghbln added a comment.Dec 1 2016, 8:47 PM

There was nothing wrong with extension registration or wfLoadExtension(). The REL1_28 extension.json had $wgPageImagesNamespaces as an integer, instead of an array. Calling array_filp on that is going to fail.

Ah, I figured that extension registration did not load the default values. That was the reason why I "ranted" about it here. Sorry for this. Now that the array is doing the backflip again we are cool and I am fluffy. :) Thanks!

Still the docu needs to be rectified I believe: 1. use extension registration from MW 1.28 onwards and 2. use namespace numbers instead of namespace constants from MW 1.28 onwards. Right?

Still the docu needs to be rectified I believe: 1. use extension registration from MW 1.28 onwards and 2. use namespace numbers instead of namespace constants from MW 1.28 onwards. Right?

Even if you use require_once "$IP/extensions/PageImages/PageImages.php"; everything will work fine. And namespace constants in LocalSettings.php will continue to work fine. Really, no change is *required* by sysadmins, it's just recommended that you use wfLoadExtension() as the require_once method will emit deprecation warnings in the future.

Kghbln added a comment.Dec 1 2016, 9:11 PM

@Legoktm Thanks for clarifying. I just updated the docu. The result is pretty ugly since the ExtensionInstall template is broken. Still, people will get the message.

<3 @Legoktm for making sense of all this.