OutputPage: Make use of WrappedStringList in headElement()
authorTimo Tijhof <krinklemail@gmail.com>
Thu, 7 Jul 2016 19:26:36 +0000 (20:26 +0100)
committerOri.livneh <ori@wikimedia.org>
Tue, 12 Jul 2016 18:56:48 +0000 (18:56 +0000)
Right now, getInlineHeadScripts(), buildCssLinks() and getExternalHeadScripts()
all return WrappedString::join(). But because they don't know about each other
and because they need to return strings (not arrays), headElement() has no way
of merging them.

WrappedStringList allows this array to be kept, whilst still being backward-compatible
with code that calls these methods and assumig a string (since it will lazy-join
the array if the object is treated like a string).

To be used by I8b6c6a10d965e7396. Output is not changed in this commit.
Merely refactoring.

Change-Id: Iae08345473bd93cc0948d51b62c48aeb1ea460a3

includes/OutputPage.php

index 15b70c8..c667fb9 100644 (file)
@@ -23,6 +23,7 @@
 use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\Session\SessionManager;
 use WrappedString\WrappedString;
+use WrappedString\WrappedStringList;
 
 /**
  * This class should be covered by a general architecture document which does
@@ -2633,7 +2634,8 @@ class OutputPage extends ContextSource {
                $userdir = $this->getLanguage()->getDir();
                $sitedir = $wgContLang->getDir();
 
-               $ret = Html::htmlHeader( $sk->getHtmlElementAttributes() );
+               $pieces = [];
+               $pieces[] = Html::htmlHeader( $sk->getHtmlElementAttributes() );
 
                if ( $this->getHTMLTitle() == '' ) {
                        $this->setHTMLTitle( $this->msg( 'pagetitle', $this->getPageTitle() )->inContentLanguage() );
@@ -2641,8 +2643,7 @@ class OutputPage extends ContextSource {
 
                $openHead = Html::openElement( 'head' );
                if ( $openHead ) {
-                       # Don't bother with the newline if $head == ''
-                       $ret .= "$openHead\n";
+                       $pieces[] = $openHead;
                }
 
                if ( !Html::isXmlMimeType( $this->getConfig()->get( 'MimeType' ) ) ) {
@@ -2654,25 +2655,25 @@ class OutputPage extends ContextSource {
                        // Our XML declaration is output by Html::htmlHeader.
                        // http://www.whatwg.org/html/semantics.html#attr-meta-http-equiv-content-type
                        // http://www.whatwg.org/html/semantics.html#charset
-                       $ret .= Html::element( 'meta', [ 'charset' => 'UTF-8' ] ) . "\n";
+                       $pieces[] = Html::element( 'meta', [ 'charset' => 'UTF-8' ] );
                }
 
-               $ret .= Html::element( 'title', null, $this->getHTMLTitle() ) . "\n";
-               $ret .= $this->getInlineHeadScripts() . "\n";
-               $ret .= $this->buildCssLinks() . "\n";
-               $ret .= $this->getExternalHeadScripts() . "\n";
+               $pieces[] = Html::element( 'title', null, $this->getHTMLTitle() );
+               $pieces[] = $this->getInlineHeadScripts();
+               $pieces[] = $this->buildCssLinks();
+               $pieces[] = $this->getExternalHeadScripts();
 
                foreach ( $this->getHeadLinksArray() as $item ) {
-                       $ret .= $item . "\n";
+                       $pieces[] = $item;
                }
 
                foreach ( $this->mHeadItems as $item ) {
-                       $ret .= $item . "\n";
+                       $pieces[] = $item;
                }
 
                $closeHead = Html::closeElement( 'head' );
                if ( $closeHead ) {
-                       $ret .= "$closeHead\n";
+                       $pieces[] = $closeHead;
                }
 
                $bodyClasses = [];
@@ -2701,9 +2702,9 @@ class OutputPage extends ContextSource {
                $sk->addToBodyAttributes( $this, $bodyAttrs );
                Hooks::run( 'OutputPageBodyAttributes', [ $this, $sk, &$bodyAttrs ] );
 
-               $ret .= Html::openElement( 'body', $bodyAttrs ) . "\n";
+               $pieces[] = Html::openElement( 'body', $bodyAttrs );
 
-               return $ret;
+               return WrappedStringList::join( "\n", $pieces );
        }
 
        /**
@@ -2904,7 +2905,7 @@ class OutputPage extends ContextSource {
        /**
         * Build html output from an array of links from makeResourceLoaderLink.
         * @param array $links
-        * @return string HTML
+        * @return string|WrappedStringList HTML
         */
        protected static function getHtmlFromLoaderLinks( array $links ) {
                $html = [];
@@ -2920,7 +2921,7 @@ class OutputPage extends ContextSource {
                // Filter out empty values
                $html = array_filter( $html, 'strlen' );
 
-               if ( count( $states ) ) {
+               if ( $states ) {
                        array_unshift( $html, ResourceLoader::makeInlineScript(
                                ResourceLoader::makeLoaderStateScript( $states )
                        ) );
@@ -2940,25 +2941,23 @@ class OutputPage extends ContextSource {
        }
 
        /**
-        * <script src="..."> tags for "<head>". This is the startup module
+        * <script src="..."> tags for "<head>".This is the startup module
         * and other modules marked with position 'top'.
         *
-        * @return string HTML fragment
+        * @return string|WrappedStringList HTML
         */
        function getExternalHeadScripts() {
-               $links = [];
-
                // Startup - this provides the client with the module
                // manifest and loads jquery and mediawiki base modules
+               $links = [];
                $links[] = $this->makeResourceLoaderLink( 'startup', ResourceLoaderModule::TYPE_SCRIPTS );
-
                return self::getHtmlFromLoaderLinks( $links );
        }
 
        /**
-        * <script>...</script> tags to put in "<head>".
+        * Inline "<script>" tags to put in "<head>".
         *
-        * @return string HTML fragment
+        * @return string|WrappedStringList HTML
         */
        function getInlineHeadScripts() {
                $links = [];
@@ -3018,7 +3017,7 @@ class OutputPage extends ContextSource {
         *
         * @param bool $unused Previously used to let this method change its output based
         *  on whether it was called by getExternalHeadScripts() or getBottomScripts().
-        * @return string
+        * @return string|WrappedStringList HTML
         */
        function getScriptsForBottomQueue( $unused = null ) {
                // Scripts "only" requests marked for bottom inclusion
@@ -3617,10 +3616,9 @@ class OutputPage extends ContextSource {
        }
 
        /**
-        * Build a set of "<link>" elements for the stylesheets specified in the $this->styles array.
-        * These will be applied to various media & IE conditionals.
+        * Build a set of "<link>" elements for stylesheets specified in the $this->styles array.
         *
-        * @return string
+        * @return string|WrappedStringList HTML
         */
        public function buildCssLinks() {
                global $wgContLang;
@@ -3722,7 +3720,9 @@ class OutputPage extends ContextSource {
                }
 
                // Add stuff in $otherTags (previewed user CSS if applicable)
-               return self::getHtmlFromLoaderLinks( $links ) . implode( '', $otherTags );
+               $links[] = implode( '', $otherTags );
+
+               return self::getHtmlFromLoaderLinks( $links );
        }
 
        /**