[BUGFIX] CSS concatenation needs to care about media types 98/19398/14
authorGeorg Ringer <georg.ringer@gmail.com>
Sun, 9 Nov 2014 20:38:48 +0000 (21:38 +0100)
committerJigal van Hemert <jigal.van.hemert@typo3.org>
Sun, 19 Jul 2015 13:56:18 +0000 (15:56 +0200)
Previously only files with media="all" have been concatenated but there
is no reason why not the others should be concatenated too.
The forceOnTop option now works for each media type.

Change-Id: Ie29e011f94d36d73b946920965a95388e46d544b
Fixes: #33137
Releases: master
Reviewed-on: http://review.typo3.org/19398
Reviewed-by: Jigal van Hemert <jigal.van.hemert@typo3.org>
Tested-by: Jigal van Hemert <jigal.van.hemert@typo3.org>
typo3/sysext/core/Classes/Resource/ResourceCompressor.php
typo3/sysext/core/Tests/Unit/Resource/ResourceCompressorTest.php

index a353095..6cd700b 100644 (file)
@@ -181,7 +181,7 @@ class ResourceCompressor {
         * @return array CSS files
         */
        public function concatenateCssFiles(array $cssFiles, array $options = array()) {
-               $filesToInclude = array();
+               $filesToIncludeByType = array('all' => array());
                foreach ($cssFiles as $key => $fileOptions) {
                        // no concatenation allowed for this file, so continue
                        if (!empty($fileOptions['excludeFromConcatenation'])) {
@@ -190,30 +190,45 @@ class ResourceCompressor {
                        // we remove BACK_PATH from $filename, so make it relative to root path
                        $filenameFromMainDir = $this->getFilenameFromMainDir($fileOptions['file']);
                        // if $options['baseDirectories'] set, we only include files below these directories
-                       if ((!isset($options['baseDirectories']) || $this->checkBaseDirectory($filenameFromMainDir, array_merge($options['baseDirectories'], array($this->targetDirectory)))) && $fileOptions['media'] === 'all') {
+                       if (
+                               !isset($options['baseDirectories'])
+                               || $this->checkBaseDirectory(
+                                       $filenameFromMainDir, array_merge($options['baseDirectories'], array($this->targetDirectory))
+                               )
+                       ) {
+
+                               $type = isset($fileOptions['media']) ? strtolower($fileOptions['media']) : 'all';
+                               if (!isset($filesToIncludeByType[$type])) {
+                                       $filesToIncludeByType[$type] = array();
+                               }
                                if ($fileOptions['forceOnTop']) {
-                                       array_unshift($filesToInclude, $filenameFromMainDir);
+                                       array_unshift($filesToIncludeByType[$type], $filenameFromMainDir);
                                } else {
-                                       $filesToInclude[] = $filenameFromMainDir;
+                                       $filesToIncludeByType[$type][] = $filenameFromMainDir;
                                }
                                // remove the file from the incoming file array
                                unset($cssFiles[$key]);
                        }
                }
-               if (!empty($filesToInclude)) {
-                       $targetFile = $this->createMergedCssFile($filesToInclude);
-                       $targetFileRelative = $this->relativePath . $targetFile;
-                       $concatenatedOptions = array(
-                               'file' => $targetFileRelative,
-                               'rel' => 'stylesheet',
-                               'media' => 'all',
-                               'compress' => TRUE,
-                               'excludeFromConcatenation' => TRUE,
-                               'forceOnTop' => FALSE,
-                               'allWrap' => ''
-                       );
-                       // place the merged stylesheet on top of the stylesheets
-                       $cssFiles = array_merge(array($targetFileRelative => $concatenatedOptions), $cssFiles);
+               if (!empty($filesToIncludeByType)) {
+                       foreach ($filesToIncludeByType as $mediaOption => $filesToInclude) {
+                               if (empty($filesToInclude)) {
+                                       continue;
+                               }
+                               $targetFile = $this->createMergedCssFile($filesToInclude);
+                               $targetFileRelative = $this->relativePath . $targetFile;
+                               $concatenatedOptions = array(
+                                       'file' => $targetFileRelative,
+                                       'rel' => 'stylesheet',
+                                       'media' => $mediaOption,
+                                       'compress' => TRUE,
+                                       'excludeFromConcatenation' => TRUE,
+                                       'forceOnTop' => FALSE,
+                                       'allWrap' => ''
+                               );
+                               // place the merged stylesheet on top of the stylesheets
+                               $cssFiles = array_merge($cssFiles, array($targetFileRelative => $concatenatedOptions));
+                       }
                }
                return $cssFiles;
        }
index 846a987..1aebe1d 100644 (file)
@@ -155,6 +155,124 @@ class ResourceCompressorTest extends BaseTestCase {
        /**
         * @test
         */
+       public function concatenatedCssFilesAreSeparatedByMediaType() {
+               $allFileName = 'allFile.css';
+               $screenFileName1 = 'screenFile.css';
+               $screenFileName2 = 'screenFile2.css';
+               $testFileFixture = array(
+                       $allFileName => array(
+                               'file' => $allFileName,
+                               'excludeFromConcatenation' => FALSE,
+                               'media' => 'all',
+                       ),
+                       // use two screen files to check if they are merged into one, even with a different media type
+                       $screenFileName1 => array(
+                               'file' => $screenFileName1,
+                               'excludeFromConcatenation' => FALSE,
+                               'media' => 'screen',
+                       ),
+                       $screenFileName2 => array(
+                               'file' => $screenFileName2,
+                               'excludeFromConcatenation' => FALSE,
+                               'media' => 'screen',
+                       ),
+               );
+               $this->subject->expects($this->exactly(2))
+                       ->method('createMergedCssFile')
+                       ->will($this->onConsecutiveCalls(
+                               $this->returnValue('merged_' . $allFileName),
+                               $this->returnValue('merged_' . $screenFileName1)
+                       ));
+               $this->subject->setRelativePath('');
+
+               $result = $this->subject->concatenateCssFiles($testFileFixture);
+
+               $this->assertEquals(array(
+                       'merged_' . $allFileName,
+                       'merged_' . $screenFileName1
+               ), array_keys($result));
+               $this->assertEquals('all', $result['merged_' . $allFileName]['media']);
+               $this->assertEquals('screen', $result['merged_' . $screenFileName1]['media']);
+       }
+
+       /**
+        * @test
+        */
+       public function concatenatedCssFilesObeyForceOnTopOption() {
+               $screen1FileName = 'screen1File.css';
+               $screen2FileName = 'screen2File.css';
+               $screen3FileName = 'screen3File.css';
+               $testFileFixture = array(
+                       $screen1FileName => array(
+                               'file' => $screen1FileName,
+                               'excludeFromConcatenation' => FALSE,
+                               'media' => 'screen',
+                       ),
+                       $screen2FileName => array(
+                               'file' => $screen2FileName,
+                               'excludeFromConcatenation' => FALSE,
+                               'media' => 'screen',
+                       ),
+                       $screen3FileName => array(
+                               'file' => $screen3FileName,
+                               'excludeFromConcatenation' => FALSE,
+                               'forceOnTop' => TRUE,
+                               'media' => 'screen',
+                       ),
+               );
+               // Replace mocked method getFilenameFromMainDir by passthrough callback
+               $this->subject->expects($this->any())->method('getFilenameFromMainDir')->willReturnArgument(0);
+               $this->subject->expects($this->once())
+                       ->method('createMergedCssFile')
+                       ->with($this->equalTo(array($screen3FileName, $screen1FileName, $screen2FileName)));
+               $this->subject->setRelativePath('');
+
+               $this->subject->concatenateCssFiles($testFileFixture);
+       }
+
+       /**
+        * @test
+        */
+       public function concatenatedCssFilesObeyExcludeFromConcatenation() {
+               $screen1FileName = 'screen1File.css';
+               $screen2FileName = 'screen2File.css';
+               $screen3FileName = 'screen3File.css';
+               $testFileFixture = array(
+                       $screen1FileName => array(
+                               'file' => $screen1FileName,
+                               'excludeFromConcatenation' => FALSE,
+                               'media' => 'screen',
+                       ),
+                       $screen2FileName => array(
+                               'file' => $screen2FileName,
+                               'excludeFromConcatenation' => TRUE,
+                               'media' => 'screen',
+                       ),
+                       $screen3FileName => array(
+                               'file' => $screen3FileName,
+                               'excludeFromConcatenation' => FALSE,
+                               'media' => 'screen',
+                       ),
+               );
+               $this->subject->expects($this->any())->method('getFilenameFromMainDir')->willReturnArgument(0);
+               $this->subject->expects($this->once())
+                       ->method('createMergedCssFile')
+                       ->with($this->equalTo(array($screen1FileName, $screen3FileName)))
+                       ->will($this->returnValue('merged_screen'));
+               $this->subject->setRelativePath('');
+
+               $result = $this->subject->concatenateCssFiles($testFileFixture);
+               $this->assertEquals(array(
+                       $screen2FileName,
+                       'merged_screen'
+               ), array_keys($result));
+               $this->assertEquals('screen', $result[$screen2FileName]['media']);
+               $this->assertEquals('screen', $result['merged_screen']['media']);
+       }
+
+       /**
+        * @test
+        */
        public function concatenatedJsFileIsFlaggedToNotConcatenateAgain() {
                $fileName = 'fooFile.js';
                $concatenatedFileName = 'merged_' . $fileName;