From 0088c506bb86d93055a4a335f78ed74c3a754173 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Thu, 14 Nov 2013 17:54:19 +0100
Subject: [PATCH] resourceloader: Refactor module links output

Changes:

* Removed hardcoded logic in OutputPage regarding modules
  being "enabled".
  Previously we would always output state=loading and use
  $wgAllowUserJs (and others) to decide whether to output
  state=ready or makeResourceLoaderLink.

  Now, we no longer unconditionally output state=loading and
  simply always call makeResourceLoaderLink. That method takes
  care of checking whether modules are enabled and non-empty
  and returns state=ready when that is the case.

  This cleans up cases where the duplicated and incomplete logic
  in OutputPage thought the module was non-empty but turned out
  to be empty and thus would output both state=loading and later
  state=ready for the same module.

* Clean up documentation for makeResourceLoaderLink (inconsistent
  ordering of type hint and $var, and @return was missing the fact
  that the returned html can also contain <link>).

* makeResourceLoaderLink now returns an array of html and module
  states. This allows the consumer of this method to combine the
  states in 1 larger script tag on top of multiple
  makeResourceLoaderLink calls (e.g. one state script followed
  by multiple <script src=load.php>).
  This isn't to reduce html output, but to make sure we inform
  mw.loader about modules before the <script src=load.php>.
  If we were to mix/alternate the state script and load.php
  requests (which are blocking in html), it is possible for those
  scripts to request other modules. We need to prevent duplicate
  loading of modules we already know are going to be requested
  by the HTML output futher down.

* Removed spurious new line.

Example of change in HTML output:

* The output has been reduced from:
  - loader.state( site: loading, user: loading, user.groups: loading )
  - loader.load( .. )
  - <script src="load.php?modules=site ..">
  - loader.state( user: ready, user.groups: ready )

  to:
  - loader.state( site: loading, user: ready, user.groups: ready )
  - loader.load( .. )
  - <script src="load.php?modules=site ..">

Change-Id: I91754ce5fae3d05b4bfa7372372eba81ee2fc579
---
 includes/OutputPage.php | 305 +++++++++++++++++++---------------------
 1 file changed, 148 insertions(+), 157 deletions(-)

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index 854290477e..3a6787f726 100644
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2530,12 +2530,15 @@ $templates
 		// jQuery can work correctly.
 		$ret .= Html::element( 'meta', array( 'http-equiv' => 'X-UA-Compatible', 'content' => 'IE=EDGE' ) ) . "\n";
 
-		$ret .= implode( "\n", array(
-			$this->getHeadLinks(),
-			$this->buildCssLinks(),
-			$this->getHeadScripts(),
+		$ret .= (
+			$this->getHeadLinks() .
+			"\n" .
+			$this->buildCssLinks() .
+			// No newline after buildCssLinks since makeResourceLoaderLink did that already
+			$this->getHeadScripts() .
+			"\n" .
 			$this->getHeadItems()
-		) );
+		);
 
 		$closeHead = Html::closeElement( 'head' );
 		if ( $closeHead ) {
@@ -2586,22 +2589,28 @@ $templates
 
 	/**
 	 * TODO: Document
-	 * @param $modules Array/string with the module name(s)
+	 * @param array|string $modules One or more module names
 	 * @param string $only ResourceLoaderModule TYPE_ class constant
-	 * @param $useESI boolean
+	 * @param boolean $useESI
 	 * @param array $extraQuery with extra query parameters to add to each request. array( param => value )
-	 * @param $loadCall boolean If true, output an (asynchronous) mw.loader.load() call rather than a "<script src='...'>" tag
-	 * @return string html "<script>" and "<style>" tags
+	 * @param boolean $loadCall If true, output an (asynchronous) mw.loader.load() call rather than a "<script src='...'>" tag
+	 * @return string The html "<script>", "<link>" and "<style>" tags
 	 */
 	protected function makeResourceLoaderLink( $modules, $only, $useESI = false, array $extraQuery = array(), $loadCall = false ) {
 		global $wgResourceLoaderUseESI;
 
 		$modules = (array)$modules;
 
+		$links = array(
+			'html' => '',
+			'states' => array(),
+		);
+
 		if ( !count( $modules ) ) {
-			return '';
+			return $links;
 		}
 
+
 		if ( count( $modules ) > 1 ) {
 			// Remove duplicate module requests
 			$modules = array_unique( $modules );
@@ -2610,13 +2619,15 @@ $templates
 
 			if ( ResourceLoader::inDebugMode() ) {
 				// Recursively call us for every item
-				$links = '';
 				foreach ( $modules as $name ) {
-					$links .= $this->makeResourceLoaderLink( $name, $only, $useESI );
+					$link = $this->makeResourceLoaderLink( $name, $only, $useESI );
+					$links['html'] .= $link['html'];
+					$links['states'] += $link['states'];
 				}
 				return $links;
 			}
 		}
+
 		if ( !is_null( $this->mTarget ) ) {
 			$extraQuery['target'] = $this->mTarget;
 		}
@@ -2644,7 +2655,6 @@ $templates
 			$groups[$group][$name] = $module;
 		}
 
-		$links = '';
 		foreach ( $groups as $group => $grpModules ) {
 			// Special handling for user-specific groups
 			$user = null;
@@ -2667,25 +2677,20 @@ $templates
 				$extraQuery
 			);
 			$context = new ResourceLoaderContext( $resourceLoader, new FauxRequest( $query ) );
+
 			// Extract modules that know they're empty
-			$emptyModules = array();
 			foreach ( $grpModules as $key => $module ) {
+				// Inline empty modules: since they're empty, just mark them as 'ready' (bug 46857)
+				// If we're only getting the styles, we don't need to do anything for empty modules.
 				if ( $module->isKnownEmpty( $context ) ) {
-					$emptyModules[$key] = 'ready';
 					unset( $grpModules[$key] );
+					if ( $only !== ResourceLoaderModule::TYPE_STYLES ) {
+						$links['states'][$key] = 'ready';
+					}
 				}
 			}
-			// Inline empty modules: since they're empty, just mark them as 'ready'
-			if ( count( $emptyModules ) > 0 && $only !== ResourceLoaderModule::TYPE_STYLES ) {
-				// If we're only getting the styles, we don't need to do anything for empty modules.
-				$links .= Html::inlineScript(
-						ResourceLoader::makeLoaderConditionalScript(
-								ResourceLoader::makeLoaderStateScript( $emptyModules )
-						)
-				) . "\n";
-			}
 
-			// If there are no modules left, skip this group
+			// If there are no non-empty modules, skip this group
 			if ( count( $grpModules ) === 0 ) {
 				continue;
 			}
@@ -2696,19 +2701,20 @@ $templates
 			// properly use them as dependencies (bug 30914)
 			if ( $group === 'private' ) {
 				if ( $only == ResourceLoaderModule::TYPE_STYLES ) {
-					$links .= Html::inlineStyle(
+					$links['html'] .= Html::inlineStyle(
 						$resourceLoader->makeModuleResponse( $context, $grpModules )
 					);
 				} else {
-					$links .= Html::inlineScript(
+					$links['html'] .= Html::inlineScript(
 						ResourceLoader::makeLoaderConditionalScript(
 							$resourceLoader->makeModuleResponse( $context, $grpModules )
 						)
 					);
 				}
-				$links .= "\n";
+				$links['html'] .= "\n";
 				continue;
 			}
+
 			// Special handling for the user group; because users might change their stuff
 			// on-wiki like user pages, or user preferences; we need to find the highest
 			// timestamp of these user-changeable modules so we can ensure cache misses on change
@@ -2756,18 +2762,56 @@ $templates
 					);
 				} else {
 					$link = Html::linkedScript( $url );
+
+					// For modules requested directly in the html via <link> or <script>,
+					// tell mw.loader they are being loading to prevent duplicate requests.
+					foreach ( $grpModules as $key => $module ) {
+						// Don't output state=loading for the startup module..
+						if ( $key !== 'startup' ) {
+							$links['states'][$key] = 'loading';
+						}
+					}
 				}
 			}
 
 			if ( $group == 'noscript' ) {
-				$links .= Html::rawElement( 'noscript', array(), $link ) . "\n";
+				$links['html'] .= Html::rawElement( 'noscript', array(), $link ) . "\n";
 			} else {
-				$links .= $link . "\n";
+				$links['html'] .= $link . "\n";
 			}
 		}
+
 		return $links;
 	}
 
+	/**
+	 * Build html output from an array of links from makeResourceLoaderLink.
+	 * @param array $links
+	 * @return string HTML
+	 */
+	protected static function getHtmlFromLoaderLinks( Array $links ) {
+		$html = '';
+		$states = array();
+		foreach ( $links as $link ) {
+			if ( !is_array( $link ) ) {
+				$html .= $link;
+			} else {
+				$html .= $link['html'];
+				$states += $link['states'];
+			}
+		}
+
+		if ( count( $states ) ) {
+			$html = Html::inlineScript(
+				ResourceLoader::makeLoaderConditionalScript(
+					ResourceLoader::makeLoaderStateScript( $states )
+				)
+			) . "\n" . $html;
+		}
+
+		return $html;
+	}
+
 	/**
 	 * JS stuff to put in the "<head>". This is the startup module, config
 	 * vars and modules marked with position 'top'
@@ -2778,10 +2822,11 @@ $templates
 		global $wgResourceLoaderExperimentalAsyncLoading;
 
 		// Startup - this will immediately load jquery and mediawiki modules
-		$scripts = $this->makeResourceLoaderLink( 'startup', ResourceLoaderModule::TYPE_SCRIPTS, true );
+		$links = array();
+		$links[] = $this->makeResourceLoaderLink( 'startup', ResourceLoaderModule::TYPE_SCRIPTS, true );
 
 		// Load config before anything else
-		$scripts .= Html::inlineScript(
+		$links[] = Html::inlineScript(
 			ResourceLoader::makeLoaderConditionalScript(
 				ResourceLoader::makeConfigSetScript( $this->getJSVars() )
 			)
@@ -2791,18 +2836,18 @@ $templates
 		// This needs to be TYPE_COMBINED so these modules are properly wrapped
 		// in mw.loader.implement() calls and deferred until mw.user is available
 		$embedScripts = array( 'user.options', 'user.tokens' );
-		$scripts .= $this->makeResourceLoaderLink( $embedScripts, ResourceLoaderModule::TYPE_COMBINED );
+		$links[] = $this->makeResourceLoaderLink( $embedScripts, ResourceLoaderModule::TYPE_COMBINED );
 
-		// Script and Messages "only" requests marked for top inclusion
+		// Scripts and messages "only" requests marked for top inclusion
 		// Messages should go first
-		$scripts .= $this->makeResourceLoaderLink( $this->getModuleMessages( true, 'top' ), ResourceLoaderModule::TYPE_MESSAGES );
-		$scripts .= $this->makeResourceLoaderLink( $this->getModuleScripts( true, 'top' ), ResourceLoaderModule::TYPE_SCRIPTS );
+		$links[] = $this->makeResourceLoaderLink( $this->getModuleMessages( true, 'top' ), ResourceLoaderModule::TYPE_MESSAGES );
+		$links[] = $this->makeResourceLoaderLink( $this->getModuleScripts( true, 'top' ), ResourceLoaderModule::TYPE_SCRIPTS );
 
 		// Modules requests - let the client calculate dependencies and batch requests as it likes
 		// Only load modules that have marked themselves for loading at the top
 		$modules = $this->getModules( true, 'top' );
 		if ( $modules ) {
-			$scripts .= Html::inlineScript(
+			$links[] = Html::inlineScript(
 				ResourceLoader::makeLoaderConditionalScript(
 					Xml::encodeJsCall( 'mw.loader.load', array( $modules ) )
 				)
@@ -2810,17 +2855,17 @@ $templates
 		}
 
 		if ( $wgResourceLoaderExperimentalAsyncLoading ) {
-			$scripts .= $this->getScriptsForBottomQueue( true );
+			$links[] = $this->getScriptsForBottomQueue( true );
 		}
 
-		return $scripts;
+		return self::getHtmlFromLoaderLinks( $links );
 	}
 
 	/**
 	 * JS stuff to put at the 'bottom', which can either be the bottom of the "<body>"
 	 * or the bottom of the "<head>" depending on $wgResourceLoaderExperimentalAsyncLoading:
 	 * modules marked with position 'bottom', legacy scripts ($this->mScripts),
-	 * user preferences, site JS and user JS
+	 * user preferences, site JS and user JS.
 	 *
 	 * @param $inHead boolean If true, this HTML goes into the "<head>", if false it goes into the "<body>"
 	 * @return string
@@ -2828,14 +2873,15 @@ $templates
 	function getScriptsForBottomQueue( $inHead ) {
 		global $wgUseSiteJs, $wgAllowUserJs;
 
-		// Script and Messages "only" requests marked for bottom inclusion
+		// Scripts and messages "only" requests marked for bottom inclusion
 		// If we're in the <head>, use load() calls rather than <script src="..."> tags
 		// Messages should go first
-		$scripts = $this->makeResourceLoaderLink( $this->getModuleMessages( true, 'bottom' ),
+		$links = array();
+		$links[] = $this->makeResourceLoaderLink( $this->getModuleMessages( true, 'bottom' ),
 			ResourceLoaderModule::TYPE_MESSAGES, /* $useESI = */ false, /* $extraQuery = */ array(),
 			/* $loadCall = */ $inHead
 		);
-		$scripts .= $this->makeResourceLoaderLink( $this->getModuleScripts( true, 'bottom' ),
+		$links[] = $this->makeResourceLoaderLink( $this->getModuleScripts( true, 'bottom' ),
 			ResourceLoaderModule::TYPE_SCRIPTS, /* $useESI = */ false, /* $extraQuery = */ array(),
 			/* $loadCall = */ $inHead
 		);
@@ -2844,7 +2890,7 @@ $templates
 		// Only load modules that have marked themselves for loading at the bottom
 		$modules = $this->getModules( true, 'bottom' );
 		if ( $modules ) {
-			$scripts .= Html::inlineScript(
+			$links[] = Html::inlineScript(
 				ResourceLoader::makeLoaderConditionalScript(
 					Xml::encodeJsCall( 'mw.loader.load', array( $modules, null, true ) )
 				)
@@ -2852,88 +2898,40 @@ $templates
 		}
 
 		// Legacy Scripts
-		$scripts .= "\n" . $this->mScripts;
-
-		$defaultModules = array();
+		$links[] = "\n" . $this->mScripts;
 
 		// Add site JS if enabled
-		if ( $wgUseSiteJs ) {
-			$scripts .= $this->makeResourceLoaderLink( 'site', ResourceLoaderModule::TYPE_SCRIPTS,
-				/* $useESI = */ false, /* $extraQuery = */ array(), /* $loadCall = */ $inHead
-			);
-			$defaultModules['site'] = 'loading';
-		} else {
-			// Site module is empty, save request by marking ready in advance (bug 46857)
-			$defaultModules['site'] = 'ready';
-		}
+		$links[] = $this->makeResourceLoaderLink( 'site', ResourceLoaderModule::TYPE_SCRIPTS,
+			/* $useESI = */ false, /* $extraQuery = */ array(), /* $loadCall = */ $inHead
+		);
 
 		// Add user JS if enabled
-		if ( $wgAllowUserJs ) {
-			if ( $this->getUser()->isLoggedIn() ) {
-				if ( $this->getTitle() && $this->getTitle()->isJsSubpage() && $this->userCanPreview() ) {
-					# XXX: additional security check/prompt?
-					// We're on a preview of a JS subpage
-					// Exclude this page from the user module in case it's in there (bug 26283)
-					$scripts .= $this->makeResourceLoaderLink( 'user', ResourceLoaderModule::TYPE_SCRIPTS, false,
-						array( 'excludepage' => $this->getTitle()->getPrefixedDBkey() ), $inHead
-					);
-					// Load the previewed JS
-					$scripts .= Html::inlineScript( "\n" . $this->getRequest()->getText( 'wpTextbox1' ) . "\n" ) . "\n";
-					// FIXME: If the user is previewing, say, ./vector.js, his ./common.js will be loaded
-					// asynchronously and may arrive *after* the inline script here. So the previewed code
-					// may execute before ./common.js runs. Normally, ./common.js runs before ./vector.js...
-				} else {
-					// Include the user module normally, i.e., raw to avoid it being wrapped in a closure.
-					$scripts .= $this->makeResourceLoaderLink( 'user', ResourceLoaderModule::TYPE_SCRIPTS,
-						/* $useESI = */ false, /* $extraQuery = */ array(), /* $loadCall = */ $inHead
-					);
-				}
-				$defaultModules['user'] = 'loading';
-			} else {
-				// Non-logged-in users have an empty user module.
-				// Save request by marking ready in advance (bug 46857)
-				$defaultModules['user'] = 'ready';
-			}
+		if ( $wgAllowUserJs && $this->getUser()->isLoggedIn() && $this->getTitle() && $this->getTitle()->isJsSubpage() && $this->userCanPreview() ) {
+			# XXX: additional security check/prompt?
+			// We're on a preview of a JS subpage
+			// Exclude this page from the user module in case it's in there (bug 26283)
+			$links[] = $this->makeResourceLoaderLink( 'user', ResourceLoaderModule::TYPE_SCRIPTS, false,
+				array( 'excludepage' => $this->getTitle()->getPrefixedDBkey() ), $inHead
+			);
+			// Load the previewed JS
+			$links[] = Html::inlineScript( "\n" . $this->getRequest()->getText( 'wpTextbox1' ) . "\n" ) . "\n";
+
+			// FIXME: If the user is previewing, say, ./vector.js, his ./common.js will be loaded
+			// asynchronously and may arrive *after* the inline script here. So the previewed code
+			// may execute before ./common.js runs. Normally, ./common.js runs before ./vector.js...
 		} else {
-			// User modules are disabled on this wiki.
-			// Save request by marking ready in advance (bug 46857)
-			$defaultModules['user'] = 'ready';
+			// Include the user module normally, i.e., raw to avoid it being wrapped in a closure.
+			$links[] = $this->makeResourceLoaderLink( 'user', ResourceLoaderModule::TYPE_SCRIPTS,
+				/* $useESI = */ false, /* $extraQuery = */ array(), /* $loadCall = */ $inHead
+			);
 		}
 
 		// Group JS is only enabled if site JS is enabled.
-		if ( $wgUseSiteJs ) {
-			if ( $this->getUser()->isLoggedIn() ) {
-				$scripts .= $this->makeResourceLoaderLink( 'user.groups', ResourceLoaderModule::TYPE_COMBINED,
-					/* $useESI = */ false, /* $extraQuery = */ array(), /* $loadCall = */ $inHead
-				);
-				$defaultModules['user.groups'] = 'loading';
-			} else {
-				// Non-logged-in users have no user.groups module.
-				// Save request by marking ready in advance (bug 46857)
-				$defaultModules['user.groups'] = 'ready';
-			}
-		} else {
-			// Site (and group JS) disabled
-			$defaultModules['user.groups'] = 'ready';
-		}
+		$links[] = $this->makeResourceLoaderLink( 'user.groups', ResourceLoaderModule::TYPE_COMBINED,
+			/* $useESI = */ false, /* $extraQuery = */ array(), /* $loadCall = */ $inHead
+		);
 
-		$loaderInit = '';
-		if ( $inHead ) {
-			// We generate loader calls anyway, so no need to fix the client-side loader's state to 'loading'.
-			foreach ( $defaultModules as $m => $state ) {
-				if ( $state == 'loading' ) {
-					unset( $defaultModules[$m] );
-				}
-			}
-		}
-		if ( count( $defaultModules ) > 0 ) {
-			$loaderInit = Html::inlineScript(
-				ResourceLoader::makeLoaderConditionalScript(
-					ResourceLoader::makeLoaderStateScript( $defaultModules )
-				)
-			) . "\n";
-		}
-		return $loaderInit . $scripts;
+		return self::getHtmlFromLoaderLinks( $links );
 	}
 
 	/**
@@ -3399,7 +3397,7 @@ $templates
 			# If wanted, and the interface is right-to-left, flip the CSS
 			$style_css = CSSJanus::transform( $style_css, true, false );
 		}
-		$this->mInlineStyles .= Html::inlineStyle( $style_css );
+		$this->mInlineStyles .= Html::inlineStyle( $style_css ) . "\n";
 	}
 
 	/**
@@ -3414,49 +3412,43 @@ $templates
 		$this->getSkin()->setupSkinUserCss( $this );
 
 		// Add ResourceLoader styles
-		// Split the styles into four groups
+		// Split the styles into these groups
 		$styles = array( 'other' => array(), 'user' => array(), 'site' => array(), 'private' => array(), 'noscript' => array() );
+		$links = array();
 		$otherTags = ''; // Tags to append after the normal <link> tags
 		$resourceLoader = $this->getResourceLoader();
 
 		$moduleStyles = $this->getModuleStyles();
 
 		// Per-site custom styles
-		if ( $wgUseSiteCss ) {
-			$moduleStyles[] = 'site';
-			$moduleStyles[] = 'noscript';
-			if ( $this->getUser()->isLoggedIn() ) {
-				$moduleStyles[] = 'user.groups';
-			}
-		}
+		$moduleStyles[] = 'site';
+		$moduleStyles[] = 'noscript';
+		$moduleStyles[] = 'user.groups';
 
 		// Per-user custom styles
-		if ( $wgAllowUserCss ) {
-			if ( $this->getTitle()->isCssSubpage() && $this->userCanPreview() ) {
-				// We're on a preview of a CSS subpage
-				// Exclude this page from the user module in case it's in there (bug 26283)
-				$otherTags .= $this->makeResourceLoaderLink( 'user', ResourceLoaderModule::TYPE_STYLES, false,
-					array( 'excludepage' => $this->getTitle()->getPrefixedDBkey() )
-				);
-
-				// Load the previewed CSS
-				// If needed, Janus it first. This is user-supplied CSS, so it's
-				// assumed to be right for the content language directionality.
-				$previewedCSS = $this->getRequest()->getText( 'wpTextbox1' );
-				if ( $this->getLanguage()->getDir() !== $wgContLang->getDir() ) {
-					$previewedCSS = CSSJanus::transform( $previewedCSS, true, false );
-				}
-				$otherTags .= Html::inlineStyle( $previewedCSS );
-			} else {
-				// Load the user styles normally
-				$moduleStyles[] = 'user';
+		if ( $wgAllowUserCss && $this->getTitle()->isCssSubpage() && $this->userCanPreview() ) {
+			// We're on a preview of a CSS subpage
+			// Exclude this page from the user module in case it's in there (bug 26283)
+			$link = $this->makeResourceLoaderLink( 'user', ResourceLoaderModule::TYPE_STYLES, false,
+				array( 'excludepage' => $this->getTitle()->getPrefixedDBkey() )
+			);
+			$otherTags .= $link['html'];
+
+			// Load the previewed CSS
+			// If needed, Janus it first. This is user-supplied CSS, so it's
+			// assumed to be right for the content language directionality.
+			$previewedCSS = $this->getRequest()->getText( 'wpTextbox1' );
+			if ( $this->getLanguage()->getDir() !== $wgContLang->getDir() ) {
+				$previewedCSS = CSSJanus::transform( $previewedCSS, true, false );
 			}
+			$otherTags .= Html::inlineStyle( $previewedCSS ) . "\n";
+		} else {
+			// Load the user styles normally
+			$moduleStyles[] = 'user';
 		}
 
 		// Per-user preference styles
-		if ( $wgAllowUserCssPrefs ) {
-			$moduleStyles[] = 'user.cssprefs';
-		}
+		$moduleStyles[] = 'user.cssprefs';
 
 		foreach ( $moduleStyles as $name ) {
 			$module = $resourceLoader->getModule( $name );
@@ -3464,34 +3456,33 @@ $templates
 				continue;
 			}
 			$group = $module->getGroup();
-			// Modules in groups named "other" or anything different than "user", "site" or "private"
+			// Modules in groups different than the ones listed on top (see $styles assignment)
 			// will be placed in the "other" group
-			$styles[isset( $styles[$group] ) ? $group : 'other'][] = $name;
+			$styles[ isset( $styles[$group] ) ? $group : 'other' ][] = $name;
 		}
 
 		// We want site, private and user styles to override dynamically added styles from modules, but we want
 		// dynamically added styles to override statically added styles from other modules. So the order
 		// has to be other, dynamic, site, private, user
 		// Add statically added styles for other modules
-		$ret = $this->makeResourceLoaderLink( $styles['other'], ResourceLoaderModule::TYPE_STYLES );
+		$links[] = $this->makeResourceLoaderLink( $styles['other'], ResourceLoaderModule::TYPE_STYLES );
 		// Add normal styles added through addStyle()/addInlineStyle() here
-		$ret .= implode( "\n", $this->buildCssLinksArray() ) . $this->mInlineStyles;
+		$links[] = implode( "\n", $this->buildCssLinksArray() ) . $this->mInlineStyles;
 		// Add marker tag to mark the place where the client-side loader should inject dynamic styles
 		// We use a <meta> tag with a made-up name for this because that's valid HTML
-		$ret .= Html::element( 'meta', array( 'name' => 'ResourceLoaderDynamicStyles', 'content' => '' ) ) . "\n";
+		$links[] = Html::element( 'meta', array( 'name' => 'ResourceLoaderDynamicStyles', 'content' => '' ) ) . "\n";
 
 		// Add site, private and user styles
 		// 'private' at present only contains user.options, so put that before 'user'
 		// Any future private modules will likely have a similar user-specific character
 		foreach ( array( 'site', 'noscript', 'private', 'user' ) as $group ) {
-			$ret .= $this->makeResourceLoaderLink( $styles[$group],
-					ResourceLoaderModule::TYPE_STYLES
+			$links[] = $this->makeResourceLoaderLink( $styles[$group],
+				ResourceLoaderModule::TYPE_STYLES
 			);
 		}
 
 		// Add stuff in $otherTags (previewed user CSS if applicable)
-		$ret .= $otherTags;
-		return $ret;
+		return self::getHtmlFromLoaderLinks( $links ) . $otherTags;
 	}
 
 	/**
-- 
2.20.1