From: Timo Tijhof <krinklemail@gmail.com>
Date: Mon, 20 Aug 2018 23:42:42 +0000 (+0100)
Subject: resourceloader: Move logo preload from OutputPage to SkinModule
X-Git-Tag: 1.34.0-rc.0~4264^2~1
X-Git-Url: http://git.cyclocoop.org/data/Luca_Pacioli_%28Gemaelde%29.jpeg?a=commitdiff_plain;h=5d0b5a402e384897288ad569da8d534fa2e432cb;p=lhc%2Fweb%2Fwiklou.git

resourceloader: Move logo preload from OutputPage to SkinModule

This was introduced in OutputPage before support for getPreloadLinks()
was added to ResourceLoader. The introduction in ResourceLoader was
actually inspired by this original implementation.

Now that we have it, we should make use of it for this module
as well. Doing so has several benefits:

* Makes the code cleaner by not requiring every skin to implement
  the extra boolean method. Instead, it naturally works. If
  the skin loads the SkinModule, it gets the preload as well.
  If not (such as Minerva, which has a different logo config),
  then it also doesn't get the preload link.
  Naturally, automatic.

* Makes code cleaner by not having static methods, and by not
  having OutputPage call into a Module class.

* Fixes the problem where, if a site's logo is changed, all cached
  HTML is preloading the old logo whilst the stylesheet fetches
  the newer one. Causing both to be downloaded.

* Still preloads the logo well before it can render.

Change-Id: I11b390f2e4f5e7db8b4506ab547839152888005c
---

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index c8e18e0218..3675e8a4ee 100644
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2364,10 +2364,6 @@ class OutputPage extends ContextSource {
 
 		if ( !$this->mArticleBodyOnly ) {
 			$sk = $this->getSkin();
-
-			if ( $sk->shouldPreloadLogo() ) {
-				$this->addLogoPreloadLinkHeaders();
-			}
 		}
 
 		$linkHeader = $this->getLinkHeader();
@@ -3915,80 +3911,6 @@ class OutputPage extends ContextSource {
 		] );
 	}
 
-	/**
-	 * Add Link headers for preloading the wiki's logo.
-	 *
-	 * @since 1.26
-	 */
-	protected function addLogoPreloadLinkHeaders() {
-		$logo = ResourceLoaderSkinModule::getLogo( $this->getConfig() );
-
-		$tags = [];
-		$logosPerDppx = [];
-		$logos = [];
-
-		if ( !is_array( $logo ) ) {
-			// No media queries required if we only have one variant
-			$this->addLinkHeader( '<' . $logo . '>;rel=preload;as=image' );
-			return;
-		}
-
-		if ( isset( $logo['svg'] ) ) {
-			// No media queries required if we only have a 1x and svg variant
-			// because all preload-capable browsers support SVGs
-			$this->addLinkHeader( '<' . $logo['svg'] . '>;rel=preload;as=image' );
-			return;
-		}
-
-		foreach ( $logo as $dppx => $src ) {
-			// Keys are in this format: "1.5x"
-			$dppx = substr( $dppx, 0, -1 );
-			$logosPerDppx[$dppx] = $src;
-		}
-
-		// Because PHP can't have floats as array keys
-		uksort( $logosPerDppx, function ( $a , $b ) {
-			$a = floatval( $a );
-			$b = floatval( $b );
-			// Sort from smallest to largest (e.g. 1x, 1.5x, 2x)
-			return $a <=> $b;
-		} );
-
-		foreach ( $logosPerDppx as $dppx => $src ) {
-			$logos[] = [ 'dppx' => $dppx, 'src' => $src ];
-		}
-
-		$logosCount = count( $logos );
-		// Logic must match ResourceLoaderSkinModule:
-		// - 1x applies to resolution < 1.5dppx
-		// - 1.5x applies to resolution >= 1.5dppx && < 2dppx
-		// - 2x applies to resolution >= 2dppx
-		// Note that min-resolution and max-resolution are both inclusive.
-		for ( $i = 0; $i < $logosCount; $i++ ) {
-			if ( $i === 0 ) {
-				// Smallest dppx
-				// min-resolution is ">=" (larger than or equal to)
-				// "not min-resolution" is essentially "<"
-				$media_query = 'not all and (min-resolution: ' . $logos[ 1 ]['dppx'] . 'dppx)';
-			} elseif ( $i !== $logosCount - 1 ) {
-				// In between
-				// Media query expressions can only apply "not" to the entire expression
-				// (e.g. can't express ">= 1.5 and not >= 2).
-				// Workaround: Use <= 1.9999 in place of < 2.
-				$upper_bound = floatval( $logos[ $i + 1 ]['dppx'] ) - 0.000001;
-				$media_query = '(min-resolution: ' . $logos[ $i ]['dppx'] .
-					'dppx) and (max-resolution: ' . $upper_bound . 'dppx)';
-			} else {
-				// Largest dppx
-				$media_query = '(min-resolution: ' . $logos[ $i ]['dppx'] . 'dppx)';
-			}
-
-			$this->addLinkHeader(
-				'<' . $logos[$i]['src'] . '>;rel=preload;as=image;media=' . $media_query
-			);
-		}
-	}
-
 	/**
 	 * Get (and set if not yet set) the CSP nonce.
 	 *
diff --git a/includes/resourceloader/ResourceLoaderSkinModule.php b/includes/resourceloader/ResourceLoaderSkinModule.php
index de25d32e54..69313bfbed 100644
--- a/includes/resourceloader/ResourceLoaderSkinModule.php
+++ b/includes/resourceloader/ResourceLoaderSkinModule.php
@@ -76,6 +76,89 @@ class ResourceLoaderSkinModule extends ResourceLoaderFileModule {
 		return $styles;
 	}
 
+	/**
+	 * @param ResourceLoaderContext $context
+	 * @return array
+	 */
+	public function getPreloadLinks( ResourceLoaderContext $context ) {
+		return $this->getLogoPreloadlinks();
+	}
+
+	/**
+	 * Helper method for getPreloadLinks()
+	 * @return array
+	 */
+	private function getLogoPreloadlinks() {
+		$logo = $this->getLogoData( $this->getConfig() );
+
+		$tags = [];
+		$logosPerDppx = [];
+		$logos = [];
+
+		$preloadLinks = [];
+
+		if ( !is_array( $logo ) ) {
+			// No media queries required if we only have one variant
+			$preloadLinks[ $logo ] = [ 'as' => 'image' ];
+			return $preloadLinks;
+		}
+
+		if ( isset( $logo['svg'] ) ) {
+			// No media queries required if we only have a 1x and svg variant
+			// because all preload-capable browsers support SVGs
+			$preloadLinks [ $logo['svg'] ] = [ 'as' => 'image' ];
+			return $preloadLinks;
+		}
+
+		foreach ( $logo as $dppx => $src ) {
+			// Keys are in this format: "1.5x"
+			$dppx = substr( $dppx, 0, -1 );
+			$logosPerDppx[$dppx] = $src;
+		}
+
+		// Because PHP can't have floats as array keys
+		uksort( $logosPerDppx, function ( $a , $b ) {
+			$a = floatval( $a );
+			$b = floatval( $b );
+			// Sort from smallest to largest (e.g. 1x, 1.5x, 2x)
+			return $a <=> $b;
+		} );
+
+		foreach ( $logosPerDppx as $dppx => $src ) {
+			$logos[] = [ 'dppx' => $dppx, 'src' => $src ];
+		}
+
+		$logosCount = count( $logos );
+		// Logic must match ResourceLoaderSkinModule:
+		// - 1x applies to resolution < 1.5dppx
+		// - 1.5x applies to resolution >= 1.5dppx && < 2dppx
+		// - 2x applies to resolution >= 2dppx
+		// Note that min-resolution and max-resolution are both inclusive.
+		for ( $i = 0; $i < $logosCount; $i++ ) {
+			if ( $i === 0 ) {
+				// Smallest dppx
+				// min-resolution is ">=" (larger than or equal to)
+				// "not min-resolution" is essentially "<"
+				$media_query = 'not all and (min-resolution: ' . $logos[ 1 ]['dppx'] . 'dppx)';
+			} elseif ( $i !== $logosCount - 1 ) {
+				// In between
+				// Media query expressions can only apply "not" to the entire expression
+				// (e.g. can't express ">= 1.5 and not >= 2).
+				// Workaround: Use <= 1.9999 in place of < 2.
+				$upper_bound = floatval( $logos[ $i + 1 ]['dppx'] ) - 0.000001;
+				$media_query = '(min-resolution: ' . $logos[ $i ]['dppx'] .
+					'dppx) and (max-resolution: ' . $upper_bound . 'dppx)';
+			} else {
+				// Largest dppx
+				$media_query = '(min-resolution: ' . $logos[ $i ]['dppx'] . 'dppx)';
+			}
+
+			$preloadLinks[ $logos[$i]['src'] ] = [ 'as' => 'image', 'media' => $media_query ];
+		}
+
+		return $preloadLinks;
+	}
+
 	/**
 	 * Ensure all media keys use array values.
 	 *
diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php
index b05fb0bcd9..2f5e0c8325 100644
--- a/includes/skins/Skin.php
+++ b/includes/skins/Skin.php
@@ -503,6 +503,10 @@ abstract class Skin extends ContextSource {
 
 	/**
 	 * Whether the logo should be preloaded with an HTTP link header or not
+	 *
+	 * @deprecated since 1.32 Redundant. It now happens automatically based on whether
+	 *  the skin loads a stylesheet based on ResourceLoaderSkinModule, which all
+	 *  skins that use wgLogo in CSS do, and other's would not.
 	 * @since 1.29
 	 * @return bool
 	 */
diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index b0cefc7333..1f3ee27fce 100644
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -2039,26 +2039,26 @@ class OutputPageTest extends MediaWikiTestCase {
 
 	/**
 	 * @dataProvider providePreloadLinkHeaders
-	 * @covers OutputPage::addLogoPreloadLinkHeaders
+	 * @covers ResourceLoaderSkinModule::getPreloadLinks
+	 * @covers ResourceLoaderSkinModule::getLogoPreloadlinks
 	 * @covers ResourceLoaderSkinModule::getLogo
 	 */
-	public function testPreloadLinkHeaders( $config, $result, $baseDir = null ) {
-		if ( $baseDir ) {
-			$this->setMwGlobals( 'IP', $baseDir );
-		}
-		$out = TestingAccessWrapper::newFromObject( $this->newInstance( $config ) );
-		$out->addLogoPreloadLinkHeaders();
+	public function testPreloadLinkHeaders( $config, $result ) {
+		$this->setMwGlobals( $config );
+		$ctx = $this->getMockBuilder( ResourceLoaderContext::class )
+			->disableOriginalConstructor()->getMock();
+		$module = new ResourceLoaderSkinModule();
 
-		$this->assertEquals( $result, $out->getLinkHeader() );
+		$this->assertEquals( [ $result ], $module->getHeaders( $ctx ) );
 	}
 
 	public function providePreloadLinkHeaders() {
 		return [
 			[
 				[
-					'ResourceBasePath' => '/w',
-					'Logo' => '/img/default.png',
-					'LogoHD' => [
+					'wgResourceBasePath' => '/w',
+					'wgLogo' => '/img/default.png',
+					'wgLogoHD' => [
 						'1.5x' => '/img/one-point-five.png',
 						'2x' => '/img/two-x.png',
 					],
@@ -2071,17 +2071,17 @@ class OutputPageTest extends MediaWikiTestCase {
 			],
 			[
 				[
-					'ResourceBasePath' => '/w',
-					'Logo' => '/img/default.png',
-					'LogoHD' => false,
+					'wgResourceBasePath' => '/w',
+					'wgLogo' => '/img/default.png',
+					'wgLogoHD' => false,
 				],
 				'Link: </img/default.png>;rel=preload;as=image'
 			],
 			[
 				[
-					'ResourceBasePath' => '/w',
-					'Logo' => '/img/default.png',
-					'LogoHD' => [
+					'wgResourceBasePath' => '/w',
+					'wgLogo' => '/img/default.png',
+					'wgLogoHD' => [
 						'2x' => '/img/two-x.png',
 					],
 				],
@@ -2091,9 +2091,9 @@ class OutputPageTest extends MediaWikiTestCase {
 			],
 			[
 				[
-					'ResourceBasePath' => '/w',
-					'Logo' => '/img/default.png',
-					'LogoHD' => [
+					'wgResourceBasePath' => '/w',
+					'wgLogo' => '/img/default.png',
+					'wgLogoHD' => [
 						'svg' => '/img/vector.svg',
 					],
 				],
@@ -2102,13 +2102,13 @@ class OutputPageTest extends MediaWikiTestCase {
 			],
 			[
 				[
-					'ResourceBasePath' => '/w',
-					'Logo' => '/w/test.jpg',
-					'LogoHD' => false,
-					'UploadPath' => '/w/images',
+					'wgResourceBasePath' => '/w',
+					'wgLogo' => '/w/test.jpg',
+					'wgLogoHD' => false,
+					'wgUploadPath' => '/w/images',
+					'IP' => dirname( __DIR__ ) . '/data/media',
 				],
 				'Link: </w/test.jpg?edcf2>;rel=preload;as=image',
-				'baseDir' => dirname( __DIR__ ) . '/data/media',
 			],
 		];
 	}