resoureloader: Consolidate styles-only queue at the top

This effectively reverts d6b4d3c537 and declines T97420.
This was previously attempted in b7c0e537eb.

Drop support for position "bottom" for addModuleStyles().
This feature was only recently introduced with the intent
to optimise page load performance, but had an adverse effect.

It increases chances of FOUC due to late discovery of these styles.
It also caused minor problems for some gadgets and extensions
that did not or were unable to set these flags. Some mobile
code was introduced around the same time as this feature and
was never given position=top.

Stylesheets that don't affect initial render or are only needed
on interaction should be loaded via addModules() instead; which
is handled by the asynchronous loader in JavaScript.

Change-Id: Ib9821b7b87cfc8a59062bb6ca358974fdb01ced1

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index d29ec54..147527e 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -610,20 +610,6 @@ class OutputPage extends ContextSource {
         * @return array Array of module names
         */
        public function getModuleStyles( $filter = false, $position = null ) {
-               // T97420
-               $resourceLoader = $this->getResourceLoader();
-
-               foreach ( $this->mModuleStyles as $val ) {
-                       $module = $resourceLoader->getModule( $val );
-
-                       if ( $module instanceof ResourceLoaderModule && $module->isPositionDefault() ) {
-                               $warning = __METHOD__ . ': style module should define its position explicitly: ' .
-                                       $val . ' ' . get_class( $module );
-                               wfDebugLog( 'resourceloader', $warning );
-                               wfLogWarning( $warning );
-                       }
-               }
-
                return $this->getModules( $filter, $position, 'mModuleStyles' );
        }
 
@@ -3074,10 +3060,6 @@ class OutputPage extends ContextSource {
                        ResourceLoaderModule::TYPE_SCRIPTS
                );
 
-               $links[] = $this->makeResourceLoaderLink( $this->getModuleStyles( true, 'bottom' ),
-                       ResourceLoaderModule::TYPE_STYLES
-               );
-
                // Modules requests - let the client calculate dependencies and batch requests as it likes
                // Only load modules that have marked themselves for loading at the bottom
                $modules = $this->getModules( true, 'bottom' );
@@ -3143,9 +3125,6 @@ class OutputPage extends ContextSource {
         * @return string
         */
        function getBottomScripts() {
-               // In case the skin wants to add bottom CSS
-               $this->getSkin()->setupSkinUserCss( $this );
-
                return $this->getScriptsForBottomQueue();
        }
 
@@ -3684,7 +3663,7 @@ class OutputPage extends ContextSource {
                $otherTags = array(); // Tags to append after the normal <link> tags
                $resourceLoader = $this->getResourceLoader();
 
-               $moduleStyles = $this->getModuleStyles( true, 'top' );
+               $moduleStyles = $this->getModuleStyles();
 
                // Per-site custom styles
                $moduleStyles[] = 'site';

diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php
index 1b827db..e4def4d 100644 (file)
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -279,7 +279,6 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                                        break;
                                // Single strings
                                case 'position':
-                                       $this->isPositionDefined = true;
                                case 'group':
                                case 'skipFunction':
                                        $this->{$member} = (string)$option;

diff --git a/includes/resourceloader/ResourceLoaderImageModule.php b/includes/resourceloader/ResourceLoaderImageModule.php
index 8de87f2..e2da28b 100644 (file)
--- a/includes/resourceloader/ResourceLoaderImageModule.php
+++ b/includes/resourceloader/ResourceLoaderImageModule.php
@@ -184,7 +184,6 @@ class ResourceLoaderImageModule extends ResourceLoaderModule {
                                        break;
 
                                case 'position':
-                                       $this->isPositionDefined = true;
                                case 'prefix':
                                case 'selectorWithoutVariant':
                                case 'selectorWithVariant':
@@ -456,9 +455,4 @@ class ResourceLoaderImageModule extends ResourceLoaderModule {
                $this->loadFromDefinition();
                return $this->position;
        }
-
-       public function isPositionDefault() {
-               $this->loadFromDefinition();
-               return parent::isPositionDefault();
-       }
 }

diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php
index 714a8ff..3dd7a4b 100644 (file)
--- a/includes/resourceloader/ResourceLoaderModule.php
+++ b/includes/resourceloader/ResourceLoaderModule.php
@@ -67,10 +67,6 @@ abstract class ResourceLoaderModule {
        // In-object cache for module content
        protected $contents = array();
 
-       // Whether the position returned by getPosition() is defined in the module configuration
-       // and not a default value
-       protected $isPositionDefined = false;
-
        /**
         * @var Config
         */
@@ -291,19 +287,6 @@ abstract class ResourceLoaderModule {
                return 'bottom';
        }
 
-       /**
-        * Whether the position returned by getPosition() is a default value or comes from the module
-        * definition. This method is meant to be short-lived, and is only useful until classes added
-        * via addModuleStyles with a default value define an explicit position. See getModuleStyles()
-        * in OutputPage for the related migration warning.
-        *
-        * @return bool
-        * @since  1.26
-        */
-       public function isPositionDefault() {
-               return !$this->isPositionDefined;
-       }
-
        /**
         * Whether this module's JS expects to work without the client-side ResourceLoader module.
         * Returning true from this function will prevent mw.loader.state() call from being

diff --git a/includes/resourceloader/ResourceLoaderWikiModule.php b/includes/resourceloader/ResourceLoaderWikiModule.php
index 156ff4e..57555f0 100644 (file)
--- a/includes/resourceloader/ResourceLoaderWikiModule.php
+++ b/includes/resourceloader/ResourceLoaderWikiModule.php
@@ -72,8 +72,6 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
                foreach ( $options as $member => $option ) {
                        switch ( $member ) {
                                case 'position':
-                                       $this->isPositionDefined = true;
-                                       // Don't break since we need the member set as well
                                case 'styles':
                                case 'scripts':
                                case 'group':