Page MenuHomePhabricator

"PageTranslationHooks.php": Call to a member function getIsSectionPreview() on null
Open, MediumPublic

Description

Setup

  • MediaWiki 1.33.0 (1c47e93) 15:49, 16. Jul. 2019
  • PHP 7.2.19-0ubuntu0.18.04.1 (apache2handler)
  • MariaDB 10.1.40-MariaDB-0ubuntu0.18.04.1
  • Translate 2019-07-22 MLEB 2019.07 (rETRAc4d04b7874bb) 08:41, 24. Jul. 2019

Issue
Originally reported here.

[9889623253b9e362dfc78301] /w/index.php?title=Sp%C3%A9cial:Requ%C3%AAter& Error from line 52 of /../w/extensions/Translate/tag/PageTranslationHooks.php: Call to a member function getIsSectionPreview() on null

Backtrace

#0 /../w/includes/Hooks.php(174): PageTranslationHooks::renderTagPage(Parser, string, NULL)
#1 /../w/includes/Hooks.php(202):Hooks::callHook(string, array, array, NULL)
#2 /../w/includes/parser/Parser.php(695): Hooks::run(string, array)
#3 /../w/extensions/SemanticResultFormats/src/Graph/GraphPrinter.php(152): Parser->recursiveTagParse(string)
#4 /../w/extensions/SemanticMediaWiki/src/Query/ResultPrinters/ResultPrinter.php(342): SRF\Graph\GraphPrinter->getResultText(SMW\Query\QueryResult, integer)
#5 /../w/extensions/SemanticMediaWiki/src/Query/ResultPrinters/ResultPrinter.php(307): SMW\Query\ResultPrinters\ResultPrinter->buildResult(SMW\Query\QueryResult)
#6 /../w/extensions/SemanticMediaWiki/src/MediaWiki/Specials/SpecialAsk.php(481): SMW\Query\ResultPrinters\ResultPrinter->getResult(SMW\Query\QueryResult, array, integer)
#7 /../w/extensions/SemanticMediaWiki/src/MediaWiki/Specials/SpecialAsk.php(316): SMW\MediaWiki\Specials\SpecialAsk->fetchResults(SRF\Graph\GraphPrinter, SMWQuery, SMW\MediaWiki\Specials\Ask\UrlArgs)
#8 /../w/extensions/SemanticMediaWiki/src/MediaWiki/Specials/SpecialAsk.php(169): SMW\MediaWiki\Specials\SpecialAsk->makeHTMLResult()
#9 /../w/includes/specialpage/SpecialPage.php(569): SMW\MediaWiki\Specials\SpecialAsk->execute(NULL)
#10 /../w/includes/specialpage/SpecialPageFactory.php(558): SpecialPage->run(NULL)
#11 /../w/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#12 /../w/includes/MediaWiki.php(865): MediaWiki->performRequest()
#13 /../w/includes/MediaWiki.php(515): MediaWiki->main()
#14 /../w/index.php(42): MediaWiki->run()
#15 {main}

Event Timeline

https://gerrit.wikimedia.org/g/mediawiki/extensions/Translate/+/b3cdb6f7059cf102852dd096841e42230c65463f/tag/PageTranslationHooks.php#38

Parser::getTitle() is documented to return Title|null, and the full file is sprinkled with is_null checks (but majority of cases assume mTitle is not null. I'd say the parser should be updated to require a valid title without everything having to check whether it has one or not.

If (and only if) SMW is creating the parser object, it could play nice by calling setTitle( null ) on it.

Parser::getTitle() is documented to return Title|null

I just came across rMWc251368ff7e1: Add null to Parser::getTitle return doc which I disagree with.

I'm planning to leave this task unattended unless one of the following happens:

  • The change is properly announced as a breaking change
  • Someone does the work in Translate by submitting a patch in Gerrit
  • This is fixed in the code who creates Parser without a Title
  • This is fixed in Parser to not give out null titles
  • Someone convinces me I should fix this anyway in Translate.

Thanks for digging into this as well as the insight on it. I am not sure if @Samwilson who authored the change is aware. Thus copying him in.

I'm a bit confused here: Parser::getTitle() has for a long time been able to return null; the recent change was just to document the fact. That change hasn't introduced any new behaviour, it didn't change any code at all.

Is the idea that it should be fixed to not return null, and be able to be relied on to always provide a Title? Maybe that makes sense, but I'm not sure of the argument in favour of that. Documenting the status quo seemed like a safe thing to do.

The fact that there's code in extensions that assumes a Title object is why I suggested the change, because it'll help people find bugs like this one (which already existed).

I'm a bit confused here: Parser::getTitle() has for a long time been able to return null

But that doesn't seem to happen with MediaWiki core... only if some other extensions users the parser in unexpected ways.

The fact that there's code in extensions that assumes a Title object is why I suggested the change, because it'll help people find bugs like this one (which already existed).

Only if announced, preferably also alerting the developers of affected code directly.... but this one is hard to grep for.

Change 532558 had a related patch set uploaded (by Nikerabbit; owner: Nikerabbit):
[mediawiki/core@master] Use a dummy title in Parser if none is provided

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

Change 533559 had a related patch set uploaded (by Fomafix; owner: Fomafix):
[mediawiki/core@master] Parser: Move dummy title from setter to getter

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

Change 532558 abandoned by Nikerabbit:
Use a dummy title in Parser if none is provided

Reason:
Not much left after rebase, and https://gerrit.wikimedia.org/r/c/mediawiki/core/ /533559 seems to handle this better.

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

@Kghbln Could you check if this error is currently gone (before the changes to Parser are reverted)?

Cannot tell exactly: I am now getting issue 4330. Perhaps this hides the issue reported here or it fixes it and introduces this new error. Note that the wiki is now running MW 1.34.0-rc.0 :(

Change 533559 abandoned by Fomafix:
Parser: Move dummy title from setter to getter

Reason:
Not needed anymore.

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