Page MenuHomePhabricator

ext.uls.interface should set targets and explicitly not target the Minerva skin
Closed, ResolvedPublic

Description

The ext.uls.interface module is a 3.36kb module that loads on launch of all pages. It accounts for many of the warnings in T235712. It loads if isToolbarEnabled and that function should return false when the skin is Minerva which I believe is the intended behaviour here.

Acceptance criteria

  • Note any dependencies of the module will also need to be updated to set appropriate targets - ext.uls.webfonts, ext.uls.common and jquery.uls.data. These don't appear to be added directly to the page so this should not impact the mobile site.
  • The module itself should set "targets": [ "desktop", "mobile" ], so that it does not throw the warning described in T235712
  • Update the body of isToolbarEnabled to consider skin. isToolbarEnabled should return false in skins such as Minerva where no toolbar is enabled.
private static function isToolbarEnabled( User $user ) {

Event Timeline

Change 604184 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/UniversalLanguageSelector@master] Define unsupported skins

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

Change 604184 merged by jenkins-bot:
[mediawiki/extensions/UniversalLanguageSelector@master] Define unsupported skins

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

Jdlrobson claimed this task.

https://en.m.wikipedia.beta.wmflabs.org/wiki/Main_Page

mw.loader.getState('ext.uls.interface') === 'registered'
true

LGTM!