Page MenuHomePhabricator

Re-evaluate Message 'interface' and 'title' flags for messages bundled via load.php, remove "Dwimmerlaik"
Closed, ResolvedPublic

Description

Follows-up T291590: DiscussionTools markup appears in dropdowns on Special:MediaSearch:

  • We call wfMessage()->inLanguage() in ResourceLoaderContext, which core responds to by turning off Message::$interface. It's unclear why this exists in core in the first place, but at least for RL I think we can skip that and restore it with setInterfaceMessageFlag().

The call to inLanguage() is here only because on static/cookieless endpoints like load.php we get the user language from a different source than for index.php requests. In the transition to NO_SESSION we plugged that by calling inLanguage() but another way could have been to feed RequestContext::getMain() somehow the right language to use for load.php, in which case we wouldn't have started calling this method and still had the interface flag.

Having said that, it is a mystery to me why this is turned off by Message::inLanguage() in the first place, so we should look at why that was, and more importantly look at the current state of the world in terms of when something checks Message::$interface and how that affects MessageCache and Parser today. Once we understand that, we'll know if it is actually sensible to restore this in RL or to do it everywhere.

  • We set wfMessage()->title() with the famous "Dwimmerlaik" placeholder (codesearch). The placeholder is needed because RL primarily operates in a statically-cached context and not setting it to something will produce a warning, but also if we don't set it and we call RL outside its static context (e.g. embedded preview/private modules) would wronlgy expose those to the title of the current page view and potentially poison a shared cache.

This should be updated to the newer pattern used in other parts of core where the placeholder is in NS_SPECIAL instead of NS_MAIN, and uses "Badtitle" with an explaination rather than a cryptic Tolkien reference.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

[…]

ResourceLoaderContext's msg() function uses inLanguage():
```lang=php, name=[mediawiki/core]/includes/resourceloader/ResourceLoaderContext.php
	public function msg( $key, ...$params ): Message {
		return wfMessage( $key, ...$params )
			->inLanguage( $this->getLanguage() )
			// Use a dummy title because there is no real title
			// for this endpoint, and the cache won't vary on it
			// anyways.
			->page( PageReferenceValue::localReference( NS_MAIN, 'Dwimmerlaik' ) );
	}

And inLanguage() turns off the interface flag!

[mediawiki/core]/includes/language/Message.php
	/**
	 * Request the message in any language that is supported.
	 *
	 * As a side effect interface message status is unconditionally
	 * turned off.
...
	 */
	public function inLanguage( $lang ) {
...
		$this->interface = false;
...
	}

This behavior was added in 2010 in commit rMWe5941506f9c3: Tidy up the class, which doesn't explain the reason why, other than asserting that it is "better handling for languages".

We phased out Dwimmerlaik in the API a few years ago in favor of Special:Badtitle/dummy title for API calls set in api.php, it's probably time for ResourceLoader to do the same. As a side-effect this would probably fix this too since no one is adding NS_SPECIAL as an extra signature namespace?

Krinkle moved this task from Inbox to Accepted Enhancement on the MediaWiki-ResourceLoader board.
Krinkle moved this task from Inbox to Backlog: Maintenance on the Performance-Team board.
matmarex renamed this task from Re-evaluate Message 'interface' and 'title' flags for messages bundled via load.php to Re-evaluate Message 'interface' and 'title' flags for messages bundled via load.php, remove "Dwimmerlaik".Sep 23 2021, 12:45 PM

Having said that, it is a mystery to me why this is turned off by Message::inLanguage() in the first place, so we should look at why that was, and more importantly look at the current state of the world in terms of when something checks Message::$interface and how that affects MessageCache and Parser today. Once we understand that, we'll know if it is actually sensible to restore this in RL or to do it everywhere.

I did some archaeology. It appears that this behavior was added when the Parser could only parse wikitext in content language or interface language, and this flag simply controlled the target language. But since 2008 (92363755ce) you can set any target language, so clearing the flag in inLanguage() / inContentLanguage() is not necessary. There is still some vestigial code in Parser that uses the flag that way if the target language is not set (see Parser::getTargetLanguage()), but the Message class doesn't rely on that.

The interface flag has some other effects, but I don't think anything relies on them.

Change 727630 had a related patch set uploaded (by Bartosz Dziewoński; author: Bartosz Dziewoński):

[mediawiki/core@master] ResourceLoaderContext: Set message 'interface' flag and unique 'title'

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

Change 727630 merged by jenkins-bot:

[mediawiki/core@master] ResourceLoaderContext: Set message 'interface' flag and unique 'title'

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

A follow-up endavour might be to also make this the default in Message.php such that interface=true is always false unless setInterfaceMessageFlag(false) is explicitly called (which we could subsequently deprecate if we believe there are no use cases for it). An initial way to detect this might for fetchMessage() to check whether interface is false (without setInterfaceMessageFlag false being called) and issue a Logstash warning in that case and see if we get any interesting hits that are valid (or bugs waiting to be fixed).

From what I can tell, the main purpose of it within the Parser is to add the wrapping div. The only other use (target language) seems obsoleted by MessageCache setting that explicitly already.