Page MenuHomePhabricator

A non-numeric value encountered in includes/LinksUpdateHookHandler.php:142.
Closed, ResolvedPublic

Description

The array passed to scoreFromTable() sometimes contains strings. This forces the return to cast to an integer so that math performed with the result does not throw a non-numeric warning.

Event Timeline

Change 479769 had a related patch set uploaded (by Alexia; owner: Alexia):
[mediawiki/extensions/PageImages@master] Cast the return to integer to ensure a proper return type for math operations.

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

Jdlrobson subscribed.

sometimes contains strings

Can you provide an example of where this occurs? The patch looks fine but we'll need a way to replicate and verify there is a problem here.
I'd rather fix the issue in scoreFromTable if possibly as it doesn't sound like we should ever be generating strings here.

Low until we have some replication instructions as I can't gauge how big a problem this is without those.

Change 483089 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/PageImages@master] Fix score calculation in LinksUpdateHookHandler failing on unordered input

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

Change 483090 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/PageImages@master] Relax score calculation in LinksUpdateHookHandler to use floats

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

@Alexia, please check the local $wgPageImagesScores setting you are using in your wiki (farm). It seems it contains strings, but should only contain integers.

@Alexia, please check the local $wgPageImagesScores setting you are using in your wiki (farm). It seems it contains strings, but should only contain integers.

That setting is untouched and at the stock values in extension.json. They also all appear to be integers. This copy of PageImages was updated on 2018-10-01 on the Gamepedia stack.

Change 483098 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/PageImages@master] Remove bogus @doc… elements from default $wgPageImagesScores setting

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

Ok, cool! Then I believe the issue might be related to this mistake: https://gerrit.wikimedia.org/r/483098.

My next question is how your wiki can contain images that are reported to have 0 pixels with? Can you please check if you have a custom $wgThumbLimits setting?

Another idea I have is that the PageImages code is – for some reason – executed for files that are not images. E.g. audio files don't have a width in pixels. Two hooks trigger the code in question: ParserMakeImageParams and AfterParserFetchFileAndTitle. I believe the first is only executed for actual images, but the second might be executed when a gallery contains audio files. This would indeed be a more complicated bug in the PageImages extension.

The $wgThumbLimits setting is default/stock in the Gamepedia stack.

The Extension:EmbedVideo that Gamepedia uses has a media handler for video/audio files(in game character audio) and these tend to get used in infoboxes near the top of articles. However, that code is supposed to enforce width for the containers for browser media controls. I will have to do some in depth testing on this.
https://github.com/HydraWiki/mediawiki-embedvideo/tree/master/classes/media

Change 483105 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/PageImages@master] Don't use non-images as candidates, e.g. audio files from a <gallery>

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

Change 483098 merged by jenkins-bot:
[mediawiki/extensions/PageImages@master] Remove bogus @doc… elements from default $wgPageImagesScores setting

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

Change 483105 merged by jenkins-bot:
[mediawiki/extensions/PageImages@master] Don't use non-images as candidates, e.g. audio files from a <gallery>

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

Change 479769 abandoned by Alexia:
Cast the return to integer to ensure a proper return type for math operations.

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

Jdlrobson claimed this task.

Thanks for the report and the patches! Looks like we're done here. Thanks @Alexia and sorry it took a bit longer than normal to wrap this up. I'll make sure we do better next time!

Change 483089 merged by jenkins-bot:
[mediawiki/extensions/PageImages@master] Fix score calculation in LinksUpdateHookHandler failing on unordered input

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

Change 483090 merged by jenkins-bot:
[mediawiki/extensions/PageImages@master] Relax score calculation in LinksUpdateHookHandler to use floats

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