Page MenuHomePhabricator

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

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 (c4d04b7) 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

Kghbln created this task.Jul 24 2019, 2:39 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 24 2019, 2:39 PM

Similar situation as T228595.

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.

Nikerabbit triaged this task as Normal priority.Mon, Aug 26, 2:13 PM

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.
Kghbln added a subscriber: Samwilson.EditedMon, Aug 26, 2:24 PM

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