Page MenuHomePhabricator

TextFormatter treats all messages as non-interface messages
Open, Needs TriagePublic

Description

TextFormatter::format calls $message->inLanguage( $this->langCode ); to set the language, but inLanguage unconditionally sets $this->interface = false;, meaning all messages formatted via TextFormatter are not considered interface languages. This leads to issues such as {{GENDER:|x|y|z}} (implicit user) not being parsed correctly, like the example reported in T315205.

It should be noted that this is not an issue in code using wfMessage because that doesn't call inLanguage, and also code using RequestContext::msg(), which calls Message::setContext() which in turn does $this->interface = true; after calling inLanguage.

I know pretty much nothing about the specifics of the "interfaceMessage" flag, so I have no idea if this is intentional and what the best way forward would be.

Event Timeline

The documentation of the Message::$interface flag indeed needs an update, there are many behaviour differences behind it instead of simply True means the current user language, false content language.

I think TextFormatter is not a complete replacement or successor for MessageLocalizer, especially when we have a Context in hand. It just hid or postponed access to needed information, in a more uncontrollable way. For example, Parser would fallback to the global state for the title (Parser.php#4873) since the context page is not set via Message::setContext() or Message::page().

For the reported case, if you insist on TextFormatter, you can add a message param passing the username to the parser. It's hard for TextFormatter to preserve the same behaviour of setting the interface message flag, since that again requires access to the context user.

I think TextFormatter is not a complete replacement or successor for MessageLocalizer, especially when we have a Context in hand. It just hid or postponed access to needed information, in a more uncontrollable way. For example, Parser would fallback to the global state for the title (Parser.php#4873) since the context page is not set via Message::setContext() or Message::page().

This is correct, but I thought the eventual goal was to make ITextFormatter a complete replacement for MessageLocalizer, since the latter cannot always be conveniently obtained. It's also worth noting that ITextFormatter is not even complete in its own; for instance, it only supports the text format (T260689). But maybe I'm just wrong and MessageLocalizer is meant to stay around. CC'ing @daniel, who worked on ITextFormatter with Petr a while ago.

Cc'ing @ihurbain because {{GENDER:|x|y|z}} is probably a good testcase for Parsoid's l10n pass as well, and because apparently we should be trying to use MessageSpecifier and HtmlFormatter (the latter doesn't not yet exist! but should be similar to TextFormatter) instead of Message in new code.

The interface flag should be set when parsing wiki interface, and unset when parsing wiki content. It's checked only to enable and disable some parser features in the interface and content: https://codesearch.wmcloud.org/search/?q=getInterfaceMessage (the reasons behind of them are more obvious than others… don't ask me why these features in particular).

The Message class has some bad defaults for it for historical reasons (T291601#7410926), causing some things to be parsed in the wrong mode, but you're allowed to change them. We do that in a number of places: https://codesearch.wmcloud.org/search/?q=setInterfaceMessageFlag (admittedly, I added a lot of them: T291601, T291776, T326855 but it never caused any issues).

TextFormatter should just set it unconditionally.