It's possible to enable a non-existing theme

1. +++ b/core/lib/Drupal/Core/Extension/ThemeHandler.php
   @@ -109,6 +109,18 @@ class ThemeHandler implements ThemeHandlerInterface {
    
   +    public function getDefault() {
   ...
   +    public function setDefault($theme) {
   ...
   +
   

   Let's put a {@inheritdoc} here.

2. +++ b/core/lib/Drupal/Core/Extension/ThemeHandler.php
   @@ -109,6 +109,18 @@ class ThemeHandler implements ThemeHandlerInterface {
   +      $this->configFactory->get('system.theme')->set('default', $theme);
   

   We certainly should also save the config :)

3. +++ b/core/lib/Drupal/Core/Extension/ThemeHandler.php
   @@ -158,7 +174,11 @@ class ThemeHandler implements ThemeHandlerInterface {
   +        throw new Exception("Theme $key does not exists.");
   

   Please extend the documentation of disable() to include the @throws as well.

4. +++ b/core/lib/Drupal/Core/Extension/ThemeHandlerInterface.php
   @@ -13,6 +13,18 @@
   +   * @throws Exception
   +   */
   

   It would be kind of cool to know when in which case the Exception is thrown.

5. +++ b/core/lib/Drupal/Core/Extension/ThemeHandler.php
   @@ -147,7 +163,7 @@ class ThemeHandler implements ThemeHandlerInterface {
   -    if ($pos = array_search($this->configFactory->get('system.theme')->get('default'), $theme_list) !== FALSE) {
   +    if ($pos = array_search($this->getDefault(), $theme_list) !== FALSE) {
   

   nice!

It has no assertions!

A LOT of unit tests don't have ... you work with mocks to check that it works.
In this example it uses the following annotations:

   * @expectedException \Drupal\Core\Extension\ExtensionNameLengthException
   * @expectedExceptionMessage Theme name <em class="placeholder">thisNameIsFarTooLong0000000000000000000000000000051</em> is over the maximum allowed length of 50 characters.

this does basically what you think as try/catch.

Hmmm ... it's a UnitTestCase ... how can that enable `theme_test' module? Weird.

You don't ...

Have a look at

    $this->themeHandler->enable(array('bartik'));
    $this->themeHandler->systemList['bartik'] = (object) array(

... switching this two code blocks should just work.

1. +++ b/core/lib/Drupal/Core/Theme/AjaxBasePageNegotiator.php
   @@ -81,7 +81,7 @@ class AjaxBasePageNegotiator implements ThemeNegotiatorInterface {
   diff --git a/core/modules/system/system.module b/core/modules/system/system.module
   
   diff --git a/core/modules/system/system.module b/core/modules/system/system.module
   index f9ae056..83cc650 100644
   
   index f9ae056..83cc650 100644
   --- a/core/modules/system/system.module
   
   --- a/core/modules/system/system.module
   +++ b/core/modules/system/system.module
   
   +++ b/core/modules/system/system.module
   +++ b/core/modules/system/system.module
   @@ -419,7 +419,7 @@ function system_element_info() {
   
   @@ -419,7 +419,7 @@ function system_element_info() {
      );
      $types['password_confirm'] = array(
        '#input' => TRUE,
   -    '#process' => array('form_process_password_confirm', 'user_form_process_password_confirm'),
   +    '#process' => array('form_process_password_confirm'),
        '#theme_wrappers' => array('form_element'),
      );
      $types['textarea'] = array(
   diff --git a/core/modules/user/lib/Drupal/user/AccountFormController.php b/core/modules/user/lib/Drupal/user/AccountFormController.php
   
   diff --git a/core/modules/user/lib/Drupal/user/AccountFormController.php b/core/modules/user/lib/Drupal/user/AccountFormController.php
   index 4983b0f..eca2eb2 100644
   
   index 4983b0f..eca2eb2 100644
   --- a/core/modules/user/lib/Drupal/user/AccountFormController.php
   
   --- a/core/modules/user/lib/Drupal/user/AccountFormController.php
   +++ b/core/modules/user/lib/Drupal/user/AccountFormController.php
   
   +++ b/core/modules/user/lib/Drupal/user/AccountFormController.php
   +++ b/core/modules/user/lib/Drupal/user/AccountFormController.php
   @@ -99,6 +99,7 @@
   
   @@ -99,6 +99,7 @@
            '#type' => 'password_confirm',
            '#size' => 25,
            '#description' => $this->t('To change the current user password, enter the new password in both fields.'),
   +        '#pre_render' => array('user_form_pre_render_password_confirm'),
          );
    
          // To skip the current password field, the user must have logged in via a
   @@ -146,6 +147,7 @@
   
   @@ -146,6 +147,7 @@
            '#type' => 'password_confirm',
            '#size' => 25,
            '#description' => $this->t('Provide a password for the new account in both fields.'),
   +        '#pre_render' => array('user_form_pre_render_password_confirm'),
            '#required' => TRUE,
          );
        }
   diff --git a/core/modules/user/user.module b/core/modules/user/user.module
   
   diff --git a/core/modules/user/user.module b/core/modules/user/user.module
   index f6a30cb..f551063 100644
   
   index f6a30cb..f551063 100644
   --- a/core/modules/user/user.module
   
   --- a/core/modules/user/user.module
   +++ b/core/modules/user/user.module
   
   +++ b/core/modules/user/user.module
   +++ b/core/modules/user/user.module
   @@ -1699,13 +1699,10 @@ function _user_mail_notify($op, $account, $langcode = NULL) {
   
   @@ -1699,13 +1699,10 @@ function _user_mail_notify($op, $account, $langcode = NULL) {
    /**
     * Form element process handler for client-side password validation.
     *
   - * This #process handler is automatically invoked for 'password_confirm' form
   - * elements to add the JavaScript and string translations for dynamic password
   - * validation.
   - *
   - * @see system_element_info()
   + * This #pre_render handler is added to the user registration form to add the
   + * JavaScript and string translations for dynamic password validation.
     */
   -function user_form_process_password_confirm($element) {
   +function user_form_pre_render_password_confirm($element) {
      $password_settings = array(
        'confirmTitle' => t('Passwords match:'),
        'confirmSuccess' => t('yes'),
   

   This changes seems 100% out of scope.

2. +++ b/core/lib/Drupal/Core/Extension/ThemeHandler.php
   @@ -112,10 +112,32 @@ class ThemeHandler implements ThemeHandlerInterface {
   +  public function setDefault($theme) {
   +    $available_themes = $this->listInfo();
   +    if (!isset($available_themes[$key])) {
   +      throw new \Exception("Theme $key does not exists.");
   +    }
   +    $this->configFactory
   +        ->get('system.theme')
   +        ->set('default', $theme)
   +        ->save();
   +  }
   +
   +  /**
   

   Should we also take care of checking whether the default theme is enabled?

Yeah phpunit should just not be runned from the simpletest UI. This integration just exists for the testbot.

+ /**
+ * Tries to enable a non existing theme.
+ *
+ * @expectedException \Drupal\Core\Extension\ThemeDoesNotExistException
+ * @expectedExceptionMessage Cannot set the default theme to a non existing theme 'non_existing_theme'.
+ */
+ public function testSetDefaultToNonExistingTheme() {
+ $this->themeHandler->systemList = array();
+ $this->themeHandler->setDefault('non_existing_theme');
+ }

I wonder what should happen if the theme exists but is simply not enabled.

We have no code for missing base theme in ThemeHandler.

All the work you have done is great, also this idea, though we should better stop at some point and continue to work on a follow up?

+++ b/core/lib/Drupal/Core/Extension/ThemeHandler.php
@@ -125,10 +125,70 @@ class ThemeHandler implements ThemeHandlerInterface {
+  public function setDefault($theme) {
...
+  public function getDefault() {
...
+  public function isDefault($theme) {
...
+  public function getAdmin() {
...
+  public function setAdmin($theme) {
...
+   */
+  public function isAdmin($theme) {

Lets's maybe also step back and think whether the ThemeHandler should contain these values. The default/admin theme is a concept which is just used by theme negotiators but nothing else.

It would be great to get some oppinions from other people on that. I don't want to be the bad guy, I was just thinking about it.

+++ b/core/lib/Drupal/Core/Extension/ThemeHandler.php
@@ -171,7 +236,11 @@ class ThemeHandler implements ThemeHandlerInterface {
+        throw new \Exception("Theme $key does not exists.");

Why is this an Exception and not a ThemeDoesNotExistException?

This is a bug report and it looks like it's including things that are not necessary to fix the bug.

If we can get back to just the bug fix we may be able to get this in the 8.0.x branch but without that it's going to be pushed to 8.1.x as a feature request.

Also the indent is not following Drupal coding standards for the next one to write a patch for this.

I agree with @dawehner we should step back a bit on this. And leave the stuff from #2176355: Followup: rewrite remaining code to use themehandler get|set|is for default and admin to that issue's scope.

This was fixed in 8.x #1067408: Themes do not have an installation status

Therefore, closing as outdated. If this is incorrect reopen the issue, by setting the status to 'Active', and add a comment explaining what still needs to be done.

Thanks!