Page MenuHomePhabricator
Create Task
Maniphest T266182

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

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.

Related Objects
Search...

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
Tgr mentioned this in T257066: Extension:Score / Lilypond is disabled on all wikis.Oct 21 2020, 6:48 PM
Tgr added a parent task: T257066: Extension:Score / Lilypond is disabled on all wikis.
Jonesey95 added a comment.Oct 21 2020, 6:56 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.

Deacon_Vorbis added a subscriber: Deacon_Vorbis.Oct 21 2020, 7:27 PM
DannyS712 added a subscriber: DannyS712.Oct 21 2020, 8:36 PM
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
In T266182#6570028, @Reedy wrote:

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
In T266182#6570484, @Tgr wrote:
In T266182#6570028, @Reedy wrote:

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

Reedy updated the task description. (Show Details)Oct 23 2020, 2:16 AM
Reedy edited projects, added MediaWiki-Parser, Wikimedia-production-error, Math; removed Wikimedia-General-or-Unknown.
Reedy added a comment.Oct 23 2020, 11:14 AM

Seems to WFM locally

Krinkle removed a project: Wikimedia-production-error.Oct 23 2020, 7:36 PM
Krinkle updated the task description. (Show Details)
Jonesey95 added a comment.EditedOct 24 2020, 12:23 AM
In T266182#6574150, @Reedy wrote:

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?

Reedy added a comment.Oct 24 2020, 12:31 AM
In T266182#6575771, @Jonesey95 wrote:
In T266182#6574150, @Reedy wrote:

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
In T266182#6570484, @Tgr wrote:
In T266182#6570028, @Reedy wrote:

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
matmarex mentioned this in T272733: Review Math strip marker bugs (UNIQ / QINU) that might be fixed by 655884.Jan 22 2021, 7:05 PM
Physikerwelt added a subscriber: Physikerwelt.Sat, Feb 20, 10:57 AM

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

Physikerwelt moved this task from Watching to Blocked: needs help on the Math board.Sat, Feb 20, 10:57 AM
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.

Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL