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

@@ -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'])) {

core/modules/quickedit/tests/src/Functional/QuickEditMinimalTest.php

@@ -0,0 +1,40 @@
+<?php
+
+namespace Drupal\Tests\quickedit\Functional;
+
+use Drupal\Tests\BrowserTestBase;
+
+/**
+ * Tests Quick Edit can be installed with Minimal.
+ *
+ * @group quickedit
+ */
+class QuickEditMinimalTest extends BrowserTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $profile = 'minimal';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = [
+    'quickedit',
+    'quickedit_test',
+  ];
+
+  /**
+   * Tests that Quick Edit works with no admin theme.
+   *
+   * @see \quickedit_library_info_alter()
+   */
+  public function testSuccessfulInstall() {
+    $editor_user = $this->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.');
+  }
+
+}

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;

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'],

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'),

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);
+  }
+}

core/modules/system/tests/fixtures/update/drupal-8.admin_theme_0.php

@@ -0,0 +1,17 @@
+<?php
+
+/**
+ * @file
+ * Test fixture.
+ */
+
+use Drupal\Core\Database\Database;
+
+$connection = Database::getConnection();
+
+$config = unserialize($connection->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();

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));
     }
   }
 

core/modules/system/tests/src/Functional/Update/AdminThemeUpdateTest.php

@@ -0,0 +1,34 @@
+<?php
+
+namespace Drupal\Tests\system\Functional\Update;
+
+use Drupal\FunctionalTests\Update\UpdatePathTestBase;
+
+/**
+ * Tests system.theme:admin is updated.
+ *
+ * @group system
+ * @group legacy
+ */
+class AdminThemeUpdateTest extends UpdatePathTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setDatabaseDumpFiles() {
+    $this->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'));
+  }
+
+}