From ad77bb10f2351511e2076b490e89b18dac493616 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Thu, 30 Jul 2015 17:13:04 -0700 Subject: [PATCH] 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 --- includes/OutputPage.php | 42 ++++++++++++---------- includes/resourceloader/ResourceLoader.php | 9 +++-- tests/phpunit/includes/OutputPageTest.php | 25 ++++++------- 3 files changed, 40 insertions(+), 36 deletions(-) diff --git a/includes/OutputPage.php b/includes/OutputPage.php index f9f2470098..c972045655 100644 --- 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 tags + $otherTags = array(); // Tags to append after the normal 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 fa131bdd6d..0ea91fbd15 100644 --- 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 ), + "" + ); } /** diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php index bee44b9e30..0b381682cc 100644 --- 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} );\n" + . "\n} );" ), array( // Don't condition wrap raw modules (like the startup module) array( 'test.raw', ResourceLoaderModule::TYPE_SCRIPTS ), - ' -' + '' ), // 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 ), - ' -' + '' ), // Load private module (only=scripts) array( array( 'test.quux', ResourceLoaderModule::TYPE_SCRIPTS ), "\n" + . "\n} );" ), // Load private module (combined) array( @@ -174,19 +172,17 @@ class OutputPageTest extends MediaWikiTestCase { "\n" + . "\"]});\n\n} );" ), // Load module script with ESI array( array( 'test.foo', ResourceLoaderModule::TYPE_SCRIPTS, true ), - ' -' + '' ), // Load module styles with ESI array( array( 'test.foo', ResourceLoaderModule::TYPE_STYLES, true ), - ' -', + '', ), // Load no modules array( @@ -196,8 +192,7 @@ class OutputPageTest extends MediaWikiTestCase { // noscript group array( array( 'test.noscript', ResourceLoaderModule::TYPE_STYLES ), - ' -' + '' ), // Load two modules in separate groups array( @@ -207,7 +202,7 @@ class OutputPageTest extends MediaWikiTestCase { . "\n} );\n" . "\n" + . "\n} );" ), ); } @@ -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 ); } } -- 2.20.1