-
-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
"DisplayTitleHooks.php": Call to a member function isTalkPage() on null #63
Comments
@kghbln FYI |
Thanks for the note. Reported with task T228595. |
@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 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:
- Condition `[[Has number::+]]`
- Printout: `Has number`
- Format: `graph`
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 ***@***.***> 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:
#63 (comment)
|
@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.
Ah, and I thought just calling the page would create the issue. |
Reported with T228881. |
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 In each of the stack traces, I find a line of the form: |
@cicalese Thanks for digging into this and notifying us here. Lucky us the "graph" format is currently being refactored (SemanticMediaWiki/SemanticResultFormats#503), this I will notify folks of your findings to add this further improvement for the future. |
$result = $GLOBALS['wgParser']->recursiveTagParse(
"<graphviz>$graphInput</graphviz>" );
This was introduced by [0] in Jan 2018 as replacement for
`GraphViz::graphvizParserHook( $graphInput, "", $GLOBALS['wgParser'],
"" )`.
[0] SemanticMediaWiki/SemanticResultFormats#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 ***@***.***> 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(
"<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?
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#63 (comment)
|
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. |
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 ***@***.***> 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:
#63 (comment)
|
@cicalese As I'm working on the graph format I've tried to add the //GraphPrinter.php [0]
With this approach I also get an error when calling the graph format under the "Special:Ask" page. |
@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? |
Agreed. Perhaps there is something different in the way the Parser object is constructed for special pages. |
The fix probably caused SemanticMediaWiki/SemanticMediaWiki#4330 |
The fix probably caused
SemanticMediaWiki/SemanticMediaWiki#4330
The issue here is different (I will comment there) but it scratches
the same operational surface.
…On 10/16/19, Karsten Hoffmeyer ***@***.***> wrote:
The fix probably caused
SemanticMediaWiki/SemanticMediaWiki#4330
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#63 (comment)
|
Fixed last year with https://gerrit.wikimedia.org/r/525191 |
Setup
refs: https://sandbox.semantic-mediawiki.org/w/index.php?title=Sp%C3%A9cial:Requ%C3%AAter&#search
Issue
Backtrace:
The text was updated successfully, but these errors were encountered: