From 50ac19a702b0be24270e75dfe2697df24071ee4c Mon Sep 17 00:00:00 2001 From: Lauri Eskola Date: Fri, 27 Sep 2019 18:07:23 +0300 Subject: [PATCH] Issue #2587119 by alexpott, Jaesin, malaynayak, netw3rker, Wim Leers: Form sets system.theme:admin to '0' breaking Quick Edit and making no sense --- core/modules/quickedit/quickedit.module | 2 +- .../src/Functional/QuickEditMinimalTest.php | 40 +++++++++++++++++++ .../src/Controller/SystemController.php | 2 +- .../system/src/Controller/ThemeController.php | 2 +- .../system/src/Form/ThemeAdminForm.php | 2 +- core/modules/system/system.install | 13 ++++++ .../update/drupal-8.admin_theme_0.php | 17 ++++++++ .../tests/src/Functional/System/ThemeTest.php | 7 ++-- .../Update/AdminThemeUpdateTest.php | 34 ++++++++++++++++ 9 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 core/modules/quickedit/tests/src/Functional/QuickEditMinimalTest.php create mode 100644 core/modules/system/tests/fixtures/update/drupal-8.admin_theme_0.php create mode 100644 core/modules/system/tests/src/Functional/Update/AdminThemeUpdateTest.php diff --git a/core/modules/quickedit/quickedit.module b/core/modules/quickedit/quickedit.module index 3c318f19105..fc92d9ca789 100644 --- a/core/modules/quickedit/quickedit.module +++ b/core/modules/quickedit/quickedit.module @@ -79,7 +79,7 @@ function quickedit_library_info_alter(&$libraries, $extension) { // First let the base theme modify the library, then the actual theme. $alter_library = function (&$library, $theme) use (&$alter_library) { - if (isset($theme) && $theme_path = drupal_get_path('theme', $theme)) { + if (!empty($theme) && $theme_path = drupal_get_path('theme', $theme)) { $info = system_get_info('theme', $theme); // Recurse to process base theme(s) first. if (isset($info['base theme'])) { diff --git a/core/modules/quickedit/tests/src/Functional/QuickEditMinimalTest.php b/core/modules/quickedit/tests/src/Functional/QuickEditMinimalTest.php new file mode 100644 index 00000000000..b61dc5ab78c --- /dev/null +++ b/core/modules/quickedit/tests/src/Functional/QuickEditMinimalTest.php @@ -0,0 +1,40 @@ +drupalCreateUser([ + 'access in-place editing', + ]); + $this->drupalLogin($editor_user); + $this->assertSame('', $this->config('system.theme')->get('admin'), 'There is no admin theme set on the site.'); + } + +} diff --git a/core/modules/system/src/Controller/SystemController.php b/core/modules/system/src/Controller/SystemController.php index 35f22282664..b0c3ccc2564 100644 --- a/core/modules/system/src/Controller/SystemController.php +++ b/core/modules/system/src/Controller/SystemController.php @@ -196,7 +196,7 @@ class SystemController extends ControllerBase { continue; } $theme->is_default = ($theme->getName() == $theme_default); - $theme->is_admin = ($theme->getName() == $admin_theme || ($theme->is_default && $admin_theme == '0')); + $theme->is_admin = ($theme->getName() == $admin_theme || ($theme->is_default && empty($admin_theme))); // Identify theme screenshot. $theme->screenshot = NULL; diff --git a/core/modules/system/src/Controller/ThemeController.php b/core/modules/system/src/Controller/ThemeController.php index 29a015f5d0c..3a77a59512d 100644 --- a/core/modules/system/src/Controller/ThemeController.php +++ b/core/modules/system/src/Controller/ThemeController.php @@ -181,7 +181,7 @@ class ThemeController extends ControllerBase { // use: a value of 0 means the admin theme is set to be the default // theme. $admin_theme = $config->get('admin'); - if ($admin_theme != 0 && $admin_theme != $theme) { + if (!empty($admin_theme) && $admin_theme != $theme) { $this->messenger() ->addStatus($this->t('Please note that the administration theme is still set to the %admin_theme theme; consequently, the theme on this page remains unchanged. All non-administrative sections of the site, however, will show the selected %selected_theme theme by default.', [ '%admin_theme' => $themes[$admin_theme]->info['name'], diff --git a/core/modules/system/src/Form/ThemeAdminForm.php b/core/modules/system/src/Form/ThemeAdminForm.php index 1c85a788964..1951f548296 100644 --- a/core/modules/system/src/Form/ThemeAdminForm.php +++ b/core/modules/system/src/Form/ThemeAdminForm.php @@ -38,7 +38,7 @@ class ThemeAdminForm extends ConfigFormBase { ]; $form['admin_theme']['admin_theme'] = [ '#type' => 'select', - '#options' => [0 => $this->t('Default theme')] + $theme_options, + '#options' => ['' => $this->t('Default theme')] + $theme_options, '#title' => $this->t('Administration theme'), '#description' => $this->t('Choose "Default theme" to always use the same theme as the rest of the site.'), '#default_value' => $this->config('system.theme')->get('admin'), diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 218bc724428..6881523f8c3 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -2357,3 +2357,16 @@ function system_update_8801() { ->save(TRUE); } } + +/** + * Fix system.theme:admin when the default theme is used as the admin theme. + */ +function system_update_8802() { + $config = Drupal::configFactory()->getEditable('system.theme'); + // Replace '0' with an empty string as '0' is not a valid value. + if ($config->get('admin') == '0') { + $config + ->set('admin', '') + ->save(TRUE); + } +} diff --git a/core/modules/system/tests/fixtures/update/drupal-8.admin_theme_0.php b/core/modules/system/tests/fixtures/update/drupal-8.admin_theme_0.php new file mode 100644 index 00000000000..fb04d316dc7 --- /dev/null +++ b/core/modules/system/tests/fixtures/update/drupal-8.admin_theme_0.php @@ -0,0 +1,17 @@ +query("SELECT data FROM {config} where name = :name", [':name' => 'system.theme'])->fetchField()); +$config['admin'] = '0'; +$connection->update('config') + ->fields(['data' => serialize($config)]) + ->condition('name', 'system.theme') + ->execute(); diff --git a/core/modules/system/tests/src/Functional/System/ThemeTest.php b/core/modules/system/tests/src/Functional/System/ThemeTest.php index 1964e4172ed..0b6fb81bf4e 100644 --- a/core/modules/system/tests/src/Functional/System/ThemeTest.php +++ b/core/modules/system/tests/src/Functional/System/ThemeTest.php @@ -313,7 +313,7 @@ class ThemeTest extends BrowserTestBase { // Reset to the default theme settings. $edit = [ - 'admin_theme' => '0', + 'admin_theme' => '', 'use_admin_theme' => FALSE, ]; $this->drupalPostForm('admin/appearance', $edit, t('Save configuration')); @@ -458,9 +458,10 @@ class ThemeTest extends BrowserTestBase { $themes = \Drupal::service('theme_handler')->rebuildThemeData(); $version = $themes[$theme_machine_name]->info['version']; - // Confirm the theme is indicated as the default theme. + // Confirm the theme is indicated as the default theme and administration + // theme because the admin theme is the default theme. $out = $this->getSession()->getPage()->getContent(); - $this->assertTrue((bool) preg_match("/$theme_name " . preg_quote($version) . '\s{2,}\(default theme\)/', $out)); + $this->assertTrue((bool) preg_match("/$theme_name " . preg_quote($version) . '\s{2,}\(default theme, administration theme\)/', $out)); } } diff --git a/core/modules/system/tests/src/Functional/Update/AdminThemeUpdateTest.php b/core/modules/system/tests/src/Functional/Update/AdminThemeUpdateTest.php new file mode 100644 index 00000000000..8fa4e07a864 --- /dev/null +++ b/core/modules/system/tests/src/Functional/Update/AdminThemeUpdateTest.php @@ -0,0 +1,34 @@ +databaseDumpFiles = [ + __DIR__ . '/../../../../../system/tests/fixtures/update/drupal-8.6.0.bare.testing.php.gz', + __DIR__ . '/../../../../../system/tests/fixtures/update/drupal-8.admin_theme_0.php', + ]; + } + + /** + * Tests that system.theme:admin is updated as expected. + */ + public function testUpdateHookN() { + $this->assertSame('0', $this->config('system.theme')->get('admin')); + $this->runUpdates(); + $this->assertSame('', $this->config('system.theme')->get('admin')); + } + +}