resourceloader: Use WrappedString library to merge RLQ inline scripts

Update unit test to account for the internal 'html' prop now
being an array instead of string. And update expected values to
no longer have a trailing line break.

Bug: T27202
Change-Id: I105b6ef2e64ab8b891562e16940edb88592bd415

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index f9f2470..c972045 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -21,6 +21,7 @@
  */
 
 use MediaWiki\Logger\LoggerFactory;
+use WrappedString\WrappedString;
 
 /**
  * This class should be covered by a general architecture document which does
@@ -2778,7 +2779,9 @@ class OutputPage extends ContextSource {
                $modules = (array)$modules;
 
                $links = array(
-                       'html' => '',
+                       // List of html strings
+                       'html' => array(),
+                       // Associative array of module names and their states
                        'states' => array(),
                );
 
@@ -2796,7 +2799,7 @@ class OutputPage extends ContextSource {
                                // Recursively call us for every item
                                foreach ( $modules as $name ) {
                                        $link = $this->makeResourceLoaderLink( $name, $only, $useESI );
-                                       $links['html'] .= $link['html'];
+                                       $links['html'] = array_merge( $links['html'], $link['html'] );
                                        $links['states'] += $link['states'];
                                }
                                return $links;
@@ -2880,15 +2883,14 @@ class OutputPage extends ContextSource {
                                // properly use them as dependencies (bug 30914)
                                if ( $group === 'private' ) {
                                        if ( $only == ResourceLoaderModule::TYPE_STYLES ) {
-                                               $links['html'] .= Html::inlineStyle(
+                                               $links['html'][] = Html::inlineStyle(
                                                        $resourceLoader->makeModuleResponse( $context, $grpModules )
                                                );
                                        } else {
-                                               $links['html'] .= ResourceLoader::makeInlineScript(
+                                               $links['html'][] = ResourceLoader::makeInlineScript(
                                                        $resourceLoader->makeModuleResponse( $context, $grpModules )
                                                );
                                        }
-                                       $links['html'] .= "\n";
                                        continue;
                                }
 
@@ -2945,9 +2947,9 @@ class OutputPage extends ContextSource {
                                }
 
                                if ( $group == 'noscript' ) {
-                                       $links['html'] .= Html::rawElement( 'noscript', array(), $link ) . "\n";
+                                       $links['html'][] = Html::rawElement( 'noscript', array(), $link );
                                } else {
-                                       $links['html'] .= $link . "\n";
+                                       $links['html'][] = $link;
                                }
                        }
                }
@@ -2961,24 +2963,26 @@ class OutputPage extends ContextSource {
         * @return string HTML
         */
        protected static function getHtmlFromLoaderLinks( array $links ) {
-               $html = '';
+               $html = array();
                $states = array();
                foreach ( $links as $link ) {
                        if ( !is_array( $link ) ) {
-                               $html .= $link;
+                               $html[] = $link;
                        } else {
-                               $html .= $link['html'];
+                               $html = array_merge( $html, $link['html'] );
                                $states += $link['states'];
                        }
                }
+               // Filter out empty values
+               $html = array_filter( $html, 'strlen' );
 
                if ( count( $states ) ) {
-                       $html = ResourceLoader::makeInlineScript(
+                       array_unshift( $html, ResourceLoader::makeInlineScript(
                                ResourceLoader::makeLoaderStateScript( $states )
-                       ) . "\n" . $html;
+                       ) );
                }
 
-               return $html;
+               return WrappedString::join( "\n", $html );
        }
 
        /**
@@ -3063,7 +3067,7 @@ class OutputPage extends ContextSource {
                }
 
                // Legacy Scripts
-               $links[] = "\n" . $this->mScripts;
+               $links[] = $this->mScripts;
 
                // Add user JS if enabled
                // This must use TYPE_COMBINED instead of only=scripts so that its request is handled by
@@ -3651,7 +3655,7 @@ class OutputPage extends ContextSource {
                        'noscript' => array()
                );
                $links = array();
-               $otherTags = ''; // Tags to append after the normal <link> tags
+               $otherTags = array(); // Tags to append after the normal <link> tags
                $resourceLoader = $this->getResourceLoader();
 
                $moduleStyles = $this->getModuleStyles( true, 'top' );
@@ -3670,7 +3674,7 @@ class OutputPage extends ContextSource {
                        $link = $this->makeResourceLoaderLink( 'user', ResourceLoaderModule::TYPE_STYLES, false,
                                array( 'excludepage' => $this->getTitle()->getPrefixedDBkey() )
                        );
-                       $otherTags .= $link['html'];
+                       $otherTags = array_merge( $otherTags, $link['html'] );
 
                        // Load the previewed CSS
                        // If needed, Janus it first. This is user-supplied CSS, so it's
@@ -3679,7 +3683,7 @@ class OutputPage extends ContextSource {
                        if ( $this->getLanguage()->getDir() !== $wgContLang->getDir() ) {
                                $previewedCSS = CSSJanus::transform( $previewedCSS, true, false );
                        }
-                       $otherTags .= Html::inlineStyle( $previewedCSS ) . "\n";
+                       $otherTags[] = Html::inlineStyle( $previewedCSS ) . "\n";
                } else {
                        // Load the user styles normally
                        $moduleStyles[] = 'user';
@@ -3720,7 +3724,7 @@ class OutputPage extends ContextSource {
                $links[] = Html::element(
                        'meta',
                        array( 'name' => 'ResourceLoaderDynamicStyles', 'content' => '' )
-               ) . "\n";
+               );
 
                // Add site-specific and user-specific styles
                // 'private' at present only contains user.options, so put that before 'user'
@@ -3732,7 +3736,7 @@ class OutputPage extends ContextSource {
                }
 
                // Add stuff in $otherTags (previewed user CSS if applicable)
-               return self::getHtmlFromLoaderLinks( $links ) . $otherTags;
+               return self::getHtmlFromLoaderLinks( $links ) . implode( '', $otherTags );
        }
 
        /**

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index fa131bd..0ea91fb 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -25,6 +25,7 @@
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
+use WrappedString\WrappedString;
 
 /**
  * Dynamic JavaScript and CSS resource loading system.
@@ -1389,11 +1390,15 @@ MESSAGE;
         * only if the client has adequate support for MediaWiki JavaScript code.
         *
         * @param string $script JavaScript code
-        * @return string HTML
+        * @return WrappedString HTML
         */
        public static function makeInlineScript( $script ) {
                $js = self::makeLoaderConditionalScript( $script );
-               return Html::inlineScript( $js );
+               return new WrappedString(
+                       Html::inlineScript( $js ),
+                       "<script>var RLQ = RLQ || []; RLQ.push( function () {\n",
+                       "\n} );</script>"
+               );
        }
 
        /**

diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index bee44b9..0b38168 100644 (file)
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -145,28 +145,26 @@ class OutputPageTest extends MediaWikiTestCase {
                                        . 'document.write("\u003Cscript src=\"http://127.0.0.1:8080/w/load.php?'
                                        . 'debug=false\u0026amp;lang=en\u0026amp;modules=test.foo\u0026amp;only'
                                        . '=scripts\u0026amp;skin=fallback\u0026amp;*\"\u003E\u003C/script\u003E");'
-                                       . "\n} );</script>\n"
+                                       . "\n} );</script>"
                        ),
                        array(
                                // Don't condition wrap raw modules (like the startup module)
                                array( 'test.raw', ResourceLoaderModule::TYPE_SCRIPTS ),
-                               '<script src="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.raw&amp;only=scripts&amp;skin=fallback&amp;*"></script>
-'
+                               '<script src="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.raw&amp;only=scripts&amp;skin=fallback&amp;*"></script>'
                        ),
                        // Load module styles only
                        // This also tests the order the modules are put into the url
                        array(
                                array( array( 'test.baz', 'test.foo', 'test.bar' ), ResourceLoaderModule::TYPE_STYLES ),
 
-                               '<link rel=stylesheet href="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.bar%2Cbaz%2Cfoo&amp;only=styles&amp;skin=fallback&amp;*">
-'
+                               '<link rel=stylesheet href="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.bar%2Cbaz%2Cfoo&amp;only=styles&amp;skin=fallback&amp;*">'
                        ),
                        // Load private module (only=scripts)
                        array(
                                array( 'test.quux', ResourceLoaderModule::TYPE_SCRIPTS ),
                                "<script>var RLQ = RLQ || []; RLQ.push( function () {\n"
                                        . "mw.test.baz({token:123});mw.loader.state({\"test.quux\":\"ready\"});\n"
-                                       . "\n} );</script>\n"
+                                       . "\n} );</script>"
                        ),
                        // Load private module (combined)
                        array(
@@ -174,19 +172,17 @@ class OutputPageTest extends MediaWikiTestCase {
                                "<script>var RLQ = RLQ || []; RLQ.push( function () {\n"
                                        . "mw.loader.implement(\"test.quux\",function($,jQuery){"
                                        . "mw.test.baz({token:123});},{\"css\":[\".mw-icon{transition:none}\\n"
-                                       . "\"]});\n\n} );</script>\n"
+                                       . "\"]});\n\n} );</script>"
                        ),
                        // Load module script with ESI
                        array(
                                array( 'test.foo', ResourceLoaderModule::TYPE_SCRIPTS, true ),
-                               '<script><esi:include src="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.foo&amp;only=scripts&amp;skin=fallback&amp;*" /></script>
-'
+                               '<script><esi:include src="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.foo&amp;only=scripts&amp;skin=fallback&amp;*" /></script>'
                        ),
                        // Load module styles with ESI
                        array(
                                array( 'test.foo', ResourceLoaderModule::TYPE_STYLES, true ),
-                               '<style><esi:include src="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.foo&amp;only=styles&amp;skin=fallback&amp;*" /></style>
-',
+                               '<style><esi:include src="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.foo&amp;only=styles&amp;skin=fallback&amp;*" /></style>',
                        ),
                        // Load no modules
                        array(
@@ -196,8 +192,7 @@ class OutputPageTest extends MediaWikiTestCase {
                        // noscript group
                        array(
                                array( 'test.noscript', ResourceLoaderModule::TYPE_STYLES ),
-                               '<noscript><link rel=stylesheet href="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.noscript&amp;only=styles&amp;skin=fallback&amp;*"></noscript>
-'
+                               '<noscript><link rel=stylesheet href="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.noscript&amp;only=styles&amp;skin=fallback&amp;*"></noscript>'
                        ),
                        // Load two modules in separate groups
                        array(
@@ -207,7 +202,7 @@ class OutputPageTest extends MediaWikiTestCase {
                                        . "\n} );</script>\n"
                                        . "<script>var RLQ = RLQ || []; RLQ.push( function () {\n"
                                        . 'document.write("\u003Cscript src=\"http://127.0.0.1:8080/w/load.php?debug=false\u0026amp;lang=en\u0026amp;modules=test.group.foo\u0026amp;skin=fallback\u0026amp;*\"\u003E\u003C/script\u003E");'
-                                       . "\n} );</script>\n"
+                                       . "\n} );</script>"
                        ),
                );
        }
@@ -275,7 +270,7 @@ class OutputPageTest extends MediaWikiTestCase {
                ) );
                $links = $method->invokeArgs( $out, $args );
                // Strip comments to avoid variation due to wgDBname in WikiID and cache key
-               $actualHtml = preg_replace( '#/\*[^*]+\*/#', '', $links['html'] );
+               $actualHtml = preg_replace( '#/\*[^*]+\*/#', '', implode( "\n", $links['html'] ) );
                $this->assertEquals( $expectedHtml, $actualHtml );
        }
 }