diff --git a/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php index 6db3321..cea15c1 100644 --- a/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php +++ b/core/lib/Drupal/Core/Asset/LibraryDiscoveryParser.php @@ -187,6 +187,13 @@ public function buildByExtension($extension) { elseif ($this->fileValidUri($source)) { $options['data'] = $source; } + // A regular URI (e.g., http://example.com/example.js) without + // 'external' explicitly specified, which may happen if, e.g. + // libraries-override is used. + else if ($this->isValidUri($source)) { + $options['type'] = 'external'; + $options['data'] = $source; + } // By default, file paths are relative to the registering extension. else { $options['data'] = $path . '/' . $source; @@ -355,17 +362,17 @@ protected function applyLibrariesOverride($libraries, $extension) { list(, , $sub_key, $value) = explode('/', $asset, 4); if ($sub_key === 'drupalSettings') { // drupalSettings may not be overridden. - throw new \LogicException(SafeMarkup::format('drupalSettings may not be overridden in libraries-override. Trying to override %asset. Use hook_library_info_alter() instead.', ['%asset' =>$asset])); + throw new \LogicException(sprintf('drupalSettings may not be overridden in libraries-override. Trying to override %s. Use hook_library_info_alter() instead.', $asset)); } else if ($sub_key === 'css') { // SMACSS category should be incorporated into the asset name. list($category, $value) = explode('/', $value, 2); $parents = [$sub_key, $category, $value]; - $new_parents = [$sub_key, $category, '/' . $theme_path . '/' . $override]; + $new_parents = [$sub_key, $category, $override]; } else { $parents = [$sub_key, $value]; - $new_parents = [$sub_key, '/' . $theme_path . '/' . $override]; + $new_parents = [$sub_key, $override]; } // Remove asset to be overridden, but keep its attributes. $attributes = NestedArray::getValue($libraries[$name], $parents); @@ -394,4 +401,11 @@ protected function fileValidUri($source) { return file_valid_uri($source); } + /** + * Determines if the supplied string is a valid URI. + */ + protected function isValidUri($string) { + return count(explode('://', $string)) === 2; + } + } diff --git a/core/lib/Drupal/Core/Theme/ThemeInitialization.php b/core/lib/Drupal/Core/Theme/ThemeInitialization.php index d953d49..8779f98 100644 --- a/core/lib/Drupal/Core/Theme/ThemeInitialization.php +++ b/core/lib/Drupal/Core/Theme/ThemeInitialization.php @@ -173,7 +173,7 @@ public function getActiveTheme(Extension $theme, array $base_themes = []) { foreach ($base_themes as $base) { if (!empty($base->info['libraries-override'])) { foreach ($base->info['libraries-override'] as $library => $override) { - $values['libraries_override'][$library] = $override; + $values['libraries_override'][$library] = $override ? $this->resolveThemeAssetPath($base, $library, $override) : $override; } } } @@ -181,7 +181,7 @@ public function getActiveTheme(Extension $theme, array $base_themes = []) { // 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; + $values['libraries_override'][$library] = $override ? $this->resolveThemeAssetPath($theme, $library, $override) : $override; } } @@ -294,4 +294,38 @@ protected function prepareStylesheetsRemove(Extension $theme, $base_themes) { return $stylesheets_remove; } + /** + * Ensures that a full path is returned for an overriding theme asset. + * + * @param \Drupal\Core\Extension\Extension $theme + * The theme or base theme. + * @param string $source + * The source asset path, i.e. the library to be replaced. + * @param string $destination + * The library or library asset replacing the source. + * + * @return string + */ + protected function resolveThemeAssetPath(Extension $theme, $source, $destination) { + // Full library definitions don't need to be resolved. + if (count(explode('/', $source)) === 2) { + return $destination; + } + // 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 + // it's relative to the theme. + if ($destination[0] !== '/' && !$this->isValidUri($destination)) { + $theme_path = $theme->getPath(); + return '/' . $theme_path . '/' . $destination; + } + return $destination; + } + + /** + * Determines if the supplied string is a valid URI. + */ + protected function isValidUri($string) { + return count(explode('://', $string)) === 2; + } + } diff --git a/core/modules/system/src/Tests/Asset/LibraryDiscoveryIntegrationTest.php b/core/modules/system/src/Tests/Asset/LibraryDiscoveryIntegrationTest.php index c46ae22..f433390 100644 --- a/core/modules/system/src/Tests/Asset/LibraryDiscoveryIntegrationTest.php +++ b/core/modules/system/src/Tests/Asset/LibraryDiscoveryIntegrationTest.php @@ -107,7 +107,7 @@ public function testLibrariesOverrideDrupalSettings() { $this->fail('Throw LogicException when trying to override drupalSettings'); } catch (\LogicException $e) { - $expected_message = 'drupalSettings may not be overridden in libraries-override. Trying to override core/drupal.ajax/drupalSettings/ajaxPageState. Use hook_library_info_alter() instead.'; + $expected_message = 'drupalSettings may not be overridden in libraries-override. Trying to override core/drupal.ajax/drupalSettings/ajaxPageState. Use hook_library_info_alter() instead.'; $this->assertEqual($e->getMessage(), $expected_message, 'Throw LogicException when trying to override drupalSettings'); } } @@ -135,15 +135,75 @@ public function testLibrariesOverrideMalformedAsset() { $this->fail('Throw LogicException when specifying invalid override'); } catch (\LogicException $e) { - $expected_message = 'Library asset core/drupal.dialog/css is not correctly specified. It should be in the form "extension/library_name/sub_key/path/to/asset.js".'; + $expected_message = 'Library asset core/drupal.dialog/css is not correctly specified. It should be in the form "extension/library_name/sub_key/path/to/asset.js".'; $this->assertEqual($e->getMessage(), $expected_message, 'Throw LogicException when specifying invalid override'); } } /** + * Tests library assets with edge-casey paths. + */ + public function testLibrariesOverrideOtherAssetLibraryNames() { + $this->container->get('theme_handler')->install(['test_theme']); + + /** @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')); + + // Assert Drupal relative paths. + $library = $library_discovery->getLibraryByName('core', 'jquery.once'); + $this->assertAssetInLibrary($library['js'], 'jquery.once', 'themes/my_theme/js/jquery.once.js'); + + // Assert streamwrappers. + $library = $library_discovery->getLibraryByName('core', 'drupal.active-link'); + $this->assertAssetInLibrary($library['js'], 'drupal.active-link', 'public://my_js/active-link.js'); + + // Assert protocol-free URI. + $library = $library_discovery->getLibraryByName('core', 'drupal.dialog'); + $this->assertAssetInLibrary($library['js'], 'drupal.dialog', '//my-server/my_theme/js/dialog.js'); + + // Assert regular URI. + $library = $library_discovery->getLibraryByName('core', 'jquery.farbtastic'); + $this->assertAssetInLibrary($library['js'], 'jquery.farbtastic', 'http://example.com/my_theme/js/farbtastic.js'); + } + + /** + * Tests that base theme libraries-remove still apply in sub themes. + */ + public function testBaseThemeLibrariesOverrideInSubTheme() { + $this->container->get('theme_handler')->install(['test_subtheme']); + + /** @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_subtheme')); + + // Assert that libraries-override specified in the base theme still applies + // in sub theme. + $library = $library_discovery->getLibraryByName('core', 'drupal.dialog'); + $this->assertNoAssetInLibrary($library['js'], 'drupal.dialog', 'core/misc/dialog/dialog.js'); + + $library = $library_discovery->getLibraryByName('core', 'jquery.farbtastic'); + $this->assertAssetInLibrary($library['css'], 'jquery.farbtastic', 'core/modules/system/tests/themes/test_basetheme/css/farbtastic.css'); + } + + /** * Asserts that the given asset is in library. * - * @param mixed[] $library_item + * @param mixed $library_item * The library where given asset should be. * @param string $library_name * Name of the library. @@ -156,20 +216,20 @@ public function testLibrariesOverrideMalformedAsset() { */ protected function assertAssetInLibrary($library_item, $library_name, $asset, $message = NULL) { if (!isset($message)) { - $message = SafeMarkup::format('Asset @asset found in library @library', ['@asset' => $asset, '@library' => $library_name]); + $message = sprintf('Asset %s found in library %s', $asset, $library_name); } foreach ($library_item as $definition) { if ($asset == $definition['data']) { - return($this->pass($message)); + return $this->pass($message); } } - return($this->fail($message)); + return $this->fail($message); } /** * Asserts that the given asset is not in library. * - * @param mixed[] $library_item + * @param mixed $library_item * The library where given asset should not be. * @param string $library_name * Name of the library. @@ -182,14 +242,14 @@ protected function assertAssetInLibrary($library_item, $library_name, $asset, $m */ protected function assertNoAssetInLibrary($library_item, $library_name, $asset, $message = NULL) { if (!isset($message)) { - $message = SafeMarkup::format('Asset @asset not found in library @library', ['@asset' => $asset, '@library' => $library_name]); + $message = sprintf('Asset %s not found in library %s', $asset, $library_name); } foreach ($library_item as $definition) { if ($asset == $definition['data']) { - return($this->fail($message)); + return $this->fail($message); } } - return($this->pass($message)); + return $this->pass($message); } } 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 dcb1a2f..587070c 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 @@ -7,3 +7,6 @@ libraries: - test_basetheme/global-styling stylesheets-remove: - '@theme_test/css/base-remove.css' +libraries-override: + core/drupal.dialog/js/misc/dialog/dialog.js: false + core/jquery.farbtastic/css/component/assets/vendor/farbtastic/farbtastic.css: css/farbtastic.css 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 b50181d..32d723c 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 @@ -14,10 +14,9 @@ description: 'Theme for testing the theme system' version: VERSION base theme: classy core: 8.x --stylesheets-remove: +stylesheets-remove: - '@system/css/system.theme.css' libraries-override: - system/base/css/component/css/system.module.css: false # Replace an entire library. core/drupal.collapse: test_theme/collapse # Remove an entire library. @@ -28,6 +27,14 @@ libraries-override: core/drupal.dialog/css/theme/misc/dialog.theme.css: false # It works for JS as well. core/jquery/js/assets/vendor/jquery/jquery.min.js: js/collapse.js + # Use Drupal relative paths. + core/jquery.once/js/assets/vendor/jquery-once/jquery.once.min.js: /themes/my_theme/js/jquery.once.js + # Use streamwrappers. + core/drupal.active-link/js/misc/active-link.js: public://my_js/active-link.js + # Use protocol-free URI. + core/drupal.dialog/js/misc/dialog/dialog.js: //my-server/my_theme/js/dialog.js + # Use regular URI. + core/jquery.farbtastic/js/component/assets/vendor/farbtastic/farbtastic.js: http://example.com/my_theme/js/farbtastic.js regions: content: Content left: Left