diff --git a/core/lib/Drupal/Core/Extension/ModuleRequiredByThemesUninstallValidator.php b/core/lib/Drupal/Core/Extension/ModuleRequiredByThemesUninstallValidator.php index 23ae25f76f..b3dc84dafd 100644 --- a/core/lib/Drupal/Core/Extension/ModuleRequiredByThemesUninstallValidator.php +++ b/core/lib/Drupal/Core/Extension/ModuleRequiredByThemesUninstallValidator.php @@ -6,7 +6,7 @@ use Drupal\Core\StringTranslation\TranslationInterface; /** - * Ensures modules required by themes cannot be uninstalled. + * Ensures modules cannot be uninstalled if enabled themes depend on them. */ class ModuleRequiredByThemesUninstallValidator implements ModuleUninstallValidatorInterface { @@ -34,7 +34,7 @@ class ModuleRequiredByThemesUninstallValidator implements ModuleUninstallValidat * @param \Drupal\Core\Extension\ModuleExtensionList $extension_list_module * The module extension list. * @param \Drupal\Core\Extension\ThemeExtensionList $extension_list_theme - * A extension discovery instance. + * The theme extension list. */ public function __construct(TranslationInterface $string_translation, ModuleExtensionList $extension_list_module, ThemeExtensionList $extension_list_theme) { $this->stringTranslation = $string_translation; @@ -53,8 +53,8 @@ public function validate($module) { $module_name = $this->getModuleName($module); $theme_names = implode(", ", $themes_depending_on_module); $reasons[] = $this->formatPlural(count($themes_depending_on_module), - 'The @module_name module is required by the theme @theme_names', - 'The @module_name module is required by the themes @theme_names', + 'Required by the theme: @theme_names', + 'Required by the themes: @theme_names', ['@module_name' => $module_name, '@theme_names' => $theme_names]); } diff --git a/core/lib/Drupal/Core/Extension/ThemeExtensionList.php b/core/lib/Drupal/Core/Extension/ThemeExtensionList.php index ad04156115..95334a1b5b 100644 --- a/core/lib/Drupal/Core/Extension/ThemeExtensionList.php +++ b/core/lib/Drupal/Core/Extension/ThemeExtensionList.php @@ -144,9 +144,8 @@ protected function doList() { foreach ($themes as $key => $theme) { // After $theme is processed by buildModuleDependencies(), there is a // `$theme->requires` array containing both module and base theme - // dependencies. The module dependencies are copied to their own - // property so they are available to operations that are specific to - // module dependencies. + // dependencies. The module dependencies are copied to their own property + // so they are available to operations specific to module dependencies. if (!isset($theme->requires)) { $theme->requires = []; } diff --git a/core/lib/Drupal/Core/Extension/ThemeInstaller.php b/core/lib/Drupal/Core/Extension/ThemeInstaller.php index 9c86e01ac3..4993fa1cad 100644 --- a/core/lib/Drupal/Core/Extension/ThemeInstaller.php +++ b/core/lib/Drupal/Core/Extension/ThemeInstaller.php @@ -107,6 +107,7 @@ public function install(array $theme_list, $install_dependencies = TRUE) { $theme_data = $this->themeHandler->rebuildThemeData(); $installed_themes = $this->configFactory->get('core.extension')->get('theme') ?: []; + $installed_modules = $this->configFactory->get('core.extension')->get('module') ?: []; if ($install_dependencies) { $theme_list = array_combine($theme_list, $theme_list); @@ -125,6 +126,13 @@ public function install(array $theme_list, $install_dependencies = TRUE) { foreach ($theme_list as $theme => $value) { $module_dependencies = $theme_data[$theme]->module_dependencies; $theme_dependencies = array_diff_key($theme_data[$theme]->requires, $module_dependencies); + $missing_module_dependencies = array_diff_key($module_dependencies, $installed_modules); + + // Prevent themes with unmet module dependencies from being installed. + if (!empty($missing_module_dependencies)) { + $missing_module_dependencies_list = implode(', ', array_keys($missing_module_dependencies)); + throw new MissingDependencyException("Unable to install theme: '$theme' due to missing module dependencies: '$missing_module_dependencies_list.'"); + } // Add dependencies to the list of themes to install. The new themes // will be processed as the parent foreach loop continues. diff --git a/core/lib/Drupal/Core/Extension/ThemeInstallerInterface.php b/core/lib/Drupal/Core/Extension/ThemeInstallerInterface.php index ae79b505ea..6f18248cf9 100644 --- a/core/lib/Drupal/Core/Extension/ThemeInstallerInterface.php +++ b/core/lib/Drupal/Core/Extension/ThemeInstallerInterface.php @@ -25,6 +25,9 @@ interface ThemeInstallerInterface { * * @throws \Drupal\Core\Extension\Exception\UnknownExtensionException * Thrown when the theme does not exist. + * + * @throws \Drupal\Core\Extension\MissingDependencyException + * Thrown when a requested dependency can't be found. */ public function install(array $theme_list, $install_dependencies = TRUE); diff --git a/core/modules/system/src/Controller/SystemController.php b/core/modules/system/src/Controller/SystemController.php index 5afa3684a4..c848221dd2 100644 --- a/core/modules/system/src/Controller/SystemController.php +++ b/core/modules/system/src/Controller/SystemController.php @@ -86,9 +86,8 @@ public function __construct(SystemManager $systemManager, ThemeAccessCheck $them $this->formBuilder = $form_builder; $this->themeHandler = $theme_handler; $this->menuLinkTree = $menu_link_tree; - $this->moduleExtensionList = $module_extension_list; if ($module_extension_list === NULL) { - @trigger_error('The extension.list.module service must be passed to \Drupal\system\Controller\SystemController::__construct. It was added in Drupal 8.9.0 and will be required before Drupal 9.0.0.', E_USER_DEPRECATED); + @trigger_error('The extension.list.module service must be passed to ' . __NAMESPACE__ . '\SystemController::__construct. It was added in Drupal 8.9.0 and will be required before Drupal 9.0.0.', E_USER_DEPRECATED); $module_extension_list = \Drupal::service('extension.list.module'); } $this->moduleExtensionList = $module_extension_list; @@ -210,7 +209,6 @@ public function themesPage() { $theme_groups = ['installed' => [], 'uninstalled' => []]; $admin_theme = $config->get('admin'); $admin_theme_options = []; - $modules = []; foreach ($themes as &$theme) { if (!empty($theme->info['hidden'])) { @@ -260,9 +258,7 @@ public function themesPage() { // Check module dependencies. if ($theme->module_dependencies) { - if (empty($modules)) { - $modules = $this->moduleExtensionList->getList(); - } + $modules = $this->moduleExtensionList->getList(); foreach ($theme->module_dependencies as $dependency => $dependency_object) { if ($incompatible = $this->checkDependencyMessage($modules, $dependency, $dependency_object)) { $theme->module_dependencies[$dependency] = $incompatible; @@ -279,9 +275,8 @@ public function themesPage() { $module_name = $modules[$dependency]->info['name']; $theme->module_dependencies[$dependency] = $modules[$dependency]->status ? $this->t('@module_name', ['@module_name' => $module_name]) : $this->t('@module_name (disabled)', ['@module_name' => $module_name]); // Create an additional property that contains only disabled module - // dependencies. This will determine if it is possible to install - // the theme, or if dependencies must first be enabled at - // admin/modules. + // dependencies. This will determine if it is possible to install the + // theme, or if modules must first be enabled. if (!$modules[$dependency]->status) { $theme->module_dependencies_disabled[$dependency] = $module_name; if (!$this->currentUser()->hasPermission('administer modules')) { diff --git a/core/modules/system/src/Form/ModulesListForm.php b/core/modules/system/src/Form/ModulesListForm.php index 46732d4e64..45a57d7d12 100644 --- a/core/modules/system/src/Form/ModulesListForm.php +++ b/core/modules/system/src/Form/ModulesListForm.php @@ -217,7 +217,7 @@ public function buildForm(array $form, FormStateInterface $form_state) { * Builds a table row for the system modules page. * * @param array $modules - * The list of existing modules. + * The list existing modules. * @param \Drupal\Core\Extension\Extension $module * The module for which to build the form row. * @param $distribution diff --git a/core/modules/system/src/Form/ModulesUninstallForm.php b/core/modules/system/src/Form/ModulesUninstallForm.php index 52fde1e5ce..9370050d15 100644 --- a/core/modules/system/src/Form/ModulesUninstallForm.php +++ b/core/modules/system/src/Form/ModulesUninstallForm.php @@ -5,7 +5,6 @@ use Drupal\Core\Extension\ModuleExtensionList; use Drupal\Core\Extension\ModuleHandlerInterface; use Drupal\Core\Extension\ModuleInstallerInterface; -use Drupal\Core\Extension\ThemeExtensionList; use Drupal\Core\Form\FormBase; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\KeyValueStore\KeyValueStoreExpirableInterface; @@ -46,13 +45,6 @@ class ModulesUninstallForm extends FormBase { */ protected $moduleExtensionList; - /** - * The theme extension list. - * - * @var \Drupal\Core\Extension\ThemeExtensionList - */ - protected $themeExtensionList; - /** * {@inheritdoc} */ @@ -61,8 +53,7 @@ public static function create(ContainerInterface $container) { $container->get('module_handler'), $container->get('module_installer'), $container->get('keyvalue.expirable')->get('modules_uninstall'), - $container->get('extension.list.module'), - $container->get('extension.list.theme') + $container->get('extension.list.module') ); } @@ -77,19 +68,12 @@ public static function create(ContainerInterface $container) { * The key value expirable factory. * @param \Drupal\Core\Extension\ModuleExtensionList $extension_list_module * The module extension list. - * @param \Drupal\Core\Extension\ThemeExtensionList $theme_extension_list - * The theme extension list. */ - public function __construct(ModuleHandlerInterface $module_handler, ModuleInstallerInterface $module_installer, KeyValueStoreExpirableInterface $key_value_expirable, ModuleExtensionList $extension_list_module, ThemeExtensionList $theme_extension_list = NULL) { + public function __construct(ModuleHandlerInterface $module_handler, ModuleInstallerInterface $module_installer, KeyValueStoreExpirableInterface $key_value_expirable, ModuleExtensionList $extension_list_module) { $this->moduleExtensionList = $extension_list_module; $this->moduleHandler = $module_handler; $this->moduleInstaller = $module_installer; $this->keyValueExpirable = $key_value_expirable; - if (is_null($theme_extension_list)) { - @trigger_error('The extension.list.theme service must be passed to ' . __NAMESPACE__ . '\ModulesUninstallForm::__construct(). It was added in Drupal 8.9.0 and will be required before Drupal 9.0.0.', E_USER_DEPRECATED); - $theme_extension_list = \Drupal::service('extension.list.theme'); - } - $this->themeExtensionList = $theme_extension_list; } /** @@ -147,19 +131,6 @@ public function buildForm(array $form, FormStateInterface $form_state) { uasort($uninstallable, 'system_sort_modules_by_info_name'); $validation_reasons = $this->moduleInstaller->validateUninstall(array_keys($uninstallable)); - $themes = $this->themeExtensionList->getList(); - - $modules_required_by_enabled_themes = []; - foreach ($themes as $theme_name => $theme) { - if (!empty($theme->info['dependencies'])) { - foreach ($theme->info['dependencies'] as $dependency) { - if (isset($uninstallable[$dependency]) && $theme->status === 1) { - $modules_required_by_enabled_themes[$dependency][] = $theme; - } - } - } - } - $form['uninstall'] = ['#tree' => TRUE]; foreach ($uninstallable as $module_key => $module) { $name = $module->info['name'] ?: $module->getName(); @@ -188,14 +159,6 @@ public function buildForm(array $form, FormStateInterface $form_state) { $form['uninstall'][$module->getName()]['#disabled'] = TRUE; } } - // Modules required by enabled themes should not be uninstallable. - if (isset($modules_required_by_enabled_themes[$module->getName()])) { - foreach ($modules_required_by_enabled_themes[$module->getName()] as $theme) { - $theme_name = $theme->getName(); - $form['modules'][$module->getName()]['#required_by'][] = $theme_name . ' (' . (string) $this->t('Theme') . ')'; - $form['uninstall'][$module->getName()]['#disabled'] = TRUE; - } - } } $form['#attached']['library'][] = 'system/drupal.system.modules'; diff --git a/core/modules/system/src/ModuleDependencyMessageTrait.php b/core/modules/system/src/ModuleDependencyMessageTrait.php index 3a55356a56..66165089e6 100644 --- a/core/modules/system/src/ModuleDependencyMessageTrait.php +++ b/core/modules/system/src/ModuleDependencyMessageTrait.php @@ -31,6 +31,13 @@ public function checkDependencyMessage(array $modules, $dependency, Dependency $ else { $module_name = $modules[$dependency]->info['name']; + // Check if the module is compatible with the installed version of core. + if ($modules[$dependency]->info['core_incompatible']) { + return $this->t('@module_name (incompatible with this version of Drupal core)', [ + '@module_name' => $module_name, + ]); + } + // Check if the module is incompatible with the dependency constraints. $version = str_replace(\Drupal::CORE_COMPATIBILITY . '-', '', $modules[$dependency]->info['version']); if (!$dependency_object->isCompatible($version)) { @@ -40,13 +47,6 @@ public function checkDependencyMessage(array $modules, $dependency, Dependency $ '@version' => $modules[$dependency]->info['version'], ]); } - - // Check if the module is compatible with the installed version of core. - if ($modules[$dependency]->info['core_incompatible']) { - return $this->t('@module_name (incompatible with this version of Drupal core)', [ - '@module_name' => $module_name, - ]); - } } } diff --git a/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php b/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php index 0e98d2b011..d6171ec831 100644 --- a/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php +++ b/core/modules/system/tests/src/Functional/Theme/ThemeUiTest.php @@ -101,8 +101,8 @@ public function testThemeInstallWithModuleDependencies($theme_name) { $this->drupalGet('admin/modules/uninstall'); $assert_session->elementExists('css', '[name="uninstall[test_module_required_by_theme]"][disabled]'); $assert_session->elementExists('css', '[name="uninstall[test_another_module_required_by_theme]"][disabled]'); - $assert_session->elementTextContains('css', '[data-drupal-selector="edit-test-another-module-required-by-theme"] .item-list', 'Required by: test_theme_depending_on_modules (Theme)'); - $assert_session->elementTextContains('css', '[data-drupal-selector="edit-test-module-required-by-theme"] .item-list', 'Required by: test_theme_depending_on_modules (Theme)'); + $assert_session->elementTextContains('css', '[data-drupal-selector="edit-test-another-module-required-by-theme"] .item-list', 'Required by the theme: test theme depending on modules'); + $assert_session->elementTextContains('css', '[data-drupal-selector="edit-test-module-required-by-theme"] .item-list', 'Required by the theme: test theme depending on modules'); // Uninstall the theme that depends on the modules, and confirm the modules // can now be uninstalled. diff --git a/core/modules/system/tests/themes/test_theme_depending_nonexisting_module/test_theme_depending_on_nonexisting_module.info.yml b/core/modules/system/tests/themes/test_theme_depending_nonexisting_module/test_theme_depending_on_nonexisting_module.info.yml index 62ecbd2fd4..2499fc444c 100644 --- a/core/modules/system/tests/themes/test_theme_depending_nonexisting_module/test_theme_depending_on_nonexisting_module.info.yml +++ b/core/modules/system/tests/themes/test_theme_depending_nonexisting_module/test_theme_depending_on_nonexisting_module.info.yml @@ -2,5 +2,6 @@ name: test theme depending on nonexisting module type: theme core: 8.x base theme: stark +version: VERSION dependencies: - test_module_non_existing diff --git a/core/modules/system/tests/themes/test_theme_depending_on_modules/test_another_module_required_by_theme/test_another_module_required_by_theme.info.yml b/core/modules/system/tests/themes/test_theme_depending_on_modules/test_another_module_required_by_theme/test_another_module_required_by_theme.info.yml index 69dae0d799..4527338d46 100644 --- a/core/modules/system/tests/themes/test_theme_depending_on_modules/test_another_module_required_by_theme/test_another_module_required_by_theme.info.yml +++ b/core/modules/system/tests/themes/test_theme_depending_on_modules/test_another_module_required_by_theme/test_another_module_required_by_theme.info.yml @@ -1,4 +1,4 @@ -name: test another module required by theme +name: Test Another Module Required by Theme type: module core: 8.x package: Testing diff --git a/core/tests/Drupal/KernelTests/Core/Theme/ThemeInstallerTest.php b/core/tests/Drupal/KernelTests/Core/Theme/ThemeInstallerTest.php index c8dd0337a8..068af6744c 100644 --- a/core/tests/Drupal/KernelTests/Core/Theme/ThemeInstallerTest.php +++ b/core/tests/Drupal/KernelTests/Core/Theme/ThemeInstallerTest.php @@ -4,6 +4,8 @@ use Drupal\Core\DependencyInjection\ContainerBuilder; use Drupal\Core\Extension\ExtensionNameLengthException; +use Drupal\Core\Extension\MissingDependencyException; +use Drupal\Core\Extension\ModuleUninstallValidatorException; use Drupal\Core\Extension\Exception\UnknownExtensionException; use Drupal\KernelTests\KernelTestBase; @@ -137,6 +139,45 @@ public function testInstallNameTooLong() { } } + /** + * Tests installing a theme with unmet module dependencies. + */ + public function testInstallThemeWithUnmetModuleDependencies() { + $name = 'test_theme_depending_on_modules'; + + try { + $message = 'ThemeInstaller::install() throws MissingDependencyException upon installing a theme with unmet module dependencies.'; + $this->themeInstaller()->install([$name]); + $this->fail($message); + } + catch (MissingDependencyException $e) { + $this->pass(get_class($e) . ': ' . $e->getMessage()); + } + } + + /** + * Tests installing a theme with module dependencies that are met. + */ + public function testInstallThemeWithMetModuleDependencies() { + $name = 'test_theme_depending_on_modules'; + $themes = $this->themeHandler()->listInfo(); + $this->assertFalse(isset($themes[$name])); + $this->container->get('module_installer')->install(['test_module_required_by_theme', 'test_another_module_required_by_theme']); + $this->themeInstaller()->install([$name]); + $themes = $this->themeHandler()->listInfo(); + $this->assertTrue(isset($themes[$name])); + + // Confirm that modules that active themes depend on can't be uninstalled. + try { + $message = 'Attempting to uninstall a module that an active theme depends on results in a ModuleUninstallValidatorException'; + $this->container->get('module_installer')->uninstall(['test_module_required_by_theme']); + $this->fail($message); + } + catch (ModuleUninstallValidatorException $e) { + $this->pass(get_class($e) . ': ' . $e->getMessage()); + } + } + /** * Tests uninstalling the default theme. */