search

SemanticMediaWiki / semantic-mediawiki.org

This is a meta repository allowing to keep track of issues and wishes concerning semantic-mediawiki.org and related websites.
3 stars 3 forks source link

"DisplayTitleHooks.php": Call to a member function isTalkPage() on null #63

Closed mwjames closed 4 years ago

mwjames commented 4 years ago

Setup

refs: https://sandbox.semantic-mediawiki.org/w/index.php?title=Sp%C3%A9cial:Requ%C3%AAter&#search

Issue

[ddc4653e9eff034859c5f6dc] /w/index.php?title=Sp%C3%A9cial:Requ%C3%AAter& Error from line 183 of /var/www/html/sbxsmw/w/extensions/DisplayTitle/includes/DisplayTitleHooks.php: Call to a member function isTalkPage() on null

Backtrace:

#0 /../w/includes/Hooks.php(174): DisplayTitleHooks::onParserBeforeStrip(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/formats/graphviz/SRF_Graph.php(136): Parser->recursiveTagParse(string)
#4 /../w/extensions/SemanticMediaWiki/src/Query/ResultPrinters/ResultPrinter.php(342): SRFGraph->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(SRFGraph, 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}
mwjames commented 4 years ago

@kghbln FYI

kghbln commented 4 years ago

FYI

Thanks for the note. Reported with task T228595.

kghbln commented 4 years ago

@mwjames I checked out the patch on sandbox. Could you please check if this fixes the issue? Admittedly I was not able to create the stacktrace form the link provided so I just trusted you on this one when reporting.

mwjames commented 4 years ago

@mwjames I checked out the patch on sandbox. Could you please check if this fixes the issue? Admittedly I was not able to create the stacktrace form the link provided so I just trusted you on this one when reporting.

You just need:

The DisplayTitle issue is gone but now you get:

[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}

The PageTranslationHooks hook has the same underlying issue which is that it isn't gated against a NULL title.

On 7/24/19, Karsten Hoffmeyer notifications@github.com wrote:

@mwjames I checked out the patch on sandbox. Could you please check if this fixes the issue? Admittedly I was not able to create the stacktrace form the link provided so I just trusted you on this one when reporting.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/SemanticMediaWiki/semantic-mediawiki.org/issues/63#issuecomment-514647703

kghbln commented 4 years ago

@mwjames Thanks for your swift reply. I will report the issue as fixed for DisplayTitle and will report an now one for Translate. Eventually paradise will be regained.

  • Condition [[Has number::+]] - Printout: Has number - Format: graph

Ah, and I thought just calling the page would create the issue.

kghbln commented 4 years ago

Reported with T228881.

cicalese commented 4 years ago

I ran the two issues by one of our engineers to try to get at the root cause. The response: "I'd look for a code path that is getting into those hooks without having triggered a call to Parser->startParse(). I see both bugs include a call from "extensions/SemanticResultFormats" to Parser->recursiveTagParse(string) downstream of SMW's "Special:Ask"; probably someone involved in one of those two components either forgot to call Parser->startExternalParse() or is incorrectly calling some code that is intended to be called only from parser hooks."

In each of the stack traces, I find a line of the form: $result = $GLOBALS['wgParser']->recursiveTagParse( "<graphviz>$graphInput</graphviz>" ); in SemanticResultFormats (at SemanticResultFormats/formats/graphviz/SRF_Graph.php" (136) and "SemanticResultFormats/src/Graph/GraphPrinter.php" (152)). Perhaps adding a call to $GLOBALS['wgParser']->startExternalParse(...) before those lines would help?

kghbln commented 4 years ago

@cicalese Thanks for digging into this and notifying us here. Lucky us the "graph" format is currently being refactored (https://github.com/SemanticMediaWiki/SemanticResultFormats/issues/503), this I will notify folks of your findings to add this further improvement for the future.

mwjames commented 4 years ago

$result = $GLOBALS['wgParser']->recursiveTagParse( "$graphInput" );

This was introduced by [0] in Jan 2018 as replacement for GraphViz::graphvizParserHook( $graphInput, "", $GLOBALS['wgParser'], "" ).

[0] https://github.com/SemanticMediaWiki/SemanticResultFormats/pull/365

Parser->recursiveTagParse(string) downstream of SMW's SpecialAsk; probably someone involved in one of those two components either forgot to call Parser->startExternalParse() or is incorrectly calling some code that is intended to be called only from parser hooks."

I have to say that I'm a bit surprised by this suggestion (meaning to use startExternalParse prior) because I never heard of this before that such call is required in connection with recursiveTagParse and the documentation [1] doesn't hint any of this as being a hard requirement to the call of recursiveTagParse.

My interpretation of [1] was that when calling Parser::recursiveTagParse (or for that matter recursiveTagParseFully) I get a HTML output (safe or unsafe) without being required to create a Title instance, handle a ParserOptions instance or think about some $outputType.

Any extension hooking into the Parser is likely to encounter a situation where a Title instance is null and as such one should gate against those cases because it is unlikely that all users of recursiveTagParse will have heard of calling startExternalParse prior the mentioned recursiveTagParse or recursiveTagParseFully.

If you think a Title instance needs to be present while calling recursiveTagParse then the Parser should add an explicit check and extend the documentation [1] for developers that are unfamiliar with the Parser instance invocation or its object dependencies.

[1] https://github.com/wikimedia/mediawiki/blob/9aeea7989766f2fec8920510ea8815aa403f9edb/includes/parser/Parser.php#L733-L757

On 7/25/19, cicalese notifications@github.com wrote:

I ran the two issues by one of our engineers to try to get at the root cause. The response: "I'd look for a code path that is getting into those hooks without having triggered a call to Parser->startParse(). I see both bugs include a call from extensions/SemanticResultFormats to Parser->recursiveTagParse(string) downstream of SMW's SpecialAsk; probably someone involved in one of those two components either forgot to call Parser->startExternalParse() or is incorrectly calling some code that is intended to be called only from parser hooks."

In each of the stack traces, I find a line of the form:

$result = $GLOBALS['wgParser']->recursiveTagParse( "$graphInput" );

in SemanticResultFormats (at SemanticResultFormats/formats/graphviz/SRF_Graph.php(136) and SemanticResultFormats/src/Graph/GraphPrinter.php(152)). Perhaps adding a call to $GLOBALS['wgParser']->startExternalParse(...) before those lines would help?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/SemanticMediaWiki/semantic-mediawiki.org/issues/63#issuecomment-514792112

cicalese commented 4 years ago

Agreed that if there is a dependency upon Parser->startExternalParse(), it should be documented. It would be good to know if adding that does indeed help your situation or whether there is some other root cause. If it does help, we could propose a change to the documentation to indicate that. Otherwise, we would still need to look further for a root cause. I initially was wondering whether the fact that the code is invoked from a special page when the problem manifests itself is material, but perhaps that is not related.

mwjames commented 4 years ago

Otherwise, we would still need to look further for a root cause. I initially was wondering whether the fact that the code is invoked from a special page when the problem manifests itself is material, but perhaps that is not related.

From my quick inspection of [0], I think this assumption is hitting a mark given those embedded in a wiki page do display correctly without an exception meaning the use of Parser::recursiveTagParse is inconsistent when it comes to the context of its execution.

[0] https://sandbox.semantic-mediawiki.org/w/index.php?title=Attribut%3AQuery+format&limit=20&offset=0&filter=graph

On 7/26/19, cicalese notifications@github.com wrote:

Agreed that if there is a dependency upon Parser->startExternalParse(), it should be documented. It would be good to know if adding that does indeed help your situation or whether there is some other root cause. If it does help, we could propose a change to the documentation to indicate that. Otherwise, we would still need to look further for a root cause. I initially was wondering whether the fact that the code is invoked from a special page when the problem manifests itself is material, but perhaps that is not related.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/SemanticMediaWiki/semantic-mediawiki.org/issues/63#issuecomment-515218909

SbstnS commented 4 years ago

@cicalese As I'm working on the graph format I've tried to add the $GLOBALS['wgParser']->startExternalParse(...) before calling the $GLOBALS['wgParser']->recursiveTagParse() [0] as you recommended but now it won't render the parser function anymore instead it prints out the raw syntax of graphViz.

//GraphPrinter.php [0]

$GLOBALS['wgParser']->startExternalParse(null, ParserOptions::newFromUser( $GLOBALS['wgUser'] ), Parser::OT_WIKI );

// Calls graphvizParserHook function from MediaWiki GraphViz extension
$result = $GLOBALS['wgParser']->recursiveTagParse( "<graphviz>$graphInput</graphviz>" );`

With this approach I also get an error when calling the graph format under the "Special:Ask" page.

cicalese commented 4 years ago

@RacoonDev I'm wondering if the reason it is not rendering the parser function is that you are passing in Parser::OT_WIKI rather than Parser::OT_HTML.

Is the error you are getting related to the title being null? Did you try passing in the Title object rather than null for the first parameter?

cicalese commented 4 years ago

@mwjames

From my quick inspection of [0], I think this assumption is hitting a mark given those embedded in a wiki page do display correctly without an exception meaning the use of Parser::recursiveTagParse is inconsistent when it comes to the context of its execution.

Agreed. Perhaps there is something different in the way the Parser object is constructed for special pages.

kghbln commented 4 years ago

The fix probably caused https://github.com/SemanticMediaWiki/SemanticMediaWiki/issues/4330

mwjames commented 4 years ago

The fix probably caused https://github.com/SemanticMediaWiki/SemanticMediaWiki/issues/4330

The issue here is different (I will comment there) but it scratches the same operational surface.

On 10/16/19, Karsten Hoffmeyer notifications@github.com wrote:

The fix probably caused https://github.com/SemanticMediaWiki/SemanticMediaWiki/issues/4330

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/SemanticMediaWiki/semantic-mediawiki.org/issues/63#issuecomment-542607058

kghbln commented 4 years ago

Fixed last year with https://gerrit.wikimedia.org/r/525191