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.

Event Timeline

Tgr created this task.Oct 21 2020, 6:48 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 21 2020, 6:48 PM

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.

Reedy added a subscriber: Reedy.Oct 21 2020, 9:46 PM

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

Tgr added a comment.Oct 22 2020, 2:43 AM

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.

Reedy updated the task description. (Show Details)Oct 23 2020, 1:41 AM

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

Seems to WFM locally

Krinkle updated the task description. (Show Details)
Jonesey95 added a comment.EditedOct 24 2020, 12:23 AM

Seems to WFM locally

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

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

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

Reedy added a comment.EditedOct 24 2020, 12:35 AM

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...

becomes

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

Reedy added a comment.Oct 24 2020, 1:22 AM
[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

Physikerwelt moved this task from Incoming to Watching on the Math board.Oct 27 2020, 6:11 PM
Izno added a subscriber: Izno.Dec 30 2020, 10:17 PM

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

matmarex closed this task as Resolved.Wed, Feb 24, 2:15 PM
matmarex added a subscriber: matmarex.

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.