diff --git a/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php index 799ee07..fa74923 100644 --- a/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php +++ b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php @@ -39,6 +39,26 @@ class LibraryDiscoveryParser { protected $root; /** + * The list of asset paths / sources that have libraries-override applied. + * + * @var array + */ + protected $overriddenLibraryPaths = []; + + /** + * Used to retrieve original libraries-override keys from base theme paths. + * + * This is used to maintain backward compatibility with previous + * libraries-override behavior where overriding of already overridden paths + * required using the full Drupal-root-relative path of the last override. + * + * @todo Remove BC layer in Drupal 9.0.0 https://www.drupal.org/node/2852314. + * + * @var array + */ + protected $originalLibraryPaths = []; + + /** * Constructs a new LibraryDiscoveryParser instance. * * @param string $root @@ -143,6 +163,14 @@ public function buildByExtension($extension) { } foreach ($library[$type] as $source => $options) { unset($library[$type][$source]); + // Get the actual overridden path to $source in libraries-override. + $source = $this->getLatestOverridePath($source, $type, $id, $extension); + if (!$source) { + // If $source is false, it means that a libraries-override entry in + // the theme or base theme has specified that the library should be + // removed. Hence this entry should not be processed. + continue; + } // Allow to omit the options hashmap in YAML declarations. if (!is_array($options)) { $options = array(); @@ -335,8 +363,8 @@ protected function applyLibrariesOverride($libraries, $extension) { // ActiveTheme::getLibrariesOverride() returns libraries-overrides for the // current theme as well as all its base themes. $all_libraries_overrides = $active_theme->getLibrariesOverride(); - foreach ($all_libraries_overrides as $theme_path => $libraries_overrides) { - foreach ($libraries as $library_name => $library) { + foreach ($libraries as $library_name => $library) { + foreach ($all_libraries_overrides as $theme_path => $libraries_overrides) { // Process libraries overrides. if (isset($libraries_overrides["$extension/$library_name"])) { // Active theme defines an override for this library. @@ -356,23 +384,23 @@ protected function applyLibrariesOverride($libraries, $extension) { elseif (is_array($override_definition)) { // An array definition implies an override for an asset within this // library. - foreach ($override_definition as $sub_key => $value) { + foreach ($override_definition as $type => $value) { // Throw an exception if the asset is not properly specified. if (!is_array($value)) { - throw new InvalidLibrariesOverrideSpecificationException(sprintf('Library asset %s is not correctly specified. It should be in the form "extension/library_name/sub_key/path/to/asset.js".', "$extension/$library_name/$sub_key")); + throw new InvalidLibrariesOverrideSpecificationException(sprintf('Library asset %s is not correctly specified. It should be in the form "extension/library_name/sub_key/path/to/asset.js".', "$extension/$library_name/$type")); } - if ($sub_key === 'drupalSettings') { + if ($type === 'drupalSettings') { // drupalSettings may not be overridden. - throw new InvalidLibrariesOverrideSpecificationException(sprintf('drupalSettings may not be overridden in libraries-override. Trying to override %s. Use hook_library_info_alter() instead.', "$extension/$library_name/$sub_key")); + throw new InvalidLibrariesOverrideSpecificationException(sprintf('drupalSettings may not be overridden in libraries-override. Trying to override %s. Use hook_library_info_alter() instead.', "$extension/$library_name/$type")); } - elseif ($sub_key === 'css') { + elseif ($type === 'css') { // SMACSS category should be incorporated into the asset name. foreach ($value as $category => $overrides) { - $this->setOverrideValue($libraries[$library_name], [$sub_key, $category], $overrides, $theme_path); + $this->setOverrideValue($overrides, $type, $library_name, $extension, $theme_path); } } else { - $this->setOverrideValue($libraries[$library_name], [$sub_key], $value, $theme_path); + $this->setOverrideValue($value, $type, $library_name, $extension, $theme_path); } } } @@ -405,37 +433,38 @@ protected function isValidUri($string) { } /** - * Overrides the specified library asset. + * Stores the specified library-overrides for later implementation. * - * @param array $library - * The containing library definition. - * @param array $sub_key - * An array containing the sub-keys specifying the library asset, e.g. - * @code['js']@endcode or @code['css', 'component']@endcode * @param array $overrides * Specifies the overrides, this is an array where the key is the asset to * be overridden while the value is overriding asset. + * @param string $type + * The type of library asset, e.g. 'js' or 'css'. + * @param string $library_name + * The name of the library being overridden. + * @param string $extension + * The extension name. + * @param string $theme_path + * The path to the theme defining the libraries-override. */ - protected function setOverrideValue(array &$library, array $sub_key, array $overrides, $theme_path) { + protected function setOverrideValue(array $overrides, $type, $library_name, $extension, $theme_path) { foreach ($overrides as $original => $replacement) { - // Get the attributes of the asset to be overridden. If the key does - // not exist, then throw an exception. - $key_exists = NULL; - $parents = array_merge($sub_key, [$original]); - // Save the attributes of the library asset to be overridden. - $attributes = NestedArray::getValue($library, $parents, $key_exists); - if ($key_exists) { - // Remove asset to be overridden. - NestedArray::unsetValue($library, $parents); - // No need to replace if FALSE is specified, since that is a removal. - if ($replacement) { - // Ensure the replacement path is relative to drupal root. - $replacement = $this->resolveThemeAssetPath($theme_path, $replacement); - $new_parents = array_merge($sub_key, [$replacement]); - // Replace with an override if specified. - NestedArray::setValue($library, $new_parents, $attributes); - } + // $original can either be the last overridden path or the original + // library definition path. + + // For BC we check if $original still refers to base-theme overrides. + // @todo Remove BC in Drupal 9.0.0 https://www.drupal.org/node/2852314. + if (isset($this->originalLibraryPaths[$original])) { + // Here $original actually refers to an overridden path (maybe from a + // base theme), so get the actual original path by which the library + // asset was keyed. + $original = $this->originalLibraryPaths[$original]; + } + if ($replacement) { + $this->originalLibraryPaths[$this->resolveThemeAssetPath($theme_path, $replacement)] = $original; } + + $this->setLatestOverridePath($this->resolveThemeAssetPath($theme_path, $replacement), $original, $type, $library_name, $extension); } } @@ -444,13 +473,17 @@ protected function setOverrideValue(array &$library, array $sub_key, array $over * * @param string $theme_path * The theme or base theme. - * @param string $overriding_asset - * The overriding library asset. + * @param string|false $overriding_asset + * The overriding library asset or false for asset removal. * - * @return string - * A fully resolved theme asset path relative to the Drupal directory. + * @return string|false + * A fully resolved theme asset path relative to the Drupal directory or + * false if the asset is to be removed. */ protected function resolveThemeAssetPath($theme_path, $overriding_asset) { + if ($overriding_asset == FALSE) { + return FALSE; + } if ($overriding_asset[0] !== '/' && !$this->isValidUri($overriding_asset)) { // The destination is not an absolute path and it's not a URI (e.g. // public://generated_js/example.js or http://example.com/js/my_js.js), so @@ -460,4 +493,47 @@ protected function resolveThemeAssetPath($theme_path, $overriding_asset) { return $overriding_asset; } + /** + * Gets the libraries-override path that applies to the library asset. + * + * @param string $original_path + * The original library asset source path. + * @param string $type + * The type of library asset, e.g. 'js' or 'css'. + * @param string $library_name + * The library name or ID string. + * @param string $extension + * The extension name. + * + * @return string|false + * The full path to the effective libraries-override asset or $original_path + * if the asset is not overridden. Returns false if the asset is to be + * removed. + */ + protected function getLatestOverridePath($original_path, $type, $library_name, $extension) { + $key = $extension . ':' . $library_name . ':' . $type . ':' . $original_path; + return isset($this->overriddenLibraryPaths[$key]) + ? $this->overriddenLibraryPaths[$key] + : $original_path; + } + + /** + * Sets the libraries-override path that applies to the library asset. + * + * @param string|false $new_path + * The Drupal-root-relative path to the overriding library asset or false if + * the asset is to be removed. + * @param string $original_path + * The original library asset source path. + * @param string $type + * The type of library asset, e.g. 'js' or 'css'. + * @param string $library_name + * The library name or ID string. + * @param string $extension + * The extension name. + */ + protected function setLatestOverridePath($new_path, $original_path, $type, $library_name, $extension) { + $this->overriddenLibraryPaths[$extension . ':' . $library_name . ':' . $type . ':' . $original_path] = $new_path; + } + } diff --git a/core/modules/system/tests/themes/test_basetheme/test_basetheme.info.yml b/core/modules/system/tests/themes/test_basetheme/test_basetheme.info.yml index 3efb45f..6c621ff 100644 --- a/core/modules/system/tests/themes/test_basetheme/test_basetheme.info.yml +++ b/core/modules/system/tests/themes/test_basetheme/test_basetheme.info.yml @@ -18,6 +18,10 @@ libraries-override: css: component: assets/vendor/farbtastic/farbtastic.css: css/farbtastic.css + core/normalize: + css: + base: + assets/vendor/normalize-css/normalize.css: css/normalize.css libraries-extend: classy/base: diff --git a/core/modules/system/tests/themes/test_subsubtheme/test_subsubtheme.info.yml b/core/modules/system/tests/themes/test_subsubtheme/test_subsubtheme.info.yml index 7bf373a..4f38ca4 100644 --- a/core/modules/system/tests/themes/test_subsubtheme/test_subsubtheme.info.yml +++ b/core/modules/system/tests/themes/test_subsubtheme/test_subsubtheme.info.yml @@ -4,3 +4,17 @@ description: 'Test theme which uses test_subtheme as the base theme.' version: VERSION core: 8.x base theme: test_subtheme + +libraries-override: + core/drupal.dialog: + js: + misc/dialog/dialog.js: js/subsubtheme-dialog.js + core/jquery.farbtastic: + css: + component: + # Tests previous behavior of using the last overridden path. + /core/modules/system/tests/themes/test_basetheme/css/farbtastic.css: css/subsubtheme-farbtastic.css + core/normalize: + css: + base: + assets/vendor/normalize-css/normalize.css: false diff --git a/core/tests/Drupal/KernelTests/Core/Asset/LibraryDiscoveryIntegrationTest.php b/core/tests/Drupal/KernelTests/Core/Asset/LibraryDiscoveryIntegrationTest.php index a77adfc..5288d6f 100644 --- a/core/tests/Drupal/KernelTests/Core/Asset/LibraryDiscoveryIntegrationTest.php +++ b/core/tests/Drupal/KernelTests/Core/Asset/LibraryDiscoveryIntegrationTest.php @@ -161,6 +161,28 @@ public function testBaseThemeLibrariesOverrideInSubTheme() { } /** + * Tests library-override on already overridden libraries. + */ + public function testLibrariesOverrideOverridden() { + // Activate a sub-theme that doesn't override a library override. + $this->activateTheme('test_subtheme'); + + // Assert that effective libraries override is from the test_subsubtheme. + $this->assertAssetInLibrary('core/modules/system/tests/themes/test_basetheme/css/farbtastic.css', 'core', 'jquery.farbtastic', 'css'); + $this->assertAssetInLibrary('core/modules/system/tests/themes/test_basetheme/css/normalize.css', 'core', 'normalize', 'css'); + + // Activate a sub-theme that overrides a library override. + $this->activateTheme('test_subsubtheme'); + + // Assert that effective libraries override is from the test_subsubtheme. + $this->assertNoAssetInLibrary('core/modules/system/tests/themes/test_basetheme/css/farbtastic.css', 'core', 'jquery.farbtastic', 'css'); + $this->assertNoAssetInLibrary('core/modules/system/tests/themes/test_basetheme/css/normalize.css', 'core', 'normalize', 'css'); + $this->assertNoAssetInLibrary('core/assets/vendor/normalize-css/normalize.css', 'core', 'normalize', 'css'); + $this->assertAssetInLibrary('core/modules/system/tests/themes/test_subsubtheme/css/subsubtheme-farbtastic.css', 'core', 'jquery.farbtastic', 'css'); + $this->assertAssetInLibrary('core/modules/system/tests/themes/test_subsubtheme/js/subsubtheme-dialog.js', 'core', 'drupal.dialog', 'js'); + } + + /** * Tests libraries-extend. */ public function testLibrariesExtend() { diff --git a/core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php b/core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php index 121aebc..6bfac65 100644 --- a/core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php +++ b/core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php @@ -505,6 +505,18 @@ public function testLibraryWithLicenses() { $this->assertEquals($library['license'], $expected_license); } + /** + * @covers ::getLatestOverridePath + * @covers ::setLatestOverridePath + */ + public function testLatestOverridePath() { + $latest = $this->libraryDiscoveryParser->getLatestOverridePath('original-path', 'type', 'extension', 'library_name'); + $this->assertEquals('original-path', $latest); + $this->libraryDiscoveryParser->setLatestOverridePath('latest-path', 'original-path', 'type', 'extension', 'library_name'); + $latest = $this->libraryDiscoveryParser->getLatestOverridePath('original-path', 'type', 'extension', 'library_name'); + $this->assertEquals('latest-path', $latest); + } + } /** @@ -532,6 +544,14 @@ public function setFileValidUri($source, $valid) { $this->validUris[$source] = $valid; } + public function getLatestOverridePath($original_path, $type, $extension, $library_name) { + return parent::getLatestOverridePath($original_path, $type, $extension, $library_name); + } + + public function setLatestOverridePath($new_path, $original_path, $type, $extension, $library_name) { + parent::setLatestOverridePath($new_path, $original_path, $type, $extension, $library_name); + } + } if (!defined('CSS_AGGREGATE_DEFAULT')) {