Page MenuHomePhabricator

Retire TitleExists hook
Open, MediumPublic

Description

TitleExists allows extensions to override a title existence check, ie. tell the caller that the page is in the database, even though it isn't. This is an obviously bad idea without detailed rules of when and how this is allowed to be done (such rules are entirely absent), and leads to issues like T265665 or T119827. The hook was introduced in T72495: TemplateSandbox doesn't handle things like #ifexist and Scribunto's getContent as a hack for pretending during edit preview the that page that's just about to be created exists (so if you e.g. create a page with {{#ifexist:{{FULLPAGENAME}}|yes|no}}, it would behave identically during preview and after creation); that is a valid use case but should be exposed in a more narrow way.

Currently used in one place in core, ParserOptions::setupFakeRevision() (which could use a dedicated mechanism shared only with Title instead of a hook), and one extension known to CodeSearch, TemplateSandbox, which uses it for previews. So probably the way to go would be to add ParserOptions::setTitleExistsCallback(), along the lines of ParserOptions::setCurrentRevisionRecordCallback(), and if possible move the code calling the callback from Title::exists() to whatever calls that method within the parser.

Event Timeline

ParserOptions::setupFakeRevision should not exist at all. The parser should have a lookup mechanism for template content injected, which just returns wikitext.

It's not necessarily wikitext, could be e.g. TemplateStyles CSS. In the MCR future it could be also something with multiple types of contents. Injecting fake revisions seems like the right approach to me. (You really only need RevisionSlots objects, not revisions, but that's a more obscure class so probably better to use the more standard one.)

Having fake RevisionRecord imho is not great - we are trying to all more and more invariants to it. Like we want RevisionRecord::getPage to return a PageIdentity, meaning that RevisionRecord can only exist for pages that potentially can exist. Doing that for example will break things like parsing in the context of the special page, which we do.

With PHP 7.4 we can introduce an interface like RevisionInfo with more relaxed type guarantees and strengthen the guarantees on RevisionRecord with type covariance. Then we can make Parser depend on RevisionInfo and allow faking RevisionInfo. That will make RevisionRecord represent an actual database record.

The TitleExists hook in TemplateSandbox is currently non-functional, the feature is broken. The template being previewed is already recorded as missing in LinkCache when parsing starts, and that takes priority over Title::exists().