diff --git a/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php index 03b458f5e5..b3f0f3dce4 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 = []; + /** * The stream wrapper manager. * @@ -160,6 +180,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 = []; @@ -352,8 +380,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. @@ -373,23 +401,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); } } } @@ -427,37 +455,39 @@ 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. + @trigger_error('Overriding a library asset using an overridden path as key is deprecated, please use the original path as key.', E_USER_DEPRECATED); + $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); } } @@ -466,13 +496,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 @@ -512,4 +546,47 @@ public static function validateCssLibrary($library) { return 0; } + /** + * 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/lib/Drupal/Core/Theme/ThemeInitialization.php b/core/lib/Drupal/Core/Theme/ThemeInitialization.php index 0bf20d7e8e..d5fbb9868d 100644 --- a/core/lib/Drupal/Core/Theme/ThemeInitialization.php +++ b/core/lib/Drupal/Core/Theme/ThemeInitialization.php @@ -190,8 +190,9 @@ public function getActiveTheme(Extension $theme, array $base_themes = []) { // modules. $values['libraries_override'] = []; - // Get libraries overrides declared by base themes. - foreach ($base_themes as $base) { + // Get libraries overrides declared by base themes. Reverse list to override + // from parent to child. + foreach (array_reverse($base_themes) as $base) { if (!empty($base->info['libraries-override'])) { foreach ($base->info['libraries-override'] as $library => $override) { $values['libraries_override'][$base->getPath()][$library] = $override; diff --git a/core/modules/system/tests/src/Kernel/Theme/ThemeTest.php b/core/modules/system/tests/src/Kernel/Theme/ThemeTest.php index 49b723b285..9e1756e1ab 100644 --- a/core/modules/system/tests/src/Kernel/Theme/ThemeTest.php +++ b/core/modules/system/tests/src/Kernel/Theme/ThemeTest.php @@ -114,7 +114,7 @@ public function testListThemes() { // Check if ThemeHandlerInterface::listInfo() returns disabled themes. // Check for base theme and subtheme lists. $base_theme_list = ['test_basetheme' => 'Theme test base theme']; - $sub_theme_list = ['test_subsubtheme' => 'Theme test subsubtheme', 'test_subtheme' => 'Theme test subtheme']; + $sub_theme_list = ['test_legacysubtheme' => 'Theme test legacysubtheme', 'test_subsubtheme' => 'Theme test subsubtheme', 'test_subtheme' => 'Theme test subtheme']; $this->assertIdentical($themes['test_basetheme']->sub_themes, $sub_theme_list, 'Base theme\'s object includes list of subthemes.'); $this->assertIdentical($themes['test_subtheme']->base_themes, $base_theme_list, 'Subtheme\'s object includes list of base themes.'); 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 3efb45f3d6..1bcf4b90b4 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,14 @@ 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 + core/jquery.ui: + css: + component: + assets/vendor/jquery.ui/themes/base/core.css: css/core.css libraries-extend: classy/base: diff --git a/core/modules/system/tests/themes/test_basetheme/test_basetheme.libraries.yml b/core/modules/system/tests/themes/test_basetheme/test_basetheme.libraries.yml index b3d34064c4..bdcc97bd49 100644 --- a/core/modules/system/tests/themes/test_basetheme/test_basetheme.libraries.yml +++ b/core/modules/system/tests/themes/test_basetheme/test_basetheme.libraries.yml @@ -4,5 +4,6 @@ global-styling: base: base-add.css: {} base-add.sub-remove.css: {} + ensure-order.css: {} samename.css: {} css/base-libraries-extend.css: {} diff --git a/core/modules/system/tests/themes/test_legacysubtheme/test_legacysubtheme.info.yml b/core/modules/system/tests/themes/test_legacysubtheme/test_legacysubtheme.info.yml new file mode 100644 index 0000000000..49afc506e4 --- /dev/null +++ b/core/modules/system/tests/themes/test_legacysubtheme/test_legacysubtheme.info.yml @@ -0,0 +1,12 @@ +name: 'Theme test legacysubtheme' +type: theme +description: 'Test theme which uses test_subtheme as the base theme.' +version: VERSION +core: 8.x +base theme: test_subtheme + +libraries-override: + core/jquery.farbtastic: + css: + component: + /core/modules/system/tests/themes/test_basetheme/css/farbtastic.css: css/legacysubtheme-farbtastic.css 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 7bf373a25a..ced35e8d46 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,12 @@ 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/normalize: + css: + base: + assets/vendor/normalize-css/normalize.css: false diff --git a/core/modules/system/tests/themes/test_subtheme/test_subtheme.info.yml b/core/modules/system/tests/themes/test_subtheme/test_subtheme.info.yml index b217374234..1a88337be9 100644 --- a/core/modules/system/tests/themes/test_subtheme/test_subtheme.info.yml +++ b/core/modules/system/tests/themes/test_subtheme/test_subtheme.info.yml @@ -9,6 +9,15 @@ libraries: stylesheets-remove: - '@theme_test/css/sub-remove.css' - '@test_basetheme/base-add.sub-remove.css' +libraries-override: + core/jquery.ui: + css: + component: + assets/vendor/jquery.ui/themes/base/core.css: css/core.css + test_basetheme/global-styling: + css: + base: + ensure-order.css: css/ensure-order.css libraries-extend: classy/base: diff --git a/core/modules/system/tests/themes/test_theme/test_theme.info.yml b/core/modules/system/tests/themes/test_theme/test_theme.info.yml index b9660a45f5..4606bc7b75 100644 --- a/core/modules/system/tests/themes/test_theme/test_theme.info.yml +++ b/core/modules/system/tests/themes/test_theme/test_theme.info.yml @@ -45,12 +45,12 @@ libraries-override: core/drupal.dropbutton: css: component: - /core/themes/stable/css/core/dropbutton/dropbutton.css: /themes/my_theme/css/dropbutton.css + misc/dropbutton/dropbutton.css: /themes/my_theme/css/dropbutton.css # Use stream wrappers. core/drupal.vertical-tabs: css: component: - /core/themes/stable/css/core/vertical-tabs.css: public://my_css/vertical-tabs.css + misc/vertical-tabs.css: public://my_css/vertical-tabs.css # Use a protocol-relative URI. core/jquery.ui: css: diff --git a/core/tests/Drupal/KernelTests/Core/Asset/LibraryDiscoveryIntegrationTest.php b/core/tests/Drupal/KernelTests/Core/Asset/LibraryDiscoveryIntegrationTest.php index a77adfcdc1..e7e67c3276 100644 --- a/core/tests/Drupal/KernelTests/Core/Asset/LibraryDiscoveryIntegrationTest.php +++ b/core/tests/Drupal/KernelTests/Core/Asset/LibraryDiscoveryIntegrationTest.php @@ -147,6 +147,31 @@ public function testLibrariesOverrideOtherAssetLibraryNames() { $this->assertAssetInLibrary('http://example.com/my_theme/css/farbtastic.css', 'core', 'jquery.farbtastic', 'css'); } + /** + * Tests that libraries-override will not affect order of assets. + */ + public function testLibrariesOverrideOverriddenOrder() { + + $this->assertNoAssetInLibrary('core/modules/system/tests/themes/test_subtheme/css/ensure-order.css', 'test_basetheme', 'global-styling', 'css'); + // Get assets for base_theme. + $original_assets = array_column($this->libraryDiscovery->getLibraryByName('test_basetheme', 'global-styling')['css'], 'data'); + + // Activate a subtheme with overrides. + $this->activateTheme('test_subtheme'); + // Assert asset is applied in subtheme. + $this->assertNoAssetInLibrary('core/modules/system/tests/themes/test_basetheme/ensure-order.css', 'test_basetheme', 'global-styling', 'css'); + $this->assertAssetInLibrary('core/modules/system/tests/themes/test_subtheme/css/ensure-order.css', 'test_basetheme', 'global-styling', 'css'); + + // Get overridden assets. + $overridden_assets = array_column($this->libraryDiscovery->getLibraryByName('test_basetheme', 'global-styling')['css'], 'data'); + + // Overridden asset should be at same position in array. + $this->assertEquals( + array_search('core/modules/system/tests/themes/test_basetheme/ensure-order.css', $original_assets, TRUE), + array_search('core/modules/system/tests/themes/test_subtheme/css/ensure-order.css', $overridden_assets, TRUE) + ); + } + /** * Tests that base theme libraries-override still apply in sub themes. */ @@ -160,6 +185,44 @@ public function testBaseThemeLibrariesOverrideInSubTheme() { $this->assertAssetInLibrary('core/modules/system/tests/themes/test_basetheme/css/farbtastic.css', 'core', 'jquery.farbtastic', 'css'); } + /** + * 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/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/js/subsubtheme-dialog.js', 'core', 'drupal.dialog', 'js'); + + // Assert that the override occurs from parent theme to child theme. + $this->assertNoAssetInLibrary('core/modules/system/tests/themes/test_basetheme/css/core.css', 'core', 'jquery.ui', 'css'); + $this->assertAssetInLibrary('core/modules/system/tests/themes/test_subtheme/css/core.css', 'core', 'jquery.ui', 'css'); + } + + /** + * Tests previous behavior of using the last overridden path. + * + * @group legacy + * @expectedDeprecationMessage Overriding a library asset using an overridden path as key is deprecated, please use the original path as key. + */ + public function testLibrariesOverrideLegacyOverride() { + // Activate a sub-theme that overrides a library override. + $this->activateTheme('test_legacysubtheme'); + // Assert that effective libraries override is from the test_legacysubtheme. + $this->assertNoAssetInLibrary('core/modules/system/tests/themes/test_basetheme/css/farbtastic.css', 'core', 'jquery.farbtastic', 'css'); + $this->assertAssetInLibrary('core/modules/system/tests/themes/test_legacysubtheme/css/legacysubtheme-farbtastic.css', 'core', 'jquery.farbtastic', 'css'); + } + /** * Tests libraries-extend. */ diff --git a/core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php b/core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php index 70fb040df9..95822a93c2 100644 --- a/core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php +++ b/core/tests/Drupal/Tests/Core/Asset/LibraryDiscoveryParserTest.php @@ -558,6 +558,18 @@ public function providerTestCssAssert() { ]; } + /** + * @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); + } + } /** @@ -585,6 +597,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')) { diff --git a/core/themes/seven/seven.info.yml b/core/themes/seven/seven.info.yml index f00d395914..8cebf42a72 100644 --- a/core/themes/seven/seven.info.yml +++ b/core/themes/seven/seven.info.yml @@ -25,9 +25,9 @@ libraries-override: system/base: css: component: - /core/themes/stable/css/system/components/system-status-counter.css: css/components/system-status-counter.css - /core/themes/stable/css/system/components/system-status-report-counters.css: css/components/system-status-report-counters.css - /core/themes/stable/css/system/components/system-status-report-general-info.css: css/components/system-status-report-general-info.css + css/components/system-status-counter.css: css/components/system-status-counter.css + css/components/system-status-report-counters.css: css/components/system-status-report-counters.css + css/components/system-status-report-general-info.css: css/components/system-status-report-general-info.css core/drupal.vertical-tabs: css: component: