diff --git a/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php index e2c5250..47e6c2c 100644 --- a/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php +++ b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php @@ -7,6 +7,7 @@ namespace Drupal\Core\Asset; +use Drupal\Component\Utility\SafeMarkup; use Drupal\Core\Asset\Exception\IncompleteLibraryDefinitionException; use Drupal\Core\Asset\Exception\InvalidLibraryFileException; use Drupal\Core\Asset\Exception\LibraryDefinitionMissingLicenseException; @@ -87,7 +88,8 @@ public function buildByExtension($extension) { $path = $this->drupalGetPath($extension_type, $extension); } - $libraries = $this->applyLibrariesOverrides($this->parseLibraryInfo($extension, $path), $extension); + $libraries = $this->parseLibraryInfo($extension, $path); + $libraries = $this->applyLibrariesOverrides($libraries, $extension); foreach ($libraries as $id => &$library) { if (!isset($library['js']) && !isset($library['css']) && !isset($library['drupalSettings'])) { @@ -330,11 +332,12 @@ protected function applyLibrariesOverrides($libraries, $extension) { $theme_path = $active_theme->getPath(); foreach ($libraries as $name => $library) { // Process libraries overrides. - foreach ($libraries_overrides as $item => $override) { + foreach ($libraries_overrides as $asset => $override) { // Active theme defines an override for this library. - if ($item === "$extension/$name") { - // The whole library. Use the override key to specify that this - // library will be overridden when it is called. + if ($asset === "$extension/$name") { + // Active theme defines an override for the whole library. Use the + // override key to specify that this library will be overridden when + // it is called. // @see \Drupal\Core\Asset\LibraryDiscovery::getLibraryByName() if ($override) { $libraries[$name]['override'] = $override; @@ -343,20 +346,28 @@ protected function applyLibrariesOverrides($libraries, $extension) { $libraries[$name]['override'] = FALSE; } } - else if (strpos($item, "$extension/$name/") !== FALSE) { - // An asset within the library. - list(, , $component, $value) = explode('/', $item, 4); - if ($component == 'css') { - // SMACSS-category should be incorporated into the component name. + else if (strpos($asset, "$extension/$name/") !== FALSE) { + // Active theme defines an override for an asset within this library. + // Throw an exception if the asset is not properly specified. + if (substr_count($asset, '/') < 3) { + throw new \LogicException(SafeMarkup::format('Library asset %asset is not correctly specified.', ['%asset' => $asset])); + } + list(, , $sub_key, $value) = explode('/', $asset, 4); + if ($sub_key === 'drupalSettings') { + // drupalSettings should not be overridden. + throw new \LogicException(SafeMarkup::format('drupalSettings cannot be overridden in libraries-override. Trying to override %asset.', ['%asset' =>$asset])); + } + else if ($sub_key === 'css') { + // SMACSS category should be incorporated into the asset name. list($category, $value) = explode('/', $value, 2); - $parents = [$component, $category, $value]; - $new_parents = [$component, $category, '/' . $theme_path . '/' . $override]; + $parents = [$sub_key, $category, $value]; + $new_parents = [$sub_key, $category, '/' . $theme_path . '/' . $override]; } else { - $parents = [$component, $value]; - $new_parents = [$component, '/' . $theme_path . '/' . $override]; + $parents = [$sub_key, $value]; + $new_parents = [$sub_key, '/' . $theme_path . '/' . $override]; } - // Remove previous component to be overridden, but keep the attributes. + // Remove asset to be overridden, but keep its attributes. $attributes = NestedArray::getValue($libraries[$name], $parents); NestedArray::unsetValue($libraries[$name], $parents); if ($override) { diff --git a/core/lib/Drupal/Core/Theme/ActiveTheme.php b/core/lib/Drupal/Core/Theme/ActiveTheme.php index a282434..73f40af 100644 --- a/core/lib/Drupal/Core/Theme/ActiveTheme.php +++ b/core/lib/Drupal/Core/Theme/ActiveTheme.php @@ -67,7 +67,7 @@ class ActiveTheme { protected $libraries; /** - * The libraries or library components overridden by the theme. + * The libraries or library assets overridden by the theme. * * @var array */ @@ -169,9 +169,10 @@ public function getBaseThemes() { } /** - * Returns the libraries or library components overridden by the active theme. + * Returns the libraries or library assets overridden by the active theme. */ public function getLibrariesOverride() { return $this->librariesOverride; } + } diff --git a/core/lib/Drupal/Core/Theme/ThemeInitialization.php b/core/lib/Drupal/Core/Theme/ThemeInitialization.php index 7231610..c94826c 100644 --- a/core/lib/Drupal/Core/Theme/ThemeInitialization.php +++ b/core/lib/Drupal/Core/Theme/ThemeInitialization.php @@ -166,7 +166,7 @@ public function getActiveTheme(Extension $theme, array $base_themes = []) { // modules. $values['libraries_override'] = array(); - // Grab libraries-overrides from base theme. + // Get libraries-override declared by base theme. foreach ($base_themes as $base) { if (!empty($base->info['libraries-override'])) { foreach ($base->info['libraries-override'] as $library => $override) { @@ -175,7 +175,7 @@ public function getActiveTheme(Extension $theme, array $base_themes = []) { } } - // Add libraries-overrides declared by this theme. + // Add libraries-override declared by this theme. if (!empty($theme->info['libraries-override'])) { foreach ($theme->info['libraries-override'] as $library => $override) { $values['libraries_override'][$library] = $override; diff --git a/core/modules/system/src/Tests/Asset/LibraryDiscoveryIntegrationTest.php b/core/modules/system/src/Tests/Asset/LibraryDiscoveryIntegrationTest.php index 5148824..9ce30b8 100644 --- a/core/modules/system/src/Tests/Asset/LibraryDiscoveryIntegrationTest.php +++ b/core/modules/system/src/Tests/Asset/LibraryDiscoveryIntegrationTest.php @@ -8,6 +8,8 @@ namespace Drupal\system\Tests\Asset; use Drupal\Component\Utility\SafeMarkup; +use Drupal\Core\Extension\Extension; +use Drupal\Core\Theme\ActiveTheme; use Drupal\simpletest\KernelTestBase; /** @@ -43,7 +45,6 @@ public function testElementInfoByTheme() { $this->assertTrue($library_discovery->getLibraryByName('test_theme', 'kitten')); } - /** * Tests that libraries-overrides are applied to library definitions. */ @@ -62,27 +63,91 @@ public function testLibrariesOverride() { // Assert that entire library was correctly overridden. $this->assertEqual($library_discovery->getLibraryByName('core', 'drupal.collapse'), $library_discovery->getLibraryByName('test_theme', 'collapse'), 'Entire library correctly overridden.'); - // Assert that library component was correctly overridden. + // Assert that library asset was correctly overridden. $library = $library_discovery->getLibraryByName('classy', 'base'); $this->assertAssetInLibraryComponent($library['css'], 'base', 'core/modules/system/tests/themes/test_theme/css/test_theme_layout.css'); // Assert that entire library was correctly removed. $this->assertFalse($library_discovery->getLibraryByName('core', 'drupal.progress'), 'Entire library correctly removed.'); - // Assert that library component was correctly removed. + // Assert that library asset was correctly removed. $library = $library_discovery->getLibraryByName('core', 'drupal.dialog'); $this->assertNoAssetInLibraryComponent($library['css'], 'drupal.dialog', 'core/misc/dialog.theme.css'); - // Assert that overridden library component still retains attributes. + // Assert that overridden library asset still retains attributes. $library = $library_discovery->getLibraryByName('core', 'jquery'); foreach ($library['js'] as $definition) { if ($definition['data'] == 'core/modules/system/tests/themes/test_theme/js/collapse.js') { - $this->assertEqual($definition['minified'] && $definition['weight'] == -20, 'Previous attributes retained'); + $this->assertTrue($definition['minified'] && $definition['weight'] == -20, 'Previous attributes retained'); break; } } } + /** + * Tests libraries-override on drupalSettings. + */ + public function testLibrariesOverrideDrupalSettings() { + $this->container->get('theme_handler')->install(['test_theme_libraries_override_with_drupal_settings']); + + /** @var \Drupal\Core\Theme\ThemeInitializationInterface $theme_initializer */ + $theme_initializer = $this->container->get('theme.initialization'); + + /** @var \Drupal\Core\Theme\ThemeManagerInterface $theme_manager */ + $theme_manager = $this->container->get('theme.manager'); + + /** @var \Drupal\Core\Render\ElementInfoManagerInterface $element_info */ + $library_discovery = $this->container->get('library.discovery'); + + $theme_manager->setActiveTheme($theme_initializer->getActiveThemeByName('test_theme_libraries_override_with_drupal_settings')); + + // Assert that drupalSettings cannot be overridden and throws an exception. + try { + $library_discovery->getLibraryByName('core', 'drupal.ajax'); + $this->fail('Throw LogicException when trying to override drupalSettings'); + } + catch (\LogicException $e) { + if ($e->getMessage() === 'drupalSettings cannot be overridden in libraries-override. Trying to override core/drupal.ajax/drupalSettings/ajaxPageState.') { + $this->pass('Throw LogicException when trying to override drupalSettings'); + } + else { + $this->fail('Throw LogicException when trying to override drupalSettings'); + } + } + } + + /** + * Tests libraries-override on malformed assets. + */ + public function testLibrariesOverrideMalformedAsset() { + $this->container->get('theme_handler')->install(['test_theme_libraries_override_with_invalid_asset']); + + /** @var \Drupal\Core\Theme\ThemeInitializationInterface $theme_initializer */ + $theme_initializer = $this->container->get('theme.initialization'); + + /** @var \Drupal\Core\Theme\ThemeManagerInterface $theme_manager */ + $theme_manager = $this->container->get('theme.manager'); + + /** @var \Drupal\Core\Render\ElementInfoManagerInterface $element_info */ + $library_discovery = $this->container->get('library.discovery'); + + $theme_manager->setActiveTheme($theme_initializer->getActiveThemeByName('test_theme_libraries_override_with_invalid_asset')); + + // Assert that improperly formed asset "specs" throw an exception. + try { + $library_discovery->getLibraryByName('core', 'drupal.dialog'); + $this->fail('Throw LogicException when specifying invalid override'); + } + catch (\LogicException $e) { + if ($e->getMessage() === 'Library asset core/drupal.dialog/css is not correctly specified.') { + $this->pass('Throw LogicException when specifying invalid override'); + } + else { + $this->fail('Throw LogicException when specifying invalid override'); + } + } + } + protected function assertAssetInLibraryComponent($component, $library_name, $asset, $message = NULL) { if (!isset($message)) { $message = SafeMarkup::format('Asset @asset found in library @library', ['@asset' => $asset, '@library' => $library_name]); diff --git a/core/modules/system/tests/themes/test_theme/css/collapse.css b/core/modules/system/tests/themes/test_theme/css/collapse.css index a774ab4..c781141 100644 --- a/core/modules/system/tests/themes/test_theme/css/collapse.css +++ b/core/modules/system/tests/themes/test_theme/css/collapse.css @@ -1,4 +1,4 @@ /** * @file - * Test css asset file for test_theme.theme + * Test CSS asset file for test_theme.theme */ diff --git a/core/modules/system/tests/themes/test_theme/js/collapse.js b/core/modules/system/tests/themes/test_theme/js/collapse.js index 05e01dd..bfb509c 100644 --- a/core/modules/system/tests/themes/test_theme/js/collapse.js +++ b/core/modules/system/tests/themes/test_theme/js/collapse.js @@ -1,4 +1,4 @@ /** * @file - * Test js asset file for test_theme.theme + * Test JS asset file for test_theme.theme */ 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 abfdc65..e285cd7 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 @@ -24,10 +24,9 @@ libraries-override: classy/base/css/theme/css/layout.css: css/test_theme_layout.css # Remove one particular asset. core/drupal.dialog/css/theme/misc/dialog.theme.css: false - # It works for js as well. + # It works for JS as well. core/jquery/js/assets/vendor/jquery/jquery.min.js: js/collapse.js regions: content: Content left: Left right: Right - diff --git a/core/modules/system/tests/themes/test_theme_libraries_override_with_drupal_settings/test_theme_libraries_override_with_drupal_settings.info.yml b/core/modules/system/tests/themes/test_theme_libraries_override_with_drupal_settings/test_theme_libraries_override_with_drupal_settings.info.yml new file mode 100644 index 0000000..0939c27 --- /dev/null +++ b/core/modules/system/tests/themes/test_theme_libraries_override_with_drupal_settings/test_theme_libraries_override_with_drupal_settings.info.yml @@ -0,0 +1,9 @@ +name: 'Test theme libraries-override' +type: theme +description: 'Theme with drupalSettings libraries-override' +version: VERSION +base theme: classy +core: 8.x +libraries-override: + # drupalSettings libraries override. Should throw a \LogicException. + core/drupal.ajax/drupalSettings/ajaxPageState: { } diff --git a/core/modules/system/tests/themes/test_theme_libraries_override_with_invalid_asset/test_theme_libraries_override_with_invalid_asset.info.yml b/core/modules/system/tests/themes/test_theme_libraries_override_with_invalid_asset/test_theme_libraries_override_with_invalid_asset.info.yml new file mode 100644 index 0000000..ecba74e --- /dev/null +++ b/core/modules/system/tests/themes/test_theme_libraries_override_with_invalid_asset/test_theme_libraries_override_with_invalid_asset.info.yml @@ -0,0 +1,9 @@ +name: 'Test theme libraries-override' +type: theme +description: 'Theme with invalid libraries-override asset spec.' +version: VERSION +base theme: classy +core: 8.x +libraries-override: + # A malformed library asset name. Should throw a \LogicException. + core/drupal.dialog/css: false