Page MenuHomePhabricator

Make position of modules currently added with addModuleStyles explicit
Closed, ResolvedPublic

Description

I will default my changes to "top", because this is the default behavior currently provided by OutputPage. In the spirit of trying to break as little as possible, I am now making this terrible historical default into an explicit value definition. The goal being to subsequently change the default behavior of OutputPage while breaking as little as possible.

"top" is a terrible default for styles that don't need to be rendered at pageload. If one of my changesets comes your way and it's obvious that the module at hand doesn't need to be in the head, please update the position value in the changeset to be "bottom" instead.

Details

Related Gerrit Patches:
mediawiki/extensions/TimedMediaHandler : masterExplicitly define module position
mediawiki/extensions/CentralAuth : masterSeparate globalrenamerequest styles from scripts
mediawiki/core : wmf/1.26wmf8Backport Calendar module position fix
mediawiki/extensions/Calendar : wmf/1.26wmf8Explicitly define module position
mediawiki/extensions/Calendar : masterExplicitly define module position
mediawiki/core : wmf/1.26wmf8Backport CodeReview module position fix
mediawiki/extensions/CodeReview : wmf/1.26wmf8Explicitly define module position
mediawiki/core : wmf/1.26wmf8Make ResourceLoaderWikiModule support custom position
mediawiki/extensions/VectorBeta : wmf/1.26wmf8Explicitly define module position
mediawiki/extensions/SyntaxHighlight_GeSHi : wmf/1.26wmf8Explicitly define module position
mediawiki/extensions/Wikibase : wmf/1.26wmf6Separate module definition for addModuleStyles
mediawiki/extensions/WikiEditor : wmf/1.26wmf8Explicitly define module position
mediawiki/core : masterMake ResourceLoaderWikiModule support custom position
mediawiki/extensions/SyntaxHighlight_GeSHi : masterRemove redundant position code defined in ResourceLoaderWikiModule
mediawiki/extensions/CodeReview : masterExplicitly define module position
mediawiki/core : masterExplicitly define filepage module position
mediawiki/extensions/VectorBeta : masterExplicitly define module position
mediawiki/extensions/SyntaxHighlight_GeSHi : wmf/1.26wmf7Explicitly define module position
mediawiki/extensions/Gather : masterExplicitly define module position
mediawiki/extensions/SyntaxHighlight_GeSHi : masterExplicitly define module position
mediawiki/extensions/MobileFrontend : masterFix more modules' inexplicit positions
mediawiki/extensions/Gadgets : wmf/1.26wmf8Acknowledge that gadgets have their position explicitly defined
mediawiki/extensions/Gadgets : masterAcknowledge that gadgets have their position explicitly defined
mediawiki/extensions/GlobalCssJs : wmf/1.26wmf7Explicitly define module position
mediawiki/extensions/GlobalCssJs : masterExplicitly define module position
mediawiki/extensions/GlobalCssJs : wmf/1.26wmf8Explicitly define module position
mediawiki/extensions/WikiEditor : masterExplicitly define module position
mediawiki/extensions/wikihiero : masterExplicitly define module position
mediawiki/extensions/Wikibase : masterSeparate module definition for addModuleStyles
mediawiki/extensions/MobileFrontend : masterExplicitly define module position
mediawiki/extensions/TimedMediaHandler : masterExplicitly define module position
mediawiki/extensions/UploadWizard : masterUploadWizard doesn't need addModuleStyles
mediawiki/extensions/MobileFrontend : masterExplicitly define module position
mediawiki/extensions/UniversalLanguageSelector : masterExplicitly define module position
mediawiki/skins/CologneBlue : masterExplicitly define module position
mediawiki/skins/Modern : masterExplicitly define module position
mediawiki/extensions/Wikibase : masterExplicitly define module position
mediawiki/skins/MonoBook : masterExplicitly define module position
mediawiki/skins/Modern : masterExplicitly define module position
mediawiki/skins/CologneBlue : masterExplicitly define module position
mediawiki/skins/Vector : masterExplicitly define module position
mediawiki/extensions/UploadWizard : masterExplicitly define module position
mediawiki/extensions/OpenStackManager : masterExplicitly define module position
mediawiki/extensions/OAuth : masterExplicitly define module position
mediawiki/extensions/MassMessage : masterExplicitly define module position
mediawiki/extensions/InputBox : masterExplicitly define module position
mediawiki/extensions/GettingStarted : masterExplicitly define module position
mediawiki/extensions/FlaggedRevs : masterExplicitly define module position
mediawiki/extensions/CodeReview : masterExplicitly define module position
mediawiki/extensions/AbuseFilter : masterExplicitly define module position
mediawiki/skins/Nostalgia : masterExplicitly define module position
mediawiki/extensions/GlobalUserPage : masterExplicitly define module position
mediawiki/extensions/Flow : masterExplicitly define module position
mediawiki/extensions/Echo : masterExplicitly define module position
mediawiki/extensions/CentralAuth : masterExplicitly define module position
mediawiki/extensions/MoodBar : masterExplicitly define module position
mediawiki/extensions/ZeroBanner : masterExplicitly define module position
mediawiki/extensions/WikimediaIncubator : masterExplicitly define module position
mediawiki/extensions/Wikidata : masterExplicitly define module position
mediawiki/extensions/ProofreadPage : masterExplicitly define module position
mediawiki/extensions/Gather : masterExplicitly define module position
mediawiki/extensions/Calendar : masterExplicitly define module position
mediawiki/extensions/CategoryTree : masterExplicitly define module position
mediawiki/extensions/Babel : masterExplicitly define module position
mediawiki/extensions/VisualEditor : masterExplicitly define module position
mediawiki/extensions/CiteThisPage : masterExplicitly define module position
mediawiki/extensions/Math : masterExplicitly define module position
mediawiki/extensions/ConfirmEdit : masterExplicitly define module position

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 214399 had a related patch set uploaded (by Legoktm):
Explicitly define module position

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

Change 214399 merged by jenkins-bot:
Explicitly define module position

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

Change 214401 had a related patch set uploaded (by Legoktm):
Explicitly define module position

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

Change 214401 merged by Legoktm:
Explicitly define module position

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

Change 213804 merged by jenkins-bot:
Acknowledge that gadgets have their position explicitly defined

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

Change 214406 had a related patch set uploaded (by Legoktm):
Acknowledge that gadgets have their position explicitly defined

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

Change 214406 merged by jenkins-bot:
Acknowledge that gadgets have their position explicitly defined

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

Change 214408 had a related patch set uploaded (by Gilles):
Fix more modules' inexplicit positions

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

Change 214410 had a related patch set uploaded (by Gilles):
Explicitly define module position

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

Change 214411 had a related patch set uploaded (by Gilles):
Explicitly define module position

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

Change 214408 abandoned by Gilles:
Fix more modules' inexplicit positions

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

Since the position is to be explicit, can we have a RL test that assert the position is defined? Can be made in tests/phpunit/structure/ResourcesTest.php. Beware that tests there are run for all extensions.

I thought the point of all CSS loading at top by default is that there is no point in loading CSS at the bottom. I can think of no scenario where CSS is not needed at page load. The only possible advantage is that content comes sooner, but you also risk style flashes.

@Edokter a notable exception, which is the use case this change was needed for (in MobileFrontend), is icons embedded in the CSS which are needed by the regular site as well as the no-JS site. As long as the space for the icons is reserved for them, which is the case on mobile web, there is no reflow, the icons just appear in place. Just like any other image on the page, such as thumbnails inside the article.

Furthermore, ignoring the no-JS/JS issue for a second, CSS can and should definitely loaded on demand when it makes sense, like for example if you invoke a modal dialog or a menu that has its own styles, not share with the main page.

You can also decide to make some concessions like having the hover and pressed styles of buttons loaded later. Any style whose loading won't cause the content to reflow, really.

The upside to getting rid of as much CSS in the head is huge as it helps display the page much sooner, particularly on the mobile site which is browsed by people with very slow connections.

As an aside, I've started discussing this idea: T100778 which might allow us to get rid of addModuleStyles entirely. These icons embedded in CSS would still be bottom-loaded, but in a much nicer way using ResourceLoader.

Change 214619 had a related patch set uploaded (by Aude):
Separate module definition for addModuleStyles

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

Change 207086 merged by jenkins-bot:
Explicitly define module position

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

Change 214632 had a related patch set uploaded (by BryanDavis):
Explicitly define module position

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

Change 214633 had a related patch set uploaded (by BryanDavis):
Explicitly define module position

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

Change 214410 merged by jenkins-bot:
Explicitly define module position

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

Change 214411 abandoned by Gilles:
Explicitly define module position

Reason:
It's empty...

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

Change 214632 abandoned by Jforrester:
Explicitly define module position

Reason:
Not needed, breaking core change is wmf8 only.

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

Change 214669 had a related patch set uploaded (by Jforrester):
Explicitly define module position

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

Change 214683 had a related patch set uploaded (by Gilles):
Explicitly define module position

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

Change 214695 had a related patch set uploaded (by Gilles):
Make ResourceLoaderWikiModule support custom position

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

Change 214698 had a related patch set uploaded (by Gilles):
Remove redundant position code defined in ResourceLoaderWikiModule

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

Change 214683 merged by jenkins-bot:
Explicitly define module position

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

Change 214695 merged by jenkins-bot:
Make ResourceLoaderWikiModule support custom position

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

Change 214698 merged by jenkins-bot:
Remove redundant position code defined in ResourceLoaderWikiModule

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

Change 214741 had a related patch set uploaded (by Gilles):
Make ResourceLoaderWikiModule support custom position

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

Change 214310 merged by jenkins-bot:
Explicitly define module position

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

Change 214619 merged by jenkins-bot:
Separate module definition for addModuleStyles

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

Change 214633 merged by jenkins-bot:
Explicitly define module position

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

Change 214669 merged by jenkins-bot:
Explicitly define module position

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

Change 214741 merged by jenkins-bot:
Make ResourceLoaderWikiModule support custom position

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

Change 215041 had a related patch set uploaded (by Gilles):
Explicitly define module position

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

Change 215041 merged by jenkins-bot:
Explicitly define module position

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

Change 215043 had a related patch set uploaded (by Gilles):
Backport CodeReview module position fix

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

Change 215043 merged by jenkins-bot:
Backport CodeReview module position fix

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

Gilles added a comment.Jun 1 2015, 4:02 PM

All warnings in group 0 wikis should have been addressed now.

Change 215545 had a related patch set uploaded (by Gilles):
Explicitly define module position

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

Change 215546 had a related patch set uploaded (by Gilles):
Explicitly define module position

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

Change 215545 merged by jenkins-bot:
Explicitly define module position

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

Change 215546 merged by jenkins-bot:
Explicitly define module position

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

Change 215547 had a related patch set uploaded (by Gilles):
Backport Calendar module position fix

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

Change 215547 merged by jenkins-bot:
Backport Calendar module position fix

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

Krinkle added a subscriber: Krinkle.EditedJun 4 2015, 1:34 PM
@Krinkle merged Ic8d3d63b1 in mediawiki/extensions/WikimediaMessages:

modules: Add position=top to style module
https://gerrit.wikimedia.org/r/215862

Change 210006 merged by jenkins-bot:
Separate globalrenamerequest styles from scripts

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

Change 217180 had a related patch set uploaded (by Paladox):
Explicitly define module position

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

Change 217180 abandoned by Paladox:
Explicitly define module position

Reason:
It seems that updating to latest branched version fixed problem.

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

mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:12 PM