Ensure logo preload transforms urls if needed
authorTimo Tijhof <krinklemail@gmail.com>
Wed, 12 Apr 2017 20:43:57 +0000 (13:43 -0700)
committerBartosz Dziewoński <matma.rex@gmail.com>
Wed, 12 Apr 2017 21:34:31 +0000 (21:34 +0000)
Follows-up 5f55e9c9c2a24.

If the logo url is from within /w, then ResourceLoaderSkinModule
will (as it should) apply a file hash query to it.

The preloader didn't do that, so it specified the wrong url.

Refactored SkinModule to make this logic re-usable.

Bug: T100999
Change-Id: I1ba11f7c70d1a725ad72754fee4a3f33c2a4c1be

includes/OutputPage.php
includes/resourceloader/ResourceLoaderSkinModule.php
tests/phpunit/includes/OutputPageTest.php

index df49bf6..5c05ad7 100644 (file)
@@ -4005,29 +4005,22 @@ class OutputPage extends ContextSource {
         * @since 1.26
         */
        protected function addLogoPreloadLinkHeaders() {
-               $logo = $this->getConfig()->get( 'Logo' ); // wgLogo
-               $logoHD = $this->getConfig()->get( 'LogoHD' ); // wgLogoHD
+               $logo = ResourceLoaderSkinModule::getLogo( $this->getConfig() );
 
                $tags = [];
                $logosPerDppx = [];
                $logos = [];
 
-               $logosPerDppx['1.0'] = $logo;
-
-               if ( !$logoHD ) {
+               if ( !is_array( $logo ) ) {
                        // No media queries required if we only have one variant
                        $this->addLinkHeader( '<' . $logo . '>;rel=preload;as=image' );
                        return;
                }
 
-               foreach ( $logoHD as $dppx => $src ) {
-                       // Only 1.5x and 2x are supported
-                       // Note: Keep in sync with ResourceLoaderSkinModule
-                       if ( in_array( $dppx, [ '1.5x', '2x' ] ) ) {
-                               // LogoHD uses a string in this format: "1.5x"
-                               $dppx = substr( $dppx, 0, -1 );
-                               $logosPerDppx[$dppx] = $src;
-                       }
+               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
index 7d37944..5740925 100644 (file)
@@ -28,45 +28,77 @@ class ResourceLoaderSkinModule extends ResourceLoaderFileModule {
         * @return array
         */
        public function getStyles( ResourceLoaderContext $context ) {
-               $conf = $this->getConfig();
-               $logo = $conf->get( 'Logo' );
-               $logoHD = $conf->get( 'LogoHD' );
-
-               $logo1 = OutputPage::transformResourcePath( $conf, $logo );
-               $logo15 = OutputPage::transformResourcePath( $conf, $logoHD['1.5x'] );
-               $logo2 = OutputPage::transformResourcePath( $conf, $logoHD['2x'] );
-
+               $logo = $this->getLogo( $this->getConfig() );
                $styles = parent::getStyles( $context );
+
+               $default = !is_array( $logo ) ? $logo : $logo['1x'];
                $styles['all'][] = '.mw-wiki-logo { background-image: ' .
-                       CSSMin::buildUrlValue( $logo1 ) .
-                       '; }';
-               // Only 1.5x and 2x are supported
-               // Note: Keep in sync with OutputPage::addLogoPreloadLinkHeaders()
-               if ( $logoHD ) {
-                       if ( isset( $logoHD['1.5x'] ) ) {
+                               CSSMin::buildUrlValue( $default ) .
+                               '; }';
+
+               if ( is_array( $logo ) ) {
+                       if ( isset( $logo['1.5x'] ) ) {
                                $styles[
                                        '(-webkit-min-device-pixel-ratio: 1.5), ' .
                                        '(min--moz-device-pixel-ratio: 1.5), ' .
                                        '(min-resolution: 1.5dppx), ' .
                                        '(min-resolution: 144dpi)'
                                ][] = '.mw-wiki-logo { background-image: ' .
-                               CSSMin::buildUrlValue( $logo15 ) . ';' .
+                               CSSMin::buildUrlValue( $logo['1.5x'] ) . ';' .
                                'background-size: 135px auto; }';
                        }
-                       if ( isset( $logoHD['2x'] ) ) {
+                       if ( isset( $logo['2x'] ) ) {
                                $styles[
                                        '(-webkit-min-device-pixel-ratio: 2), ' .
                                        '(min--moz-device-pixel-ratio: 2),' .
                                        '(min-resolution: 2dppx), ' .
                                        '(min-resolution: 192dpi)'
                                ][] = '.mw-wiki-logo { background-image: ' .
-                               CSSMin::buildUrlValue( $logo2 ) . ';' .
+                               CSSMin::buildUrlValue( $logo['2x'] ) . ';' .
                                'background-size: 135px auto; }';
                        }
                }
+
                return $styles;
        }
 
+       /**
+        * @param Config $conf
+        * @return string|array Single url if no variants are defined
+        *  or array of logo urls keyed by dppx in form "<float>x".
+        *  Key "1x" is always defined.
+        */
+       public static function getLogo( Config $conf ) {
+               $logo = $conf->get( 'Logo' );
+               $logoHD = $conf->get( 'LogoHD' );
+
+               $logo1Url = OutputPage::transformResourcePath( $conf, $logo );
+
+               if ( !$logoHD ) {
+                       return $logo1Url;
+               }
+
+               $logoUrls = [
+                       '1x' => $logo1Url,
+               ];
+
+               // Only 1.5x and 2x are supported
+               if ( isset( $logoHD['1.5x'] ) ) {
+                       $logoUrls['1.5x'] = OutputPage::transformResourcePath(
+                               $conf,
+                               $logoHD['1.5x']
+                       );
+               }
+               if ( isset( $logoHD['2x'] ) ) {
+                       $logoUrls['2x'] = OutputPage::transformResourcePath(
+                               $conf,
+                               $logoHD['2x']
+                       );
+               }
+
+               return $logoUrls;
+       }
+
        /**
         * @param ResourceLoaderContext $context
         * @return bool
index 7571e26..9893f8c 100644 (file)
@@ -507,8 +507,12 @@ class OutputPageTest extends MediaWikiTestCase {
        /**
         * @dataProvider providePreloadLinkHeaders
         * @covers OutputPage::addLogoPreloadLinkHeaders
+        * @covers ResourceLoaderSkinModule::getLogo
         */
-       public function testPreloadLinkHeaders( $config, $result ) {
+       public function testPreloadLinkHeaders( $config, $result, $baseDir = null ) {
+               if ( $baseDir ) {
+                       $this->setMwGlobals( 'IP', $baseDir );
+               }
                $out = TestingAccessWrapper::newFromObject( $this->newInstance( $config ) );
                $out->addLogoPreloadLinkHeaders();
 
@@ -519,6 +523,7 @@ class OutputPageTest extends MediaWikiTestCase {
                return [
                        [
                                [
+                                       'ResourceBasePath' => '/w',
                                        'Logo' => '/img/default.png',
                                        'LogoHD' => [
                                                '1.5x' => '/img/one-point-five.png',
@@ -533,6 +538,7 @@ class OutputPageTest extends MediaWikiTestCase {
                        ],
                        [
                                [
+                                       'ResourceBasePath' => '/w',
                                        'Logo' => '/img/default.png',
                                        'LogoHD' => false,
                                ],
@@ -540,6 +546,7 @@ class OutputPageTest extends MediaWikiTestCase {
                        ],
                        [
                                [
+                                       'ResourceBasePath' => '/w',
                                        'Logo' => '/img/default.png',
                                        'LogoHD' => [
                                                '2x' => '/img/two-x.png',
@@ -549,6 +556,16 @@ class OutputPageTest extends MediaWikiTestCase {
                                'not all and (min-resolution: 2dppx),' .
                                '</img/two-x.png>;rel=preload;as=image;media=(min-resolution: 2dppx)'
                        ],
+                       [
+                               [
+                                       'ResourceBasePath' => '/w',
+                                       'Logo' => '/w/test.jpg',
+                                       'LogoHD' => false,
+                                       'UploadPath' => '/w/images',
+                               ],
+                               'Link: </w/test.jpg?edcf2>;rel=preload;as=image',
+                               'baseDir' => dirname( __DIR__ ) . '/data/media',
+                       ],
                ];
        }