diff --git a/app/code/Magento/ConfigurableProduct/view/frontend/templates/product/view/type/options/configurable.phtml b/app/code/Magento/ConfigurableProduct/view/frontend/templates/product/view/type/options/configurable.phtml index e75831e28cf166290fbb042d6e09705a67442b82..75967a670279fba002fbce666b9f9d440669724b 100644 --- a/app/code/Magento/ConfigurableProduct/view/frontend/templates/product/view/type/options/configurable.phtml +++ b/app/code/Magento/ConfigurableProduct/view/frontend/templates/product/view/type/options/configurable.phtml @@ -35,7 +35,8 @@ $_attributes = $block->decorateArray($block->getAllowAttributes()); "#product_addtocart_form": { "configurable": { "spConfig": <?php /* @escapeNotVerified */ echo $block->getJsonConfig() ?>, - "onlyMainImg": <?php /* @escapeNotVerified */ echo $block->getVar('change_only_base_image', 'Magento_ConfigurableProduct') ?: 'false'; ?> + "gallerySwitchStrategy": "<?php /* @escapeNotVerified */ echo $block->getVar('gallery_switch_strategy', + 'Magento_ConfigurableProduct') ?: 'replace'; ?>" } } } diff --git a/app/code/Magento/ConfigurableProduct/view/frontend/web/js/configurable.js b/app/code/Magento/ConfigurableProduct/view/frontend/web/js/configurable.js index 7bea20e78620134fff06d4cd259fee029f4fe637..59b313bcb497ddb64e1fdca9bc480afdbbb2f1d9 100644 --- a/app/code/Magento/ConfigurableProduct/view/frontend/web/js/configurable.js +++ b/app/code/Magento/ConfigurableProduct/view/frontend/web/js/configurable.js @@ -29,7 +29,16 @@ define([ mediaGallerySelector: '[data-gallery-role=gallery-placeholder]', mediaGalleryInitial: null, slyOldPriceSelector: '.sly-old-price', - onlyMainImg: false + + /** + * Defines the mechanism of how images of a gallery should be + * updated when user switches between configurations of a product. + * + * As for now value of this option can be either 'replace' or 'prepend'. + * + * @type {String} + */ + gallerySwitchStrategy: 'replace' }, /** @@ -85,10 +94,10 @@ define([ this.inputSimpleProduct = this.element.find(options.selectSimpleProduct); - gallery.on('gallery:loaded', function () { - var galleryObject = gallery.data('gallery'); - options.mediaGalleryInitial = galleryObject.returnCurrentImages(); - }); + gallery.data('gallery') ? + this._onGalleryLoaded(gallery) : + gallery.on('gallery:loaded', this._onGalleryLoaded.bind(this, gallery)); + }, /** @@ -259,46 +268,33 @@ define([ */ _changeProductImage: function () { var images, - initialImages = $.extend(true, [], this.options.mediaGalleryInitial), + initialImages = this.options.mediaGalleryInitial, galleryObject = $(this.options.mediaGallerySelector).data('gallery'); - if (this.options.spConfig.images[this.simpleProduct]) { - images = $.extend(true, [], this.options.spConfig.images[this.simpleProduct]); + if (!galleryObject) { + return; } - function updateGallery(imagesArr) { - var imgToUpdate, - mainImg; + images = this.options.spConfig.images[this.simpleProduct]; - mainImg = imagesArr.filter(function (img) { - return img.isMain; - }); + if (images) { + if (this.options.gallerySwitchStrategy === 'prepend') { + images = images.concat(initialImages); + } - imgToUpdate = mainImg.length ? mainImg[0] : imagesArr[0]; - galleryObject.updateDataByIndex(0, imgToUpdate); - galleryObject.seek(1); - } + images = $.extend(true, [], images); - if (galleryObject) { - if (images) { - images.map(function (img) { - img.type = 'image'; - }); + images.forEach(function (img) { + img.type = 'image'; + }); - if (this.options.onlyMainImg) { - updateGallery(images); - } else { - galleryObject.updateData(images) - } - } else { - if (this.options.onlyMainImg) { - updateGallery(initialImages); - } else { - galleryObject.updateData(this.options.mediaGalleryInitial); - $(this.options.mediaGallerySelector).AddFotoramaVideoEvents(); - } - } + galleryObject.updateData(images); + } else { + galleryObject.updateData(initialImages); + $(this.options.mediaGallerySelector).AddFotoramaVideoEvents(); } + + galleryObject.first(); }, /** @@ -506,8 +502,18 @@ define([ } else { $(this.options.slyOldPriceSelector).hide(); } - } + }, + /** + * Callback which fired after gallery gets initialized. + * + * @param {HTMLElement} element - DOM element associated with gallery. + */ + _onGalleryLoaded: function (element) { + var galleryObject = element.data('gallery'); + + this.options.mediaGalleryInitial = galleryObject.returnCurrentImages(); + } }); return $.mage.configurable; diff --git a/app/code/Magento/Swatches/view/frontend/templates/product/view/renderer.phtml b/app/code/Magento/Swatches/view/frontend/templates/product/view/renderer.phtml index 19419d4c0a7eef2db8fe472c80b99f9246828993..478d6da720a79bf38df1bcf0b24ed1cc51061b3c 100644 --- a/app/code/Magento/Swatches/view/frontend/templates/product/view/renderer.phtml +++ b/app/code/Magento/Swatches/view/frontend/templates/product/view/renderer.phtml @@ -16,8 +16,8 @@ "jsonSwatchConfig": <?php /* @escapeNotVerified */ echo $swatchOptions = $block->getJsonSwatchConfig(); ?>, "mediaCallback": "<?php /* @escapeNotVerified */ echo $block->getMediaCallback() ?>", - "onlyMainImg": <?php /* @escapeNotVerified */ echo $block->getVar('change_only_base_image', - 'Magento_Swatches') ?: 'false'; ?> + "gallerySwitchStrategy": "<?php /* @escapeNotVerified */ echo $block->getVar('gallery_switch_strategy', + 'Magento_ConfigurableProduct') ?: 'replace'; ?>" } } } diff --git a/app/code/Magento/Swatches/view/frontend/web/js/swatch-renderer.js b/app/code/Magento/Swatches/view/frontend/web/js/swatch-renderer.js index 452c9b6f94d3c60e83bd35a5a932c6b5f8a9092f..8af48829df438261aaed469c88245fc50a349853 100644 --- a/app/code/Magento/Swatches/view/frontend/web/js/swatch-renderer.js +++ b/app/code/Magento/Swatches/view/frontend/web/js/swatch-renderer.js @@ -243,8 +243,15 @@ define([ // Cache for BaseProduct images. Needed when option unset mediaGalleryInitial: [{}], - // - onlyMainImg: false, + /** + * Defines the mechanism of how images of a gallery should be + * updated when user switches between configurations of a product. + * + * As for now value of this option can be either 'replace' or 'prepend'. + * + * @type {String} + */ + gallerySwitchStrategy: 'replace', // whether swatches are rendered in product list or on product page inProductList: false @@ -295,11 +302,9 @@ define([ this.element.parents('.product-item-info'); if (isProductViewExist) { - gallery.on('gallery:loaded', function () { - var galleryObject = gallery.data('gallery'); - - options.mediaGalleryInitial = galleryObject.returnCurrentImages(); - }); + gallery.data('gallery') ? + this._onGalleryLoaded(gallery) : + gallery.on('gallery:loaded', this._onGalleryLoaded.bind(this, gallery)); } else { options.mediaGalleryInitial = [{ 'img': $main.find('.product-image-photo').attr('src') @@ -1032,26 +1037,27 @@ define([ */ updateBaseImage: function (images, context, isProductViewExist) { var justAnImage = images[0], - updateImg, - imagesToUpdate, + initialImages = this.options.mediaGalleryInitial, gallery = context.find(this.options.mediaGallerySelector).data('gallery'), - item; + imagesToUpdate, + isInitial; if (isProductViewExist) { imagesToUpdate = images.length ? this._setImageType($.extend(true, [], images)) : []; + isInitial = _.isEqual(imagesToUpdate, initialImages); - if (this.options.onlyMainImg) { - updateImg = imagesToUpdate.filter(function (img) { - return img.isMain; - }); - item = updateImg.length ? updateImg[0] : imagesToUpdate[0]; - gallery.updateDataByIndex(0, item); + if (this.options.gallerySwitchStrategy === 'prepend' && !isInitial) { + imagesToUpdate = imagesToUpdate.concat(initialImages); + } - gallery.seek(1); - } else { - gallery.updateData(imagesToUpdate); + gallery.updateData(imagesToUpdate); + + if (isInitial) { $(this.options.mediaGallerySelector).AddFotoramaVideoEvents(); } + + gallery.first(); + } else if (justAnImage && justAnImage.img) { context.find('.product-image-photo').attr('src', justAnImage.img); } @@ -1127,6 +1133,17 @@ define([ } return selectedAttributes; + }, + + /** + * Callback which fired after gallery gets initialized. + * + * @param {HTMLElement} element - DOM element associated with a gallery. + */ + _onGalleryLoaded: function (element) { + var galleryObject = element.data('gallery'); + + this.options.mediaGalleryInitial = galleryObject.returnCurrentImages(); } }); diff --git a/app/design/frontend/Magento/luma/etc/view.xml b/app/design/frontend/Magento/luma/etc/view.xml index e0fc864e000840ebe770a85f6a78506f0d962a2f..12a51ee065edba43c9afde39ce31c2aace3d00a8 100644 --- a/app/design/frontend/Magento/luma/etc/view.xml +++ b/app/design/frontend/Magento/luma/etc/view.xml @@ -253,10 +253,7 @@ </vars> <vars module="Magento_ConfigurableProduct"> - <var name="change_only_base_image">true</var> - </vars> - <vars module="Magento_Swatches"> - <var name="change_only_base_image">true</var> + <var name="gallery_switch_strategy">prepend</var> </vars> <vars module="Js_Bundle"> diff --git a/dev/tests/integration/testsuite/Magento/Framework/App/View/Deployment/VersionTest.php b/dev/tests/integration/testsuite/Magento/Framework/App/View/Deployment/VersionTest.php new file mode 100644 index 0000000000000000000000000000000000000000..5a313c318f9e9126ac84848a3b7ddf62e71d6585 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Framework/App/View/Deployment/VersionTest.php @@ -0,0 +1,105 @@ +<?php +/** + * Copyright © 2016 Magento. All rights reserved. + * See COPYING.txt for license details. + */ +namespace Magento\Framework\App\View\Deployment; + +use Magento\Framework\App\Filesystem\DirectoryList; +use Magento\Framework\App\ObjectManager; +use Magento\Framework\App\State; +use Magento\Framework\App\View\Deployment\Version\Storage\File; +use Magento\Framework\Filesystem\Directory\WriteInterface; + +class VersionTest extends \PHPUnit_Framework_TestCase +{ + /** + * @var File + */ + private $fileStorage; + + /** + * @var WriteInterface + */ + private $directoryWrite; + + /** + * @var string + */ + private $fileName = 'deployed_version.txt'; + + public function setUp() + { + $this->fileStorage = ObjectManager::getInstance()->create( + File::class, + [ + 'directoryCode' => DirectoryList::STATIC_VIEW, + 'fileName' => $this->fileName + ] + ); + /** @var \Magento\TestFramework\App\Filesystem $filesystem */ + $filesystem = ObjectManager::getInstance()->get(\Magento\TestFramework\App\Filesystem::class); + $this->directoryWrite = $filesystem->getDirectoryWrite(DirectoryList::STATIC_VIEW); + $this->removeDeployVersionFile(); + } + + /** + * @param string $mode + * @return Version + */ + public function getVersionModel($mode) + { + $appState = ObjectManager::getInstance()->create( + State::class, + [ + 'mode' => $mode + ] + ); + return ObjectManager::getInstance()->create( + Version::class, + [ + 'appState' => $appState + ] + ); + } + + protected function tearDown() + { + $this->removeDeployVersionFile(); + } + + private function removeDeployVersionFile() + { + if ($this->directoryWrite->isExist($this->fileName)) { + $this->directoryWrite->delete($this->fileName); + } + } + + /** + * @expectedException \UnexpectedValueException + */ + public function testGetValueInProductionModeWithoutVersion() + { + $this->assertFalse($this->directoryWrite->isExist($this->fileName)); + $this->getVersionModel(State::MODE_PRODUCTION)->getValue(); + } + + public function testGetValueInDeveloperMode() + { + $this->assertFalse($this->directoryWrite->isExist($this->fileName)); + $this->getVersionModel(State::MODE_DEVELOPER)->getValue(); + $this->assertTrue($this->directoryWrite->isExist($this->fileName)); + } + + /** + * Assert that version is not regenerated on each request in developer mode + */ + public function testGetValue() + { + $this->assertFalse($this->directoryWrite->isExist($this->fileName)); + $versionModel = $this->getVersionModel(State::MODE_DEVELOPER); + $version = $versionModel->getValue(); + $this->assertTrue($this->directoryWrite->isExist($this->fileName)); + $this->assertEquals($version, $versionModel->getValue()); + } +} diff --git a/dev/tools/grunt/configs/clean.js b/dev/tools/grunt/configs/clean.js index 53bcd8a1d830ebda056e59c6ba592106abcdde72..e720b6c40c27e45e8ca27579727ee6dec67bd400 100644 --- a/dev/tools/grunt/configs/clean.js +++ b/dev/tools/grunt/configs/clean.js @@ -21,7 +21,8 @@ _.each(themes, function(theme, name) { "<%= path.tmp %>/cache/**/*", "<%= combo.autopath(\""+name+"\", path.pub ) %>**/*", "<%= combo.autopath(\""+name+"\", path.tmpLess) %>**/*", - "<%= combo.autopath(\""+name+"\", path.tmpSource) %>**/*" + "<%= combo.autopath(\""+name+"\", path.tmpSource) %>**/*", + "<%= path.deployedVersion %>" ] } ] @@ -56,7 +57,8 @@ var cleanOptions = { "dot": true, "src": [ "<%= path.pub %>frontend/**/*", - "<%= path.pub %>adminhtml/**/*" + "<%= path.pub %>adminhtml/**/*", + "<%= path.deployedVersion %>" ] } ] @@ -73,7 +75,8 @@ var cleanOptions = { "<%= path.pub %>frontend/**/*.less", "<%= path.pub %>frontend/**/*.css", "<%= path.pub %>adminhtml/**/*.less", - "<%= path.pub %>adminhtml/**/*.css" + "<%= path.pub %>adminhtml/**/*.css", + "<%= path.deployedVersion %>" ] } ] @@ -102,7 +105,8 @@ var cleanOptions = { "src": [ "<%= path.pub %>**/*.js", "<%= path.pub %>**/*.html", - "<%= path.pub %>_requirejs/**/*" + "<%= path.pub %>_requirejs/**/*", + "<%= path.deployedVersion %>" ] } ] diff --git a/dev/tools/grunt/configs/path.js b/dev/tools/grunt/configs/path.js index 03621998c14a602c46eec390263aa8450dfc2657..e6a9cf71e81354b8e6b95e6d1623bbf799d831cf 100644 --- a/dev/tools/grunt/configs/path.js +++ b/dev/tools/grunt/configs/path.js @@ -13,6 +13,7 @@ module.exports = { tmpLess: 'var/view_preprocessed/less/', tmpSource: 'var/view_preprocessed/source/', tmp: 'var', + deployedVersion: 'pub/static/deployed_version.txt', css: { setup: 'setup/pub/styles', updater: '../magento2-updater/pub/css' diff --git a/lib/internal/Magento/Framework/App/Test/Unit/View/Deployment/Version/Storage/FileTest.php b/lib/internal/Magento/Framework/App/Test/Unit/View/Deployment/Version/Storage/FileTest.php index 3981511ad47018c8847c8c9cddf889122e059d58..e7206bf5566075f4df04a977185b5f343ed42b8f 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/View/Deployment/Version/Storage/FileTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/View/Deployment/Version/Storage/FileTest.php @@ -34,48 +34,15 @@ class FileTest extends \PHPUnit_Framework_TestCase public function testLoad() { - $this->directory - ->expects($this->once()) - ->method('readFile') + $this->directory->expects($this->once()) + ->method('isReadable') ->with('fixture_file.txt') - ->will($this->returnValue('123')); - $this->assertEquals('123', $this->object->load()); - } - - /** - * @expectedException \Exception - * @expectedExceptionMessage Exception to be propagated - */ - public function testLoadExceptionPropagation() - { - $this->directory - ->expects($this->once()) + ->willReturn(true); + $this->directory->expects($this->once()) ->method('readFile') ->with('fixture_file.txt') - ->will($this->throwException(new \Exception('Exception to be propagated'))); - $this->object->load(); - } - - /** - * @expectedException \UnexpectedValueException - * @expectedExceptionMessage Unable to retrieve deployment version of static files from the file system - */ - public function testLoadExceptionWrapping() - { - $filesystemException = new \Magento\Framework\Exception\FileSystemException( - new \Magento\Framework\Phrase('File does not exist') - ); - $this->directory - ->expects($this->once()) - ->method('readFile') - ->with('fixture_file.txt') - ->will($this->throwException($filesystemException)); - try { - $this->object->load(); - } catch (\Exception $e) { - $this->assertSame($filesystemException, $e->getPrevious(), 'Wrapping of original exception is expected'); - throw $e; - } + ->willReturn('123'); + $this->assertEquals('123', $this->object->load()); } public function testSave() diff --git a/lib/internal/Magento/Framework/App/Test/Unit/View/Deployment/VersionTest.php b/lib/internal/Magento/Framework/App/Test/Unit/View/Deployment/VersionTest.php index 187c043945d059992d6dc59a8713fc413b1a9752..8d804102f7a56038fded4118692931030593caa7 100644 --- a/lib/internal/Magento/Framework/App/Test/Unit/View/Deployment/VersionTest.php +++ b/lib/internal/Magento/Framework/App/Test/Unit/View/Deployment/VersionTest.php @@ -6,7 +6,6 @@ namespace Magento\Framework\App\Test\Unit\View\Deployment; use Magento\Framework\App\View\Deployment\Version; -use Magento\Framework\Exception\FileSystemException; /** * Class VersionTest @@ -45,17 +44,6 @@ class VersionTest extends \PHPUnit_Framework_TestCase $objectManager->setBackwardCompatibleProperty($this->object, 'logger', $this->loggerMock); } - public function testGetValueDeveloperMode() - { - $this->appStateMock - ->expects($this->once()) - ->method('getMode') - ->will($this->returnValue(\Magento\Framework\App\State::MODE_DEVELOPER)); - $this->versionStorageMock->expects($this->never())->method($this->anything()); - $this->assertInternalType('integer', $this->object->getValue()); - $this->object->getValue(); // Ensure computation occurs only once and result is cached in memory - } - /** * @param string $appMode * @dataProvider getValueFromStorageDataProvider @@ -81,106 +69,46 @@ class VersionTest extends \PHPUnit_Framework_TestCase ]; } - /** - * $param bool $isUnexpectedValueExceptionThrown - * $param bool $isFileSystemExceptionThrown - * @dataProvider getValueDefaultModeDataProvider - */ - public function testGetValueDefaultMode( - $isUnexpectedValueExceptionThrown, - $isFileSystemExceptionThrown = null - ) { - $versionType = 'integer'; - $this->appStateMock - ->expects($this->once()) - ->method('getMode') - ->willReturn(\Magento\Framework\App\State::MODE_DEFAULT); - if ($isUnexpectedValueExceptionThrown) { - $storageException = new \UnexpectedValueException('Does not exist in the storage'); - $this->versionStorageMock - ->expects($this->once()) - ->method('load') - ->will($this->throwException($storageException)); - $this->versionStorageMock->expects($this->once()) - ->method('save') - ->with($this->isType($versionType)); - if ($isFileSystemExceptionThrown) { - $fileSystemException = new FileSystemException( - new \Magento\Framework\Phrase('Can not load static content version') - ); - $this->versionStorageMock - ->expects($this->once()) - ->method('save') - ->will($this->throwException($fileSystemException)); - $this->loggerMock->expects($this->once()) - ->method('critical') - ->with('Can not save static content version.'); - } else { - $this->loggerMock->expects($this->never()) - ->method('critical'); - } - } else { - $this->versionStorageMock - ->expects($this->once()) - ->method('load') - ->willReturn(1475779229); - $this->loggerMock->expects($this->never()) - ->method('critical'); - } - $this->assertInternalType($versionType, $this->object->getValue()); + public function testGetValueInNonProductionMode() + { + $version = 123123123123; + $this->versionStorageMock->expects($this->once()) + ->method('load') + ->willReturn($version); + + $this->assertEquals($version, $this->object->getValue()); $this->object->getValue(); } /** - * @return array + * @expectedException \UnexpectedValueException */ - public function getValueDefaultModeDataProvider() + public function testGetValueWithProductionModeAndException() { - return [ - [false], - [true, false], - [true, true] - ]; - } - - /** - * @param bool $isUnexpectedValueExceptionThrown - * @dataProvider getValueProductionModeDataProvider - */ - public function testGetValueProductionMode( - $isUnexpectedValueExceptionThrown - ) { - $this->appStateMock - ->expects($this->once()) + $this->versionStorageMock->expects($this->once()) + ->method('load') + ->willReturn(false); + $this->appStateMock->expects($this->once()) ->method('getMode') ->willReturn(\Magento\Framework\App\State::MODE_PRODUCTION); - if ($isUnexpectedValueExceptionThrown) { - $storageException = new \UnexpectedValueException('Does not exist in the storage'); - $this->versionStorageMock - ->expects($this->once()) - ->method('load') - ->will($this->throwException($storageException)); - $this->loggerMock->expects($this->once()) - ->method('critical') - ->with('Can not load static content version.'); - } else { - $this->versionStorageMock - ->expects($this->once()) - ->method('load') - ->willReturn(1475779229); - } - $this->assertInternalType('integer', $this->object->getValue()); + $this->loggerMock->expects($this->once()) + ->method('critical') + ->with('Can not load static content version.'); + $this->object->getValue(); } - /** - * @return array - */ - public function getValueProductionModeDataProvider() + public function testGetValueWithProductionMode() { - return [ - [false], - [true], - ]; + $this->versionStorageMock->expects($this->once()) + ->method('load') + ->willReturn(false); + $this->appStateMock->expects($this->once()) + ->method('getMode') + ->willReturn(\Magento\Framework\App\State::MODE_DEFAULT); + $this->versionStorageMock->expects($this->once()) + ->method('save'); + + $this->assertNotNull($this->object->getValue()); } } diff --git a/lib/internal/Magento/Framework/App/View/Deployment/Version.php b/lib/internal/Magento/Framework/App/View/Deployment/Version.php index 7f6dc7fa9285ccc3987d84091bc9edb76d09f794..73d4a0c926cea88c5b2cfbd6db34888f3ef651fb 100644 --- a/lib/internal/Magento/Framework/App/View/Deployment/Version.php +++ b/lib/internal/Magento/Framework/App/View/Deployment/Version.php @@ -7,7 +7,6 @@ namespace Magento\Framework\App\View\Deployment; use Psr\Log\LoggerInterface; -use Magento\Framework\Exception\FileSystemException; /** * Deployment version of static files @@ -67,23 +66,16 @@ class Version */ protected function readValue($appMode) { - if ($appMode == \Magento\Framework\App\State::MODE_DEVELOPER) { - $result = $this->generateVersion(); - } else { - try { - $result = $this->versionStorage->load(); - } catch (\UnexpectedValueException $e) { - $result = $this->generateVersion(); - if ($appMode == \Magento\Framework\App\State::MODE_DEFAULT) { - try { - $this->versionStorage->save($result); - } catch (FileSystemException $e) { - $this->getLogger()->critical('Can not save static content version.'); - } - } else { - $this->getLogger()->critical('Can not load static content version.'); - } + $result = $this->versionStorage->load(); + if (!$result) { + if ($appMode == \Magento\Framework\App\State::MODE_PRODUCTION) { + $this->getLogger()->critical('Can not load static content version.'); + throw new \UnexpectedValueException( + "Unable to retrieve deployment version of static files from the file system." + ); } + $result = $this->generateVersion(); + $this->versionStorage->save($result); } return $result; } diff --git a/lib/internal/Magento/Framework/App/View/Deployment/Version/Storage/File.php b/lib/internal/Magento/Framework/App/View/Deployment/Version/Storage/File.php index 4f8813df774d7edc9a7490e31f447e6827dd2e38..0967cb634cbd706689bc0d54aa2c9a1b5b7a8b5e 100644 --- a/lib/internal/Magento/Framework/App/View/Deployment/Version/Storage/File.php +++ b/lib/internal/Magento/Framework/App/View/Deployment/Version/Storage/File.php @@ -40,15 +40,10 @@ class File implements \Magento\Framework\App\View\Deployment\Version\StorageInte */ public function load() { - try { + if ($this->directory->isReadable($this->fileName)) { return $this->directory->readFile($this->fileName); - } catch (\Magento\Framework\Exception\FileSystemException $e) { - throw new \UnexpectedValueException( - 'Unable to retrieve deployment version of static files from the file system.', - 0, - $e - ); } + return false; } /** diff --git a/lib/internal/Magento/Framework/Css/PreProcessor/Instruction/Import.php b/lib/internal/Magento/Framework/Css/PreProcessor/Instruction/Import.php index 524ccc8c11fe311837d3390a8f9ec10ee890d375..260f6216b09bd1dd03fdee39f5102c1a717df32e 100644 --- a/lib/internal/Magento/Framework/Css/PreProcessor/Instruction/Import.php +++ b/lib/internal/Magento/Framework/Css/PreProcessor/Instruction/Import.php @@ -4,8 +4,6 @@ * See COPYING.txt for license details. */ -// @codingStandardsIgnoreFile - namespace Magento\Framework\Css\PreProcessor\Instruction; use Magento\Framework\View\Asset\LocalInterface; @@ -22,7 +20,10 @@ class Import implements PreProcessorInterface * Pattern of @import instruction */ const REPLACE_PATTERN = - '#@import\s+(\((?P<type>\w+)\)\s+)?[\'\"](?P<path>(?![/\\\]|\w:[/\\\])[^\"\']+)[\'\"]\s*?(?P<media>.*?);#'; + '#@import(?!.*?\surl\(.*?)' + . '(?P<start>[\(\),\w\s]*?[\'\"])' + . '(?P<path>(?![/\\\]|\w*?:[/\\\])[^\"\']+)' + . '(?P<end>[\'\"][\s\w\(\)]*?);#'; /** * @var \Magento\Framework\View\Asset\NotationResolver\Module @@ -133,9 +134,9 @@ class Import implements PreProcessorInterface $matchedFileId = $this->fixFileExtension($matchedContent['path'], $contentType); $this->recordRelatedFile($matchedFileId, $asset); $resolvedPath = $this->notationResolver->convertModuleNotationToPath($asset, $matchedFileId); - $typeString = empty($matchedContent['type']) ? '' : '(' . $matchedContent['type'] . ') '; - $mediaString = empty($matchedContent['media']) ? '' : ' ' . trim($matchedContent['media']); - return "@import {$typeString}'{$resolvedPath}'{$mediaString};"; + $start = $matchedContent['start']; + $end = $matchedContent['end']; + return "@import{$start}{$resolvedPath}{$end};"; } /** diff --git a/lib/internal/Magento/Framework/Css/Test/Unit/PreProcessor/Instruction/ImportTest.php b/lib/internal/Magento/Framework/Css/Test/Unit/PreProcessor/Instruction/ImportTest.php index 2441422bb182103100c3181bcfae4150423ef0b3..dc45ea75e0eb86f13771c7108a959376caad54f3 100644 --- a/lib/internal/Magento/Framework/Css/Test/Unit/PreProcessor/Instruction/ImportTest.php +++ b/lib/internal/Magento/Framework/Css/Test/Unit/PreProcessor/Instruction/ImportTest.php @@ -39,7 +39,7 @@ class ImportTest extends \PHPUnit_Framework_TestCase protected function setUp() { - $this->notationResolver = $this->getMock( + $this->notationResolver = $this->getMock( \Magento\Framework\View\Asset\NotationResolver\Module::class, [], [], '', false ); $this->asset = $this->getMock(\Magento\Framework\View\Asset\File::class, [], [], '', false); @@ -63,7 +63,11 @@ class ImportTest extends \PHPUnit_Framework_TestCase public function testProcess($originalContent, $foundPath, $resolvedPath, $expectedContent) { $chain = new \Magento\Framework\View\Asset\PreProcessor\Chain($this->asset, $originalContent, 'less', 'path'); - $this->notationResolver->expects($this->once()) + $invoke = $this->once(); + if (preg_match('/^(http:|https:|\/+)/', $foundPath)) { + $invoke = $this->never(); + } + $this->notationResolver->expects($invoke) ->method('convertModuleNotationToPath') ->with($this->asset, $foundPath) ->will($this->returnValue($resolvedPath)); @@ -78,50 +82,70 @@ class ImportTest extends \PHPUnit_Framework_TestCase public function processDataProvider() { return [ - 'non-modular notation' => [ - '@import (type) "some/file.css" media;', - 'some/file.css', - 'some/file.css', - "@import (type) 'some/file.css' media;", + 'non-modular notation, no extension' => [ + '@import (type) \'some/file\' media;', + 'some/file.less', + 'some/file.less', + '@import (type) \'some/file.less\' media;', ], 'modular, with extension' => [ '@import (type) "Magento_Module::something.css" media;', 'Magento_Module::something.css', 'Magento_Module/something.css', - "@import (type) 'Magento_Module/something.css' media;", + '@import (type) "Magento_Module/something.css" media;', + ], + 'remote file import url()' => [ + '@import (type) url("http://example.com/css/some.css") media;', + 'http://example.com/css/some.css', + null, + '@import (type) url("http://example.com/css/some.css") media;', + ], + 'invalid path' => [ + '@import (type) url("/example.com/css/some.css") media;', + '/example.com/css/some.css', + null, + '@import (type) url("/example.com/css/some.css") media;', ], 'modular, no extension' => [ '@import (type) "Magento_Module::something" media;', 'Magento_Module::something.less', 'Magento_Module/something.less', - "@import (type) 'Magento_Module/something.less' media;", + '@import (type) "Magento_Module/something.less" media;', ], 'no type' => [ '@import "Magento_Module::something.css" media;', 'Magento_Module::something.css', 'Magento_Module/something.css', - "@import 'Magento_Module/something.css' media;", + '@import "Magento_Module/something.css" media;', ], 'no media' => [ '@import (type) "Magento_Module::something.css";', 'Magento_Module::something.css', 'Magento_Module/something.css', - "@import (type) 'Magento_Module/something.css';", + '@import (type) "Magento_Module/something.css";', + ], + 'with single line comment, replace' => [ + '@import (type) "some/file" media;' . PHP_EOL + . '// @import (type) "unnecessary/file.css" media;', + 'some/file.less', + 'some/file.less', + '@import (type) "some/file.less" media;' . PHP_EOL, ], - 'with single line comment' => [ - '@import (type) "some/file.css" media;' . PHP_EOL - . '// @import (type) "unnecessary/file.css" media;', - 'some/file.css', - 'some/file.css', - "@import (type) 'some/file.css' media;" . PHP_EOL, + 'with single line comment, no replace' => [ + '@import (type) "some/file.less" media;' . PHP_EOL + . '// @import (type) "unnecessary/file" media;', + 'some/file.less', + 'some/file.less', + '@import (type) "some/file.less" media;' . PHP_EOL + . '// @import (type) "unnecessary/file" media;', ], 'with multi line comment' => [ - '@import (type) "some/file.css" media;' . PHP_EOL + '@import (type) "some/file" media;' . PHP_EOL . '/* @import (type) "unnecessary/file.css" media;' . PHP_EOL . '@import (type) "another/unnecessary/file.css" media; */', - 'some/file.css', - 'some/file.css', - "@import (type) 'some/file.css' media;" . PHP_EOL, + 'some/file.less', + 'some/file.less', + '@import (type) "some/file.less" media;' . PHP_EOL, ], ]; }