Page MenuHomePhabricator

The ext.visualEditor.desktopArticleTarget.init and ext.visualEditor.desktopArticleTarget.noscript modules should be slimmed down and not rely on the target system
Open, Needs TriagePublic

Description

When a ResourceLoader module does not set targets, it will throw a warning. The targets system was meant as a temporary protection mechanism and we are looking to disband it.

The ext.visualEditor.desktopArticleTarget.init currently relies on the target system to not load code on mobile. It accounts for the majority of warnings in T235712

Acceptance criteria

  • The ext.visualEditor.desktopArticleTarget.init module should not load on the mobile domain
  • The ext.visualEditor.desktopArticleTarget.noscript module should set group to 'noscript' so it only loads when JavaScript is not enabled similar to how the noscript module in core works.
  • The ext.visualEditor.desktopArticleTarget.noscript module should not load on the mobile domain
  • The ext.visualEditor.desktopArticleTarget.init module should load on the Minerva desktop skin.
  • The ext.visualEditor.desktopArticleTarget.noscript module should load on the Minerva desktop skin.
  • According to https://en.wikipedia.org/w/load.php?modules=ext.visualEditor.desktopArticleTarget.init it is 7.9kb gzipped and loads unconditionally - even when there is no edit button (as is the case with the apioutput skin) - is there any short-term fix to reduce the size of this bundle by deferring loading to later? if not can a task be created detailing how that should work?

Developer notes

Depending on whether this code should be loaded on the client or the server, you can use

if (  ExtensionRegistry::getInstance()->isLoaded( 'MobileFrontend' ) && MediaWikiServices::getInstance()->getService( 'MobileFrontend.Context' )->shouldDisplayMobileView() ) {
    // you are in mobile mode. Anything to do here?
} else {
    $output->addModules( [
        'ext.visualEditor.desktopArticleTarget.init',
        'ext.visualEditor.targetLoader'
    ] );
}

or

if ( mw.config.get('wgMFMode')  === undefined ) {
    mw.loader.using( 'ext.visualEditor.desktopArticleTarget.init' );
}

More details in T235712 about loading strategies.

Note: Setting targets on modules is optional here. The most important thing is to make sure you don't call OutputPage->addModule for skins operating in the mobile domain. The warning only occurs if you do that on a module without targets set.

@Jdlrobson is happy to review any changes towards this goal

Details

Related Gerrit Patches:
mediawiki/extensions/VisualEditor : masterUse MobileFrontend check for loading DesktopArticleTarget.init

Event Timeline

Jdlrobson renamed this task from The ext.visualEditor.desktopArticleTarget.init module should be slimmed down and not rely on the target system to The ext.visualEditor.desktopArticleTarget.init and ext.visualEditor.desktopArticleTarget.noscript modules should be slimmed down and not rely on the target system.Oct 30 2019, 7:14 PM
Jdlrobson created this task.
Jdlrobson updated the task description. (Show Details)

Change 547789 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/VisualEditor@master] Use MobileFrontend check for loading DesktopArticleTarget.init

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

Esanders updated the task description. (Show Details)Nov 1 2019, 8:50 PM

Change 547789 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Use MobileFrontend check for loading DesktopArticleTarget.init

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

Jdlrobson updated the task description. (Show Details)Nov 4 2019, 5:51 PM
JTannerWMF moved this task from To Triage to FY 19-20 on the VisualEditor board.Wed, Nov 6, 5:38 PM
JTannerWMF added a subscriber: JTannerWMF.

The Editing-team will pick this up again sometime before the end of the fiscal year