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 659ac9d..9d27400 100644 (file)
--- 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 2274793..bb7207d 100644 (file)
--- 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 b49a2da..b9ff732 100644 (file)
--- 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 e1f2969..156df67 100644 (file)
--- 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 5dfa7e3..2b05dd8 100644 (file)
--- 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 );
        }
 
        /**