This is really two or three separate problems in one, but they all relate to the same hook, so I figured I'd make one bug report for all of them.
The ArticleRevisionViewCustom hook was added in MW 1.32:
It is meant to replace two existing hooks, ArticleContentViewCustom and ArticleAfterFetchContentObject, both of which were deprecated when that hook was added:
Here's the commit where it all happened, by the way:
I have two extensions that use ArticleAfterFetchContentObject to modify the text displayed for particular pages: Approved Revs and an upcoming one, tentatively titled Diagrams. (These might be the only two extensions right now that use any of these three hooks, by the way, as far as I know.)
I want to replace the use of the deprecated ArticleAfterFetchContentObject with ArticleRevisionViewCustom in my extensions. However, there are some problems that prevent me from doing this. The first is that this hook is called in two different places in the code, with a different set of arguments in each:
I believe that just adding $this->mOldid as a new third argument in the DifferenceEngine.php call will fix this problem.
The potentially more difficult problem is that it doesn't seem like ArticleRevisionViewCustom really allows for modifying the page display - in either the article or the diff page. In the article display (Article.php), this hook seems to be called *before* the body text variable (OutputPage::$mBodytext) is populated - which means that you can prepend text (ironically, both $output->prependHTML() and $output->addHTML() do this), but you can't append text, and you can't modify the original text. If this hook were called after $mBodytext is generated, I believe all three of these could be done.
In DifferenceEngine.php, the hook is called after the main diff display is generated, but before the "Revision as of ..." part is added - and, I would think, if anything, that the hook should be modifying that second part. (No?)
So, I think this hook needs to be fixed. Of course, a simpler solution, at least for now, is just to de-deprecate ArticleAfterFetchContentObject... but maybe there's some good reason not to do that.