TemplateStyles uses the standard $parser->fetchCurrentRevisionOfTitle( $title ) to get the Revision of the stylesheet page.

Apparently FlaggedRevs uses only the 'BeforeParserFetchTemplateAndtitle' hook to override the logic used by the default fetcher for $parser->fetchTemplateAndTitle(), rather than using ParserOptions::setCurrentRevisionCallback() (added in MW 1.24) and ParserOptions::setTemplateCallback() (added in MW 1.12!) to override the fetchers. It should be updated.

What would happen if multiple things tried to set the callback (e.g. FlaggedRevs and TemplateSandbox, or FlaggedRevs and EditPage)?

There's only the one callback. Usually code setting the callback would chain it something like

$oldCurrentRevisionCallback = $popt->setCurrentRevisionCallback(
    function ( $title, $parser = false ) use ( &$oldCurrentRevisionCallback ) {
        /* Do whatever specialness needs to happen for certain titles */

        // Nothing special, use the default handling
        return call_user_func( $oldCurrentRevisionCallback, $title, $parser );
    }
);

FlaggedRevs might want to do the opposite:

$oldCurrentRevisionCallback = $popt->setCurrentRevisionCallback(
    function ( $title, $parser = false ) use ( &$oldCurrentRevisionCallback ) {
        $rev = call_user_func( $oldCurrentRevisionCallback, $title, $parser );
        if ( $rev ) {
            // Check which revision was returned, and maybe replace it with a
            // stable revision of the same page (without reference to $title)
        }
        return $rev;
    }
);

BTW, FlaggedRevs probably has the same problem with Scribunto's mw.title:getContent(), and core parser functions such as {{PAGESIZE:Title}}, {{REVISIONID:Title}}, and so on.

I didn't have time to do it earlier. However, it looks like an easy fix…

In T190945#4088607, @Anomie wrote:

TemplateStyles uses the standard $parser->fetchCurrentRevisionOfTitle( $title ) to get the Revision of the stylesheet page.

Apparently FlaggedRevs uses only the 'BeforeParserFetchTemplateAndtitle' hook to override the logic used by the default fetcher for $parser->fetchTemplateAndTitle(), rather than using ParserOptions::setCurrentRevisionCallback() (added in MW 1.24) and ParserOptions::setTemplateCallback() (added in MW 1.12!) to override the fetchers. It should be updated.

Looks like we can easily replace 'BeforeParserFetchTemplateAndtitle' with setCurrentRevisionCallback(). They are called by the same function Parser::statelessFetchTemplate() (hook first, then the callback), and the only other users of getCurrentRevisionCallback() are CoreParserFunctions::getCachedRevisionObject() (used by functions like {{REVISIONID:Foo}}, so they will now use the flagged revision ID rather than the latest), Parser::getRevisionObject() (used by magic words like {{REVISIONID}}, likewise) and TemplateStyles (which should fix this task!).

It seems entirely superfluous to override setTemplateCallback(). The default handler actually calls the function Parser::statelessFetchTemplate() mentioned above, which runs the other hook/callback. I am not sure why would you ever override it at all these days, maybe it's more convenient if you want to return arbitrary text rather than coming from a revision, but getCurrentRevisionCallback() is already allowed to return a "fake" Revision object.

On the other hand though, why does TemplateStyles use $parser->fetchCurrentRevisionOfTitle( $title ) instead of $parser->fetchTemplate( $title )? Could it use that instead? It seems that some other extensions use the 'BeforeParserFetchTemplateAndtitle' hook (ApprovedRevs, Memento) and using the other function in TemplateStyles should fix compatibility with them both.

Change 441020 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/FlaggedRevs@master] Replace old and busted hook with the new hotness of a callback

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

In T190945#4299253, @matmarex wrote:

On the other hand though, why does TemplateStyles use $parser->fetchCurrentRevisionOfTitle( $title ) instead of $parser->fetchTemplate( $title )? Could it use that instead? It seems that some other extensions use the 'BeforeParserFetchTemplateAndtitle' hook (ApprovedRevs, Memento) and using the other function in TemplateStyles should fix compatibility with them both.

$parser->fetchTemplate( $title ) returns a string. TemplateStyles needs the Content object from the revision. In the future it might use the Content object from a non-main MCR slot of the revision.

TemplateStyles also uses the revision ID in warnings, the in-process cache key, and the key for deduplication.

Should we deprecate the BeforeParserFetchTemplateAndtitle hook if it's usage is incompatible with extensions like Scribunto/TemplateStyles?

Change 441020 merged by jenkins-bot:
[mediawiki/extensions/FlaggedRevs@master] Replace old and busted hook with the new hotness of a callback

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