From: Timo Tijhof <krinklemail@gmail.com>
Date: Thu, 24 May 2018 13:10:48 +0000 (+0100)
Subject: Enable merging of WrappedStringList between 'bottomscripts' and 'reportime'
X-Git-Tag: 1.34.0-rc.0~5299^2
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=03db2f5b3c9f1de1dcc7098a5ec1f060a933b987

Enable merging of WrappedStringList between 'bottomscripts' and 'reportime'

* Fix ResourceLoaderClientHtml to return what it was documented to
  return, a WrappedStringList. It accidentally used the wrong join()
  method, causing it to create a plain string too early.

* Update method documentations from ClientHtml::getBodyHtml to
  BaseTemplate::getTrail for 'bottomscripts', and from
  ResourceLoader::makeInlineScript for 'reporttime'.

* Update BaseTemplate::getTrail to join by new line instead of native string
  concatenation.

This by itself would suffice for the most common case, but in order
to also account for the possibility of extensions using hooks for
'SkinAfterBottomScripts' that concatenate, update bottomScripts()
to pass a plain string to the hook and merge it later.

Change-Id: If0d227cb9db67229a27d489c082db790ea8e3840
---

diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index 659ac9d268..9d27400fd0 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -1515,7 +1515,7 @@ function wfHostname() {
  * hostname of the server handling the request.
  *
  * @param string $nonce Value from OutputPage::getCSPNonce
- * @return string
+ * @return string|WrappedString HTML
  */
 function wfReportTime( $nonce = null ) {
 	global $wgShowHostnames;
diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 227479336b..bb7207d6df 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1504,7 +1504,7 @@ MESSAGE;
 	 *
 	 * @param string $script JavaScript code
 	 * @param string $nonce [optional] Content-Security-Policy nonce (from OutputPage::getCSPNonce)
-	 * @return WrappedString HTML
+	 * @return string|WrappedString HTML
 	 */
 	public static function makeInlineScript( $script, $nonce = null ) {
 		$js = self::makeLoaderConditionalScript( $script );
diff --git a/includes/resourceloader/ResourceLoaderClientHtml.php b/includes/resourceloader/ResourceLoaderClientHtml.php
index b49a2da3d2..b9ff7325ac 100644
--- a/includes/resourceloader/ResourceLoaderClientHtml.php
+++ b/includes/resourceloader/ResourceLoaderClientHtml.php
@@ -18,6 +18,7 @@
  * @file
  */
 
+use Wikimedia\WrappedString;
 use Wikimedia\WrappedStringList;
 
 /**
@@ -351,7 +352,7 @@ class ResourceLoaderClientHtml {
 			$startupQuery
 		);
 
-		return WrappedStringList::join( "\n", $chunks );
+		return WrappedString::join( "\n", $chunks );
 	}
 
 	/**
@@ -369,7 +370,7 @@ class ResourceLoaderClientHtml {
 			);
 		}
 
-		return WrappedStringList::join( "\n", $chunks );
+		return WrappedString::join( "\n", $chunks );
 	}
 
 	private function getContext( $group, $type ) {
diff --git a/includes/skins/BaseTemplate.php b/includes/skins/BaseTemplate.php
index e1f2969a77..156df67f2e 100644
--- a/includes/skins/BaseTemplate.php
+++ b/includes/skins/BaseTemplate.php
@@ -18,6 +18,9 @@
  * @file
  */
 
+use Wikimedia\WrappedString;
+use Wikimedia\WrappedStringList;
+
 /**
  * New base template for a skin's template extended from QuickTemplate
  * this class features helper methods that provide common ways of interacting
@@ -754,14 +757,14 @@ abstract class BaseTemplate extends QuickTemplate {
 	 * debug stuff. This should be called right before outputting the closing
 	 * body and html tags.
 	 *
-	 * @return string
+	 * @return string|WrappedStringList HTML
 	 * @since 1.29
 	 */
-	function getTrail() {
-		$html = MWDebug::getDebugHTML( $this->getSkin()->getContext() );
-		$html .= $this->get( 'bottomscripts' );
-		$html .= $this->get( 'reporttime' );
-
-		return $html;
+	public function getTrail() {
+		return WrappedString::join( "\n", [
+			MWDebug::getDebugHTML( $this->getSkin()->getContext() ),
+			$this->get( 'bottomscripts' ),
+			$this->get( 'reporttime' )
+		] );
 	}
 }
diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php
index 5dfa7e36c3..2b05dd8942 100644
--- a/includes/skins/Skin.php
+++ b/includes/skins/Skin.php
@@ -21,6 +21,8 @@
  */
 
 use MediaWiki\MediaWikiServices;
+use Wikimedia\WrappedString;
+use Wikimedia\WrappedStringList;
 
 /**
  * @defgroup Skins Skins
@@ -402,7 +404,7 @@ abstract class Skin extends ContextSource {
 	/**
 	 * @param array $data
 	 * @param string $nonce OutputPage::getCSPNonce()
-	 * @return string
+	 * @return string|WrappedString HTML
 	 */
 	static function makeVariablesScript( $data, $nonce = null ) {
 		if ( $data ) {
@@ -675,16 +677,22 @@ abstract class Skin extends ContextSource {
 	/**
 	 * This gets called shortly before the "</body>" tag.
 	 *
-	 * @return string HTML-wrapped JS code to be put before "</body>"
+	 * @return string|WrappedStringList HTML containing scripts to put before `</body>`
 	 */
 	function bottomScripts() {
 		// TODO and the suckage continues. This function is really just a wrapper around
 		// OutputPage::getBottomScripts() which takes a Skin param. This should be cleaned
 		// up at some point
-		$bottomScriptText = $this->getOutput()->getBottomScripts();
-		Hooks::run( 'SkinAfterBottomScripts', [ $this, &$bottomScriptText ] );
-
-		return $bottomScriptText;
+		$chunks = [ $this->getOutput()->getBottomScripts() ];
+
+		// Keep the hook appendage separate to preserve WrappedString objects.
+		// This enables BaseTemplate::getTrail() to merge them where possible.
+		$extraHtml = '';
+		Hooks::run( 'SkinAfterBottomScripts', [ $this, &$extraHtml ] );
+		if ( $extraHtml !== '' ) {
+			$chunks[] = $extraHtml;
+		}
+		return WrappedString::join( "\n", $chunks );
 	}
 
 	/**