Error
MediaWiki version: 1.36.0-wmf.10
Use of Language::commafy with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum]
MediaWiki version: 1.36.0-wmf.10
Use of Language::commafy with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum]
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, string, array) #1 /srv/mediawiki/php-1.36.0-wmf.10/includes/debug/MWDebug.php(329): trigger_error(string, integer) #2 /srv/mediawiki/php-1.36.0-wmf.10/includes/debug/MWDebug.php(305): MWDebug::sendRawDeprecated(string, boolean, string) #3 /srv/mediawiki/php-1.36.0-wmf.10/includes/debug/MWDebug.php(234): MWDebug::deprecatedMsg(string, string, string, integer) #4 /srv/mediawiki/php-1.36.0-wmf.10/includes/GlobalFunctions.php(1029): MWDebug::deprecated(string, string, string, integer) #5 /srv/mediawiki/php-1.36.0-wmf.10/languages/Language.php(3341): wfDeprecated(string, string) #6 /srv/mediawiki/php-1.36.0-wmf.10/languages/Language.php(3271): Language->commafy(string) #7 /srv/mediawiki/php-1.36.0-wmf.10/includes/language/Message.php(1176): Language->formatNum(string) #8 /srv/mediawiki/php-1.36.0-wmf.10/includes/language/Message.php(1140): Message->extractParam(array, string) #9 /srv/mediawiki/php-1.36.0-wmf.10/includes/language/Message.php(872): Message->replaceParameters(string, string, string) #10 /srv/mediawiki/php-1.36.0-wmf.10/includes/language/Message.php(929): Message->toString(string) #11 /srv/mediawiki/php-1.36.0-wmf.10/includes/EditPage.php(3952): Message->parse() #12 /srv/mediawiki/php-1.36.0-wmf.10/includes/EditPage.php(3248): EditPage::getPreviewLimitReport(ParserOutput) #13 /srv/mediawiki/php-1.36.0-wmf.10/includes/EditPage.php(699): EditPage->showEditForm() #14 /srv/mediawiki/php-1.36.0-wmf.10/includes/actions/EditAction.php(71): EditPage->edit() #15 /srv/mediawiki/php-1.36.0-wmf.10/includes/actions/SubmitAction.php(38): EditAction->show() #16 /srv/mediawiki/php-1.36.0-wmf.10/includes/MediaWiki.php(527): SubmitAction->show() #17 /srv/mediawiki/php-1.36.0-wmf.10/includes/MediaWiki.php(313): MediaWiki->performAction(Article, Title) #18 /srv/mediawiki/php-1.36.0-wmf.10/includes/MediaWiki.php(940): MediaWiki->performRequest() #19 /srv/mediawiki/php-1.36.0-wmf.10/includes/MediaWiki.php(543): MediaWiki->main() #20 /srv/mediawiki/php-1.36.0-wmf.10/index.php(53): MediaWiki->run() #21 /srv/mediawiki/php-1.36.0-wmf.10/index.php(46): wfIndexMain() #22 /srv/mediawiki/w/index.php(3): require(string) #23 {main}
Change 629219 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] Disable deprecated warning in Language::commafy() for non numeric string
Change 629188 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@wmf/1.36.0-wmf.10] Disable deprecated warning in Language::commafy() for non numeric string
Change 629188 merged by jenkins-bot:
[mediawiki/core@wmf/1.36.0-wmf.10] Disable deprecated warning in Language::commafy() for non numeric string
The culprit here seems to be Scribunto. Eg:
https://codesearch.wmcloud.org/search/?q=scribunto-limitreport&i=nope&files=&repos= shows that we have scribunto-limitreport-timeusage-value defined. But that should be fine because https://gerrit.wikimedia.org/g/mediawiki/extensions/Scribunto/+/2bec230e3d088b8b0218c618ca261c1c6d226497/includes/engines/LuaSandbox/LuaSandboxEngine.php#63 shows that the parameters are numeric.
On the other hand, scribunto-limitreport-logs is definitely non-numeric, and the message key is defined so it will attempt to format it. Patch forthcoming.
(This isn't the case in core: cachereport-origin and limitreport-timingprofile are both non-numeric, but neither of those message keys is defined so EditPage won't try to format them.)
Change 629229 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] EditPage: Don't apply numeric formatting unless a value message is defined
Change 629203 had a related patch set uploaded (by Urbanecm; owner: Urbanecm):
[mediawiki/core@wmf/1.36.0-wmf.10] Revert "Disable deprecated warning in Language::commafy() for non numeric string"
Change 629203 merged by Urbanecm:
[mediawiki/core@wmf/1.36.0-wmf.10] Revert "Disable deprecated warning in Language::commafy() for non numeric string"
Change 629426 had a related patch set uploaded (by Ahmon Dancy; owner: Reedy):
[mediawiki/core@wmf/1.36.0-wmf.10] Disable deprecated warning in Language::commafy() for non numeric string
Change 629219 merged by jenkins-bot:
[mediawiki/core@master] Disable deprecated warning in Language::commafy() for non numeric string
Change 629426 merged by jenkins-bot:
[mediawiki/core@wmf/1.36.0-wmf.10] Disable deprecated warning in Language::commafy() for non numeric string
The commented out deprecation warning has been deployed to production so I removed the Wikimedia-production-error tag. I changed the priority from UBN to High.
We can either leave this task open for dealing with Scribunto, or a separate task can be created for that and this one can be closed.
Change 629229 merged by jenkins-bot:
[mediawiki/core@master] EditPage: Don't apply numeric formatting unless a value message is defined
Change 635374 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Re-enable deprecated warning in Language::commafy() for non-numeric string
Change 635374 merged by jenkins-bot:
[mediawiki/core@master] Re-enable deprecation warning in Language::commafy() for non-numeric string
This is now sending deprecation warnings on the beta cluster. 4 seen in the last 24 hours: https://logstash-beta.wmflabs.org/goto/bf153a71f0c6f9d43bb9c20a112cccf4
Look to be media file related
[X4@PCKwQBHcAADooZKUAAABV] /wiki/File:A_solid_ghost_hunting_breakfast_2013-06-05_10-55.jpg ErrorException from line 3263 of /srv/mediawiki/php-master/languages/Language.php: Use of Language::commafy with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum] #0 [internal function]: MWExceptionHandler::handleError(integer, string, string, string, array) #1 /srv/mediawiki/php-master/includes/debug/MWDebug.php(329): trigger_error(string, integer) #2 /srv/mediawiki/php-master/includes/debug/MWDebug.php(305): MWDebug::sendRawDeprecated(string, boolean, string) #3 /srv/mediawiki/php-master/includes/debug/MWDebug.php(234): MWDebug::deprecatedMsg(string, string, string, integer) #4 /srv/mediawiki/php-master/includes/GlobalFunctions.php(1029): MWDebug::deprecated(string, string, string, integer) #5 /srv/mediawiki/php-master/languages/Language.php(3333): wfDeprecated(string, string) #6 /srv/mediawiki/php-master/languages/Language.php(3263): Language->commafy(string) #7 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(1278): Language->formatNum(string) #8 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(992): FormatMetadata->formatNum(string) #9 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(92): FormatMetadata->makeFormattedData(array) #10 /srv/mediawiki/php-master/includes/media/MediaHandler.php(549): FormatMetadata::getFormattedData(array, RequestContext) #11 /srv/mediawiki/php-master/includes/media/ExifBitmapHandler.php(140): MediaHandler->formatMetadataHelper(array, RequestContext) #12 /srv/mediawiki/php-master/includes/filerepo/file/File.php(1919): ExifBitmapHandler->formatMetadata(ForeignDBFile, RequestContext) #13 /srv/mediawiki/php-master/includes/page/ImagePage.php(131): File->formatMetadata(RequestContext) #14 /srv/mediawiki/php-master/includes/actions/ViewAction.php(74): ImagePage->view() #15 /srv/mediawiki/php-master/includes/MediaWiki.php(530): ViewAction->show() #16 /srv/mediawiki/php-master/includes/MediaWiki.php(316): MediaWiki->performAction(ImagePage, Title) #17 /srv/mediawiki/php-master/includes/MediaWiki.php(943): MediaWiki->performRequest() #18 /srv/mediawiki/php-master/includes/MediaWiki.php(546): MediaWiki->main() #19 /srv/mediawiki/php-master/index.php(53): MediaWiki->run() #20 /srv/mediawiki/php-master/index.php(46): wfIndexMain() #21 /srv/mediawiki/w/index.php(3): require(string) #22 {main}
and 3 are MediaWiki-extensions-PdfHandler related
[X4@QLawQBHcAADooZZgAAABM] /wiki/File:A_Basic_Guide_to_Open_Educational_Resources.pdf?page=2 ErrorException from line 3263 of /srv/mediawiki/php-master/languages/Language.php: Use of Language::commafy with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum] #0 [internal function]: MWExceptionHandler::handleError(integer, string, string, string, array) #1 /srv/mediawiki/php-master/includes/debug/MWDebug.php(329): trigger_error(string, integer) #2 /srv/mediawiki/php-master/includes/debug/MWDebug.php(305): MWDebug::sendRawDeprecated(string, boolean, string) #3 /srv/mediawiki/php-master/includes/debug/MWDebug.php(234): MWDebug::deprecatedMsg(string, string, string, integer) #4 /srv/mediawiki/php-master/includes/GlobalFunctions.php(1029): MWDebug::deprecated(string, string, string, integer) #5 /srv/mediawiki/php-master/languages/Language.php(3333): wfDeprecated(string, string) #6 /srv/mediawiki/php-master/languages/Language.php(3263): Language->commafy(string) #7 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(1278): Language->formatNum(string) #8 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(992): FormatMetadata->formatNum(string) #9 /srv/mediawiki/php-master/includes/media/FormatMetadata.php(92): FormatMetadata->makeFormattedData(array) #10 /srv/mediawiki/php-master/includes/media/MediaHandler.php(549): FormatMetadata::getFormattedData(array, RequestContext) #11 /srv/mediawiki/php-master/extensions/PdfHandler/includes/PdfHandler.php(367): MediaHandler->formatMetadataHelper(array, RequestContext) #12 /srv/mediawiki/php-master/includes/filerepo/file/File.php(1919): PdfHandler->formatMetadata(ForeignAPIFile, RequestContext) #13 /srv/mediawiki/php-master/includes/page/ImagePage.php(131): File->formatMetadata(RequestContext) #14 /srv/mediawiki/php-master/includes/actions/ViewAction.php(74): ImagePage->view() #15 /srv/mediawiki/php-master/includes/MediaWiki.php(530): ViewAction->show() #16 /srv/mediawiki/php-master/includes/MediaWiki.php(316): MediaWiki->performAction(ImagePage, Title) #17 /srv/mediawiki/php-master/includes/MediaWiki.php(943): MediaWiki->performRequest() #18 /srv/mediawiki/php-master/includes/MediaWiki.php(546): MediaWiki->main() #19 /srv/mediawiki/php-master/index.php(53): MediaWiki->run() #20 /srv/mediawiki/php-master/index.php(46): wfIndexMain() #21 /srv/mediawiki/w/index.php(3): require(string) #22 {main}
Got one on my dev wiki too previewing a page... Possibly a customised messsage, but maybe not
Deprecated: Use of Language::commafy with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum in /var/www/wiki/mediawiki/core/languages/Language.php at line 3263] in /var/www/wiki/mediawiki/core/includes/debug/MWDebug.php on line 329 1 0.0001 383656 {main}( ) /var/www/wiki/mediawiki/core/index.php': 0 2 0.0245 3684864 wfIndexMain( ) /var/www/wiki/mediawiki/core/index.php': 46 3 0.0246 3687008 MediaWiki->run( ) /var/www/wiki/mediawiki/core/index.php': 53 4 0.0247 3687264 MediaWiki->main( ) /var/www/wiki/mediawiki/core/includes/MediaWiki.php': 546 5 0.0295 4366936 MediaWiki->performRequest( ) /var/www/wiki/mediawiki/core/includes/MediaWiki.php': 943 6 0.0300 4379872 MediaWiki->performAction( ) /var/www/wiki/mediawiki/core/includes/MediaWiki.php': 316 7 0.0302 4380256 SubmitAction->show( ) /var/www/wiki/mediawiki/core/includes/MediaWiki.php': 530 8 0.0314 4523968 SubmitAction->show( ) /var/www/wiki/mediawiki/core/includes/actions/SubmitAction.php': 38 9 0.0319 4563400 EditPage->edit( ) /var/www/wiki/mediawiki/core/includes/actions/EditAction.php': 71 10 0.0438 5103840 EditPage->showEditForm( ) /var/www/wiki/mediawiki/core/includes/EditPage.php': 701 11 0.6170 8945768 EditPage::getPreviewLimitReport( ) /var/www/wiki/mediawiki/core/includes/EditPage.php': 3259 12 0.6441 9019128 Message->parse( ) /var/www/wiki/mediawiki/core/includes/EditPage.php': 3971 13 0.6441 9019128 Message->toString( ) /var/www/wiki/mediawiki/core/includes/language/Message.php': 929 14 0.6441 9019128 Message->replaceParameters( ) /var/www/wiki/mediawiki/core/includes/language/Message.php': 872 15 0.6441 9019128 Message->extractParam( ) /var/www/wiki/mediawiki/core/includes/language/Message.php': 1140 16 0.6441 9019504 Language->formatNum( ) /var/www/wiki/mediawiki/core/includes/language/Message.php': 1176 17 0.6441 9019504 Language->commafy( ) /var/www/wiki/mediawiki/core/languages/Language.php': 3263 18 0.6441 9019504 wfDeprecated( ) /var/www/wiki/mediawiki/core/languages/Language.php': 3333 19 0.6441 9019504 MWDebug::deprecated( ) /var/www/wiki/mediawiki/core/includes/GlobalFunctions.php': 1029 20 0.6441 9019616 MWDebug::deprecatedMsg( ) /var/www/wiki/mediawiki/core/includes/debug/MWDebug.php': 234 21 0.6441 9148120 MWDebug::sendRawDeprecated( ) /var/www/wiki/mediawiki/core/includes/debug/MWDebug.php': 303 22 0.6441 9148120 <a href='http://www.php.net/function.trigger-error' target='_new'>trigger_error</a> ( ) /var/www/wiki/mediawiki/core/includes/debug/MWDebug.php': 329
Deprecated: Use of Language::formatNum with a non-numeric string was deprecated in MediaWiki 1.36. [Called from Language::formatNum in /var/www/wiki/mediawiki/core/languages/Language.php at line 3267] in /var/www/wiki/mediawiki/core/includes/debug/MWDebug.php on line 329 Call Stack # Time Memory Function Location 1 0.0001 384296 {main}( ) .../index.php:0 2 0.0515 3687360 wfIndexMain( ) .../index.php:46 3 0.0515 3689504 MediaWiki->run( ) .../index.php:53 4 0.0516 3689760 MediaWiki->main( ) .../MediaWiki.php:546 5 0.0570 4369752 MediaWiki->performRequest( ) .../MediaWiki.php:943 6 0.0576 4382688 MediaWiki->performAction( ) .../MediaWiki.php:316 7 0.0579 4383232 SubmitAction->show( ) .../MediaWiki.php:530 8 0.0590 4526944 SubmitAction->show( ) .../SubmitAction.php:38 9 0.0597 4566376 EditPage->edit( ) .../EditAction.php:71 10 0.0728 5112768 EditPage->showEditForm( ) .../EditPage.php:702 11 0.6202 8961576 EditPage::getPreviewLimitReport( ) .../EditPage.php:3282 12 0.6487 9027384 Message->parse( ) .../EditPage.php:3994 13 0.6487 9027384 Message->toString( ) .../Message.php:929 14 0.6487 9027384 Message->replaceParameters( ) .../Message.php:872 15 0.6487 9027384 Message->extractParam( ) .../Message.php:1140 16 0.6487 9027760 Language->formatNum( ) .../Message.php:1176 17 0.6487 9027760 Language->formatNumInternal( ) .../Language.php:3267 18 0.6487 9027760 wfDeprecated( ) .../Language.php:3289 19 0.6487 9027760 MWDebug::deprecated( ) .../GlobalFunctions.php:1029 20 0.6487 9027872 MWDebug::deprecatedMsg( ) .../MWDebug.php:234 21 0.6488 9156376 MWDebug::sendRawDeprecated( ) .../MWDebug.php:303 22 0.6488 9156376 trigger_error ( ) .../MWDebug.php:329
I'm guessing EditPage::getPreviewLimitReport( ) is the key there
It's being passed a string of 5.86 MB
<div class="preview-limit-report-wrapper"><table class="preview-limit-report wikitable"><tbody><tr><th>CPU time usage</th><td>0.222 seconds</td></tr><tr><th>Real time usage</th><td>0.467 seconds</td></tr><tr><th>Preprocessor visited node count</th><td>1,527/1,000,000</td></tr><tr><th>Post-expand include size</th><td>11,392/2,097,152 bytes</td></tr><tr><th>Template argument size</th><td>188/2,097,152 bytes</td></tr><tr><th>Highest expansion depth</th><td>5/40</td></tr><tr><th>Expensive parser function count</th><td>0/100</td></tr><tr><th>Unstrip recursion depth</th><td>0/20</td></tr><tr><th>Unstrip post-expand size</th><td>1,279/5,000,000 bytes</td></tr><tr><th>Lua time usage</th><td>0.030/7 seconds</td></tr><tr><th>Lua virtual size</th><td>5.86 MB/50 MB</td></tr><tr><th>Lua estimated memory usage</th><td>0 bytes</td></tr>
Which pretty printed...
<div class="preview-limit-report-wrapper"><table class="preview-limit-report wikitable"> CPU time usage 0.222 seconds Real time usage 0.467 seconds Preprocessor visited node count 1,527/1,000,000 Post-expand include size 11,392/2,097,152 bytes Template argument size 188/2,097,152 bytes Highest expansion depth 5/40 Expensive parser function count 0/100 Unstrip recursion depth 0/20 Unstrip post-expand size 1,279/5,000,000 bytes Lua time usage 0.030/7 seconds Lua virtual size 5.86 MB/50 MB Lua estimated memory usage 0 bytes
Looks like it's Scribunto
From LuaStandaloneEngine... I guess both of these are the problem. formatSize
case 'scribunto-limitreport-virtmemusage': $value = array_map( [ $lang, 'formatSize' ], $value ); break; case 'scribunto-limitreport-estmemusage': $value = $lang->formatSize( $value ); break;
and
$output->setLimitReportData( 'scribunto-limitreport-virtmemusage', [ $status['vsize'], $this->options['memoryLimit'] ] ); $output->setLimitReportData( 'scribunto-limitreport-estmemusage', $status['vsize'] - $this->initialStatus['vsize'] );
So either the params want passing in as a number, and then the dynamic units dropping (just hard code bytes like the other values)...
Looking at EditPage
foreach ( $output->getLimitReportData() as $key => $value ) { if ( Hooks::runner()->onParserLimitReportFormat( $key, $value, $limitReport, true, true ) ) { $keyMsg = wfMessage( $key ); $valueMsg = wfMessage( [ "$key-value-html", "$key-value" ] ); if ( !$valueMsg->exists() ) { // This is formatted raw, not as localized number. // If you want the parameter formatted as a number, // define the `$key-value` message. $valueMsg = ( new RawMessage( '$1' ) )->params( $value ); } else { // If you define the `$key-value` or `$key-value-html` // message then the argument *must* be numeric. $valueMsg = $valueMsg->numParams( $value ); } if ( !$keyMsg->isDisabled() && !$valueMsg->isDisabled() ) { $limitReport .= Html::openElement( 'tr' ) . Html::rawElement( 'th', null, $keyMsg->parse() ) . Html::rawElement( 'td', null, $valueMsg->parse() ) . Html::closeElement( 'tr' ); } } }
And Scribuntos en.json
"scribunto-limitreport-virtmemusage": "Lua virtual size", "scribunto-limitreport-virtmemusage-value": "$1/$2", "scribunto-limitreport-estmemusage": "Lua estimated memory usage", "scribunto-limitreport-estmemusage-value": "$1", "scribunto-limitreport-memusage": "Lua memory usage", "scribunto-limitreport-memusage-value": "$1/$2",
Change 635998 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/Scribunto@master] Format Scribunto memory usage only in bytes for the NewPP limit report
With the patch above..
Lua virtual size: 6152192/52428800 bytes Lua estimated memory usage: 0 bytes
Rather than
Lua virtual size 5.86 MB/50 MB Lua estimated memory usage 0 bytes
Considering the rest of the report uses bytes, I don't think this is an issue?
<!-- NewPP limit report Parsed by ubuntu64‐web‐esxi Cached time: 20201023110630 Cache expiry: 86400 Dynamic content: false Complications: [vary‐revision‐sha1] CPU time usage: 0.218 seconds Real time usage: 0.492 seconds Preprocessor visited node count: 1527/1000000 Post‐expand include size: 11392/2097152 bytes Template argument size: 188/2097152 bytes Highest expansion depth: 5/40 Expensive parser function count: 0/100 Unstrip recursion depth: 0/20 Unstrip post‐expand size: 1279/5000000 bytes Lua time usage: 0.030/7 seconds Lua virtual size: 6152192/52428800 bytes Lua estimated memory usage: 0 bytes -->
Which is only one of the usages, helpfully.
So the messages need changing for the NewPP report... Removing the formatSize calls is not helpful as it removes the pretty formatting, but also prints them as a raw number on that HTML output. Splitting my patch
No, I was right the first time.
Basically, the formatSize result in it being formatted twice. Once in Scribunto code, once in EditPage::getPreviewLimitReport() which causes the problem.
The hook does pass localise = true, so we want to keep the formatting...
Ok, PS4 now uses the hook handlers in Scribunto for onParserLimitReportFormat to do all the formatting, and then is just dealt with as "raw" in the output in EditPage
Keeps the possibility for localisation if needed... Keeps it as pretty formatted numbers. I dunno if that's actually wanted in prefixed bytes, but that's how it was done in b58ee1da94b9666092e01127a4d58ae01c4e5494...
Change 636005 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/extensions/Scribunto@master] Format Scribunto Lua Preview Limit Report memory numbers in bytes
So two options
Keep the output the same, formatting it with units (other code doesn't do this). Ignore this commit summary it's out of date
Or just simplify the code and output everything in byte(s) (with plural support!)
I don't know if/how much extra value having it output in bytes with prefixes (mega, kilo etc) is if the others aren't... It's definitely inconsistent and reduced hacks/workarounds in the code
Thanks for looking into this. Just to throw it out there, another option is to undefine the -value message, which will ensure the output is reported literally and not formatted. This would work for scribunto-limitreport-estmemusage-value which currently has the value $1. For scribunto-limitreport-virtmemusage-value and scribunto-limitreport-memusage-value you could do the $1/$2 formatting when those values are defined, and the undefine the i18n message to prevent the formatting from being applied twice.
This would look something like https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Scribunto/+/636046
EDIT: nevermind, I see this is very similar to what you did in 635998
Change 636046 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/extensions/Scribunto@master] Don't double-format memusage in limit report
Change 636046 abandoned by C. Scott Ananian:
[mediawiki/extensions/Scribunto@master] Don't double-format memusage in limit report
Reason:
Abandoned in favor of If8a71419d656530859552abaddeed66d5a9ddc4b
Change 636063 had a related patch set uploaded (by C. Scott Ananian; owner: C. Scott Ananian):
[mediawiki/core@master] Don't try to formatNum() non-numeric media metadata
Patch in 636063 for that, although it's kind of a big hammer approach. I suspect there is double-formatting going on here as well, and it would be nice to have a finer-grained approach.
Do we want to make this a blocker for wmf.16? (No train next week, but high-priority because deprecation spam isn't meant to exist in prod. ;-))
Change 636063 merged by jenkins-bot:
[mediawiki/core@master] Don't try to formatNum() non-numeric media metadata
Change 635998 merged by jenkins-bot:
[mediawiki/extensions/Scribunto@master] Fix double formatting of memory units
So imported https://upload.beta.wmflabs.org/wikipedia/commons/c/c0/A_solid_ghost_hunting_breakfast_2013-06-05_10-55.jpg
<small>/var/www/wiki/mediawiki/core/includes/media/FormatMetadata.php:1280:</small><small>string</small> <font color='#cc0000'>'����'</font> <i>(length=4)</i>
And if we use ctype_print it's "false".... But its value is \u0000\u0000\u0000\u0000
It's from a "tag" of FlashPixVersion. FlashpixVersion is handled, FlashPixVersion is not. It's case sensitive...
Exif.php has a different casing already (unhelpfully)
'FlashPixVersion' => self::UNDEFINED, # Supported Flashpix version #p32
From a look online, FlashpixVersion looks reasonably well used, but that doesn't mean FlashPixVersion is actually incorrect...
And it may just be a device in the wild implementing it incorrectly (and as such should be commented as such)
Change 636119 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] media: Handle FlashpixVersion in FormatMetadata::makeFormattedData()
Change 636120 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@master] [DNM] Add some more debugging for badly formatted numbers which probably come from unknown exif tags
Grabbing the PDF from https://en.wikipedia.beta.wmflabs.org/wiki/File:A_Basic_Guide_to_Open_Educational_Resources.pdf / https://upload.wikimedia.org/wikipedia/commons/b/b4/A_Basic_Guide_to_Open_Educational_Resources.pdf (or, well, actually just using it via instant commons)
There seems to be an exif header of pdf-Encrypted and a value of no.
This is added by MediaWiki-extensions-PdfHandler, but then MW borks because it's not numeric, and no real way to identify it's not supposed to be. Everything else seems to be standard and known about in MW... Or numeric
case 'Encrypted': // @todo: The value isn't i18n-ised. The appropriate // place to do that is in FormatMetadata.php // should add a hook a there. // For reference, if encrypted this fields value looks like: // "yes (print:yes copy:no change:no addNotes:no)" $items['pdf-Encrypted'] = $val;
Added in rEPHD7663952c3451: Display metadata from PDF files on image description page.
So how should we be dealing with these? adding a "hack" to MW core... Or implementing some sort of hook for the default statement of switch ( $tag ) in FormatMetadata::makeFormattedData() and allowing things to offer up ways to format these...? I wonder if a task was ever filed about that.
Funnily, looking at the comment above...
// These last two (version and encryption) I was unsure // if we should include in the table, since they aren't // all that useful to editors. I leaned on the side // of including. However not including if file // is optimized/linearized since that is really useless // to an editor. case 'PDF version': $items['pdf-Version'] = $val; break;
Potentially, we could just remove these and they'll go away from the exif output.
As per the question, are they actually useful?
I imagine "encrypted"-ness isn't much use on commons et al. If it's encrypted, we probably don't want it. On a private wiki? Maybe it's more useful. But of course, if it is encrypted, it's not going to thumb etc
Something like this would be potentially useful... Maybe rather than the deprecation here and in Language::commafy we should be disabling the deprecation again, and adding some better structured logging so we can play whack-a-mole for a while, and then when those logs are cleaner, re-enable the deprecation. At least in this case in FormatMetadata (more so than Language::commafy with it's many callers all over the place), we can easily add a parameter for referencing the exif tag (as done in the DNM patch atm; which also fudges the wfDeprecated text which is kinda nasty. Hence DNM). And as the function is private, easily remove it again a little down the way without causing any breaking changes
Or in the case of formatNum in FormatMetadata, maybe passing in non numbers is fine to this private method... Just returning them verbatim if they're not numerical and therefore can't be formatted.
Certainly, it's going to need something like the aformentioned hook to allow the extra stuff that MediaWiki-extensions-PdfHandler
adds in to be able to deal with formatting them. Or passing in like a Message object, and then not calling formatNum() with it passed (and we're in the switch ( $tag ) default condition if $val instanceof Message
Otherwise we're shooting fairly blind with these deprecation warnings, which in WMF prod, could be fairly prolific
Change 636219 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_35] media: Fix case of FlashPixVersion in FormatMetadata::makeFormattedData()
Change 636220 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/core@REL1_31] media: Fix case of FlashPixVersion in FormatMetadata::makeFormattedData()
Change 636220 merged by jenkins-bot:
[mediawiki/core@REL1_31] media: Fix case of FlashPixVersion in FormatMetadata::makeFormattedData()
Change 636219 merged by jenkins-bot:
[mediawiki/core@REL1_35] media: Fix case of FlashPixVersion in FormatMetadata::makeFormattedData()
Change 636119 merged by jenkins-bot:
[mediawiki/core@master] media: Fix case of FlashPixVersion in FormatMetadata::makeFormattedData()
It looks to me like there's already an endpoint which is *intended* to allow the particular *Handler class to do the proper formatting, since the comments all discuss how the main point of these methods is to turn opaque magic numbers into human-readable strings. But in any case https://gerrit.wikimedia.org/r/636063 separated out the media-related issue from the others; the wfDeprecated there could easily be downgraded to a debug log. I *think* the main commafy/formatNum path is actually fixed, though, so I'd rather not turn that deprecation warning off. We should probably split off a new task for the whack-a-mole based on media metadata, which should now be generating a deprecation warning in FormatMetadata::formatNum() rather than one in commafy.
None of the Language::formatNum() warnings seen since then, but we're still seeing FormatMetadata-related warnings now: https://logstash-beta.wmflabs.org/goto/5438534501b787a1d15cf41809637a35
Looks like PDF, which @Reedy investigated in T263592#6575833 but I don't think has been patched yet.
I'm going to fork off T266677: Use of FormatMetadata::formatNum with non-numeric value was deprecated in MediaWiki 1.36. [Called from FormatMetadata::makeFormattedData] for these media-related cases. @Jdforrester-WMF, that's the task which might be a production blocker, if any. We'll use this task for any other bad uses of Language::formatNum() which slip through, hopefully there aren't any and this bug can be closed after we confirm this doesn't show up in the logs again when the train rolls next week.
Found another, in CentralAuth: T267362.
Here's a logstash search to keep an eye out for any more: https://logstash.wikimedia.org/app/kibana#/discover/AXWZ1bBOXxHvZPC3bjjJ?_g=(refreshInterval%3A(display%3AOff%2Cpause%3A!f%2Cvalue%3A0)%2Ctime%3A(from%3Anow-24h%2Cmode%3Aquick%2Cto%3Anow))
Change 636005 merged by jenkins-bot:
[mediawiki/extensions/Scribunto@master] Format Scribunto Lua Preview Limit Report memory numbers in bytes
Change 636120 abandoned by Reedy:
[mediawiki/core@master] [DNM] Add some more debugging for badly formatted numbers which probably come from unknown exif tags
Reason: