Page MenuHomePhabricator

Replace usage of ParserOutputHook in Extension:NoCat with OutputPageParserOutput hook
Closed, ResolvedPublic

Description

Extension:NoCat attempts to use a $wgParserOutputHooks named nocat_fakecategories (NoCatHooks::noCatParserOutputHook).

However, this is currently being setup wrong (The config section does not merge into existing config variables. It only creates new ones) in extension.json so it has no affect. You can verify this by enabling $wgNoCatShowCat, and noting that suppressed category links do not show up in the footer like they should when that variable is enabled. Additionally the Parser output hooks system is considered somewhat old, and using the OutputPageParserOutput is generally considered better.

Your task is to replace the non-working ParserOutputHooks with a version that would do the same thing (if it worked), but uses the OutputPageParserOutput hook

Prerequisites:

  • You should have a basic understanding of PHP programming.
  • You should have already setup a copy of MediaWiki locally and understand how to enable extensions
  • It is recommended that you have successfully completed at least one mediawiki (or mediawiki extension) related PHP coding task, that involved submitting a patch to gerrit.

What to do:

  • First off, find the definition of the current hook callback NoCatHooks::noCatParserOutputHook in NoCatHooks.php. Find also where the hook is set via $pout->addOutputHook() in for the page in NoCatHooks::onParseAfterParse
  • The new interface doesn't support passing $data in the same way, so instead you should use ParserOutput::setExtensionData to store the old categorylinks
  • You should remove the call to ParserOutput::addOutputHook since we are no longer using an output hook
  • Rename NoCatHooks::noCatPArserOutputHook to something more appropriate for the other hook type (i.e. onOutputPageParserOutput). Change the method signature to not have the third $data argument as the new interface doesn't support it.
  • In our newly renamed function, instead of $data, we now use ParserOutput::getExtensionData() . Since unlike the old hook, this hook will run on all pages whether or not it is needed, we should be prepared to get a null from ->getExtensionData and do nothing in that case.
  • Remove the old definition for ParserOutputHooks in extension.json in the config section. Add a new entry for the newly renamed function in the hooks section as the hook handler for OuputPageParserOutput
  • Test out the extension. Set $wgNoCatShowCat = true; in LocalSettings.php and see if __NOCAT__ both prevents pages from being categorized but still shows the categories in the footer of the page.
  • Submit your patch to gerrit using an appropriate commit message

Event Timeline

[I didn't actually test, but when I copied into the other extension i was working on, the code didn't work]

Change 487364 had a related patch set uploaded (by Zoranzoki21; owner: Zoranzoki21):
[mediawiki/extensions/NoCat@master] Replaced no_catfakecategories hook to OutputPageParserOutput

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

Change 487364 had a related patch set uploaded (by Zoranzoki21; owner: Zoranzoki21):
[mediawiki/extensions/NoCat@master] Replaced no_catfakecategories hook to OutputPageParserOutput

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

Change 487364 abandoned by Zoranzoki21:
Replaced no_catfakecategories hook to OutputPageParserOutput

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

Guess i can mentor this for GCI

Bawolff renamed this task from Extension:NoCat isn't setting up ParserOutputHook properly to Replace usage of ParserOutputHook in Extension:NoCat with OutputPageParserOutput hook.Dec 9 2019, 3:08 PM
Bawolff updated the task description. (Show Details)

Change 556825 had a related patch set uploaded (by Pppery; owner: Pppery):
[mediawiki/extensions/NoCat@master] Fix $wgNoCatShowCat configuration setting

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

Change 556825 merged by jenkins-bot:
[mediawiki/extensions/NoCat@master] Fix $wgNoCatShowCat configuration setting

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