Page MenuHomePhabricator

Score (in its current disabled state) breaks unstripping for other extensions
Closed, ResolvedPublic

Description

As reported by @Jonesey95 in T257066#6568853, MediaWiki-extensions-Score in its current mostly-disabled state (see T257066: Extension:Score / Lilypond is disabled on all wikis) seems to break the parsing of unrelated extension tags following the <score> tag. Compare:

with the only difference being that the score tag is commented out in the latter. In the non-commented-out version, <math> tags following the <score> are replaced with a strip marker, instead of the proper math formula rendering.

Screenshot 2020-10-23 at 03.15.30.png (976×2 px, 344 KB)

Event Timeline

Here's a minimal test case:

https://en.wikipedia.org/w/index.php?title=User:Jonesey95/sandbox&oldid=984726275

If the score section is before the math markup, the math is rendered fine. If the score is after the math markup, the math markup is replace with stripmarkers.

Presumably it's the ScoreDisabledException being thrown during the parsing?

Presumably it's the ScoreDisabledException being thrown during the parsing?

Why, though? The parser hook should catch it. And if it didn't, that would probably result in an uglier error.

Presumably it's the ScoreDisabledException being thrown during the parsing?

Why, though? The parser hook should catch it. And if it didn't, that would probably result in an uglier error.

It's implemented using exceptions, it doesn't return anything different... So it feels like the obvious different change

rESCR7974f82e43ae: Nicer handling for disabling of shell execution

Parser.php contains very few catches

			if ( isset( $this->mTagHooks[$name] ) ) {
				$output = call_user_func_array( $this->mTagHooks[$name],
					[ $content, $attributes, $this, $frame ] );
			} else {
				$output = '<span class="error">Invalid tag extension name: ' .
					htmlspecialchars( $name ) . '</span>';
			}

It definitely sounds like something is killing (or corrupting) the execution, so it's not finishing replacing the strip markers back in again

I've not got Math setup on my dev wiki atm... but Score disabled (as is) doesn't cause the same problem

Screenshot 2020-10-23 at 02.39.25.png (434×2 px, 206 KB)

Seems to WFM locally

Screenshot 2020-10-23 at 12.14.01.png (624×2 px, 228 KB)

It looks like you have the math markup after the score, which works fine. Have you tried copying my minimal example above, or putting the math markup before the score, as I recommended above?

Seems to WFM locally

Screenshot 2020-10-23 at 12.14.01.png (624×2 px, 228 KB)

It looks like you have the math markup after the score, which works fine. Have you tried copying my minimal example above, or putting the math markup after the score, as I recommended above?

I literally used https://en.wikipedia.org/w/index.php?title=Circle_of_fifths&oldid=981481037#Chromatic_circle which didn't work on enwiki, as per the screenshot below too

Screenshot 2020-10-24 at 01.29.24.png (1×1 px, 274 KB)

But yes, I can reproduce it with your minimal case locally though...

Presumably it's the ScoreDisabledException being thrown during the parsing?

Why, though? The parser hook should catch it. And if it didn't, that would probably result in an uglier error.

diff --git a/includes/Score.php b/includes/Score.php
index b368138..e888af4 100644
--- a/includes/Score.php
+++ b/includes/Score.php
@@ -163,6 +163,7 @@ class Score {
                global $wgScoreDisableExec;
 
                if ( $wgScoreDisableExec ) {
+                       return '';
                        throw new ScoreDisabledException( wfMessage( 'score-exec-disabled' ) );
                }
 
@@ -641,6 +642,7 @@ class Score {
                        $wgScoreGhostscript;
 
                if ( $wgScoreDisableExec ) {
+                       return [];
                        throw new ScoreDisabledException( wfMessage( 'score-exec-disabled' ) );
                }
 
@@ -993,6 +995,7 @@ LILYPOND;
                global $wgScoreTimidity;
 
                if ( $wgScoreDisableExec ) {
+                       return;
                        throw new ScoreDisabledException( wfMessage( 'score-exec-disabled' ) );
                }
 
@@ -1135,6 +1138,7 @@ LILYPOND;
                global $wgScoreAbc2Ly, $wgScoreDisableExec;
 
                if ( $wgScoreDisableExec ) {
+                       return;
                        throw new ScoreDisabledException( wfMessage( 'score-exec-disabled' ) );
                }

Stop the exceptions being thrown, but abort execution in other ways...

Screenshot 2020-10-24 at 01.34.08.png (220×936 px, 31 KB)

becomes

Screenshot 2020-10-24 at 01.34.04.png (216×702 px, 15 KB)

So the exceptions are maybe not being caught/handled as they're seemingly being expected to? I guess the question is where the ScoreDisabledException is being caught...

Either way, it's definitely related to the exception

[Math] New cache key: canonical!math=5
[Math] Start rendering ${\displaystyle  \mathbb{Z}/12\mathbb{Z} }$ in mode mathml
[Math] Re-Rendering on user request
[http] POST: https://en.wikipedia.org/api/rest_v1/media/math/check/tex
[http] GET: https://en.wikipedia.org/api/rest_v1/media/math/render/mml/9707346ee2c503fe166020c1395bd53cc706ff70
[DBQuery] MathRenderer::readFromDatabase [0.001s] db:3306: SELECT  math_inputhash,math_mathml,math_tex,math_svg,math_input  FROM `mw_mathoid`    WHERE math_inputhash = '�>
[Math] Rendering successful. Writing output
[Math] Writing of cache requested.
[Math] Nothing was changed. Don't write to database.
[Math] New cache key: canonical!math=5
[ParserCache] Saved in parser cache

and with the disabled exception being thrown...

[Math] Start rendering ${\displaystyle  \mathbb{Z}/12\mathbb{Z} }$ in mode mathml
[GlobalTitleFail] MessageCache::parse called with no title set.
#0 /var/www/wiki/mediawiki/core/includes/language/Message.php(1245): MessageCache->parse()
#1 /var/www/wiki/mediawiki/core/includes/language/Message.php(876): Message->parseText()
#2 /var/www/wiki/mediawiki/core/includes/language/Message.php(929): Message->toString()
#3 /var/www/wiki/mediawiki/extensions/Score/includes/ScoreException.php(37): Message->parse()
#4 /var/www/wiki/mediawiki/extensions/Score/includes/Score.php(644): ScoreException->__construct()
#5 /var/www/wiki/mediawiki/extensions/Score/includes/Score.php(498): Score::generatePngAndMidi()
#6 /var/www/wiki/mediawiki/extensions/Score/includes/Score.php(388): Score::generateHTML()
#7 /var/www/wiki/mediawiki/extensions/Score/includes/Score.php(248): Score::renderScore()
#8 [internal function]: Score::render()
#9 /var/www/wiki/mediawiki/core/includes/parser/Parser.php(3925): call_user_func_array()
#10 /var/www/wiki/mediawiki/core/includes/parser/PPFrame_Hash.php(330): Parser->extensionSubstitution()
#11 /var/www/wiki/mediawiki/core/includes/parser/Parser.php(2874): PPFrame_Hash->expand()
#12 /var/www/wiki/mediawiki/core/includes/parser/Parser.php(1547): Parser->replaceVariables()
#13 /var/www/wiki/mediawiki/core/includes/parser/Parser.php(646): Parser->internalParse()
#14 /var/www/wiki/mediawiki/core/includes/content/WikitextContent.php(375): Parser->parse()
#15 /var/www/wiki/mediawiki/core/includes/content/AbstractContent.php(590): WikitextContent->fillParserOutput()
#16 /var/www/wiki/mediawiki/core/includes/Revision/RenderedRevision.php(263): AbstractContent->getParserOutput()
#17 /var/www/wiki/mediawiki/core/includes/Revision/RenderedRevision.php(235): MediaWiki\Revision\RenderedRevision->getSlotParserOutputUncached()
#18 /var/www/wiki/mediawiki/core/includes/Revision/RevisionRenderer.php(215): MediaWiki\Revision\RenderedRevision->getSlotParserOutput()
#19 /var/www/wiki/mediawiki/core/includes/Revision/RevisionRenderer.php(152): MediaWiki\Revision\RevisionRenderer->combineSlotOutput()
#20 [internal function]: MediaWiki\Revision\RevisionRenderer->MediaWiki\Revision\{closure}()
#21 /var/www/wiki/mediawiki/core/includes/Revision/RenderedRevision.php(197): call_user_func()
#22 /var/www/wiki/mediawiki/core/includes/poolcounter/PoolWorkArticleView.php(215): MediaWiki\Revision\RenderedRevision->getRevisionParserOutput()
#23 /var/www/wiki/mediawiki/core/includes/poolcounter/PoolCounterWork.php(162): PoolWorkArticleView->doWork()
#24 /var/www/wiki/mediawiki/core/includes/page/Article.php(814): PoolCounterWork->execute()
#25 /var/www/wiki/mediawiki/core/includes/actions/ViewAction.php(74): Article->view()
#26 /var/www/wiki/mediawiki/core/includes/MediaWiki.php(530): ViewAction->show()
#27 /var/www/wiki/mediawiki/core/includes/MediaWiki.php(316): MediaWiki->performAction()
#28 /var/www/wiki/mediawiki/core/includes/MediaWiki.php(943): MediaWiki->performRequest()
#29 /var/www/wiki/mediawiki/core/includes/MediaWiki.php(546): MediaWiki->main()
#30 /var/www/wiki/mediawiki/core/index.php(53): MediaWiki->run()
#31 /var/www/wiki/mediawiki/core/index.php(46): wfIndexMain()
#32 {main}
[Math] Re-Rendering on user request
[http] POST: https://en.wikipedia.org/api/rest_v1/media/math/check/tex
[http] GET: https://en.wikipedia.org/api/rest_v1/media/math/render/mml/9707346ee2c503fe166020c1395bd53cc706ff70
[DBQuery] MathRenderer::readFromDatabase [0.001s] db:3306: SELECT  math_inputhash,math_mathml,math_tex,math_svg,math_input  FROM `mw_mathoid`    WHERE math_inputhash = '�>
[Math] Rendering successful. Writing output
[Math] Writing of cache requested.
[Math] Nothing was changed. Don't write to database.
[Math] New cache key: canonical!math=5
[ParserCache] Saved in parser cache

I guess MessageCache::parse called with no title set isn't great (more log noise), but isn't the actual issue (even if it's making a new exception for the stack). Logging added as part of T159284: Deprecate $wgTitle completely

Is this fixed now? At least I can see the formulae for both versions linked in the ticket.

matmarex subscribed.

Seems to have been fixed by https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Math/+/655884. I can't see the problem in the linked revisions any more.