Page MenuHomePhabricator

Deprecate and remove MediaWikiI18N class from Skin system
Closed, ResolvedPublic

Description

Digging into a typical skin and seeing an innocent-looking call to $this->msg() leads to a rather unexpected alternate call graph.

  • class VectorTemplate extends BaseTemplate: Calls $this->msg()
    • BaseTemplate::msg calls BaseTemplate::getMsg
      • BaseTemplate::getMsg calls $this->getSkin()->msg()

This looks quite reasonable. But BaseTemplate::msg is actually an override of the parent class QuickTemplate for which the implementation is different, involving a seemingly unused and ancient class called MediaWikiI18N.

  • QuickTemplate constructs MediaWikiI18N and stores as $this->translator
  • QuickTemplate::msg calls $this->translator->translate
  • MediaWikiI18N::translate performs preg_replace on the value, and then calls wfMessage(), then performs a while loop to perform $N replacements based on a locally stored data set.
  • Along the way, it mentions various things about PHPTAL, which it was used for to migrate from, in 2004.

It'd be good to figure out the plan to remove this ancient wrapper class. It seems QuickTemplate already has its own getSkin, so it seems like the easiest way would be to move msg() from BaseTemplate to QuickTemplate.

On the other hand, their signatures are not at all compatible, so a better way would be perhaps to remove it from QuickTemplate without replacement. And require any existing callers to either call wfMessage directly, or extend BaseTemplate instead to use msg().

All skins currently deployed at Wikimedia use BaseTemplate and not the underlying QuickTemplate. But, codesearch reveals a few uses still:

  • Challenge: ChallengeUserTemplate
  • Collection: CollectionFailedTemplate, CollectionFinishedTemplate, ..
  • EditAccount: EditAccountCloseAccountTemplate, EditAccountDisplayUserTemplate, ..
  • MediaWikiChat: SpecialChatTemplate
  • PollNY: CreatePollTemplate
  • SpamRegex: SpamRegexUITemplate
  • TopLists: TopListEditorUITemplate, ..
  • Blackout (skin): SopaStrikeTemplate, StopSopaTemplate: 21c60791d98452fa2385662ac948e754f89b371b
  • DumpHTML (skin): SkinOfflineTemplate
  • MinervaNeue (skin): phpunit/test (doesn't use localisation methods)

Looks like almost all of these are actually not skin templates, but fragments for pieces used by extension output. These should probably be converted to Mustache HTML templates instead.

Event Timeline

Krinkle triaged this task as Medium priority.Jan 31 2018, 3:35 AM
Krinkle created this task.

Change 407048 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/Blackout@master] Remove use of SkinTemplate/QuickTemplate, use Skin directly

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

Change 407048 merged by jenkins-bot:
[mediawiki/extensions/Blackout@master] Remove use of SkinTemplate/QuickTemplate, use Skin directly

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

Change 407754 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] skins: Deprecate QuickTemplate::setTranslator

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

Change 407755 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] [WIP] skins: Remove deprecated QuickTemplate::setTranslator

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

Change 407765 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/extensions/Collection@master] Replace QuickTemplate->translator->translate() with wfMessage()->text()

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

Change 407767 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/skins/CologneBlue@master] Replace QuickTemplate->translator->translate() with wfMessage()->text()

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

Change 407861 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] skins: Deprecate MediaWikiI18N::translate()

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

Change 407861 merged by jenkins-bot:
[mediawiki/core@master] skins: Deprecate MediaWikiI18N::translate()

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

Krinkle updated the task description. (Show Details)Feb 3 2018, 1:27 AM

Change 407755 merged by jenkins-bot:
[mediawiki/core@master] skins: Remove MediaWikiI18N class and QuickTemplate::setTranslator

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

Should we make this Resolved and open tasks for the repos not yet fixed?

matmarex removed a subscriber: matmarex.Apr 19 2018, 9:09 PM

Perhaps, but note that the checklist in the task description is not a list of result that use MediaWikiI18N (via QuickTemplate->translate). Rather, it's a list of results where QuickTemplate is extended directly, rather than extending the BaseTemplate class (which extends QuickTemplate and adds the "normal" msg function instead).

Many of these fortunately, don't use the now-removed translate() function though. They're just weird cases that use QuickTemplate for something that isn't a skin. Arguably these could be marked technical debt, considering we'd now use Mustache for something like that, rather than nested PHP code with stretched of raw HTML and/or echoed variables.

Krinkle closed this task as Resolved.Apr 20 2018, 9:15 PM
Krinkle claimed this task.