Page MenuHomePhabricator

Unexpected general module "ext.RevisionSlider.lazy" in styles queue.
Closed, ResolvedPublic

Description

This warning has been in local Vagrant logs for a long time but due to it not happening on every page view, it was presumably not noticed before. Since 976943c9913b, the severity of these violations has been raised to level "Warning" in production - in preparation for removing support for this error entirely.

This is being logged at a regular frequency from production wikis in the resourceloader channel. It's simple to fix (see parent task for more information.)

resourceloader.log
Unexpected general module "ext.RevisionSlider.lazy" in styles queue.

Details

Related Gerrit Patches:
mediawiki/extensions/RevisionSlider : wmf/1.30.0-wmf.1Split ext.RevisionSlider.lazy into Js and Css modules
mediawiki/extensions/RevisionSlider : masterSplit ext.RevisionSlider.lazy into Js and Css modules

Event Timeline

Krinkle created this task.May 10 2017, 12:53 AM
Restricted Application added a project: TCB-Team. · View Herald TranscriptMay 10 2017, 12:53 AM

Module ext.RevisionSlider.lazy consists of 1 CSS file and 1 JS file. It also has one or more dependencies.

It is loaded twice in the hooks file:

		// Load styles on page load to avoid FOUC
		$out->addModuleStyles( 'ext.RevisionSlider.lazy' );
		if ( $autoExpand ) {
			..
		} else {
			$out->addModules( 'ext.RevisionSlider.lazy' );

One uses addModuleStyles which loads it as part of the blocking stylesheet, and ignores the JS file and dependencies. And then a second time in the else branch using addModules , which loads asynchronously both the CSS and JS and does wait for dependencies.

This is unsupported, and also causes the CSS to load twice at different cascading points, which can be very confusing and actually additional FOUC-like rendering bugs for components other than RevisionSlider.

A module is either a styles module or a general module. I don't know the internals of this particular JS file. Most likely either the JS/dependencies of this module should be moved to another module that is loaded instead, or the module split up.

Please note that as of MediaWiki 1.29 there is built-in support to have a dependency from a general JS module to a style module. The mw.loader JS is aware of which style modules have already been loaded through addModuleStyles. So, while this module is currently only loaded from one PHP hook, you can have the comfort if it is loaded elsewhere in the future, the dependency on the stylesheet can still be declared in a stable way, in case that other use case does not have it preloaded with addModuleStyles.

Addshore added a subscriber: Addshore.

Change 353254 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/extensions/RevisionSlider@master] Split ext.RevisionSlider.lazy into Js and Css modules

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

Addshore claimed this task.May 11 2017, 8:46 AM
Restricted Application added a project: User-Addshore. · View Herald TranscriptMay 11 2017, 8:46 AM
Addshore moved this task from Proposed to Currently in sprint on the WMDE-QWERTY-Team board.

Change 353254 merged by jenkins-bot:
[mediawiki/extensions/RevisionSlider@master] Split ext.RevisionSlider.lazy into Js and Css modules

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

Change 353562 had a related patch set uploaded (by Krinkle; owner: Addshore):
[mediawiki/extensions/RevisionSlider@wmf/1.30.0-wmf.1] Split ext.RevisionSlider.lazy into Js and Css modules

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

Krinkle closed this task as Resolved.May 12 2017, 4:06 PM

Change 353562 merged by jenkins-bot:
[mediawiki/extensions/RevisionSlider@wmf/1.30.0-wmf.1] Split ext.RevisionSlider.lazy into Js and Css modules

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

Tobi_WMDE_SW moved this task from Done to Demoed on the WMDE-QWERTY-Team board.Jun 6 2017, 2:37 PM
mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:10 PM