#3355729 Trying to save any theme settings form results in WSoD

src/Form/DevelFormHelperTrait.php

@@ -26,6 +26,13 @@ trait DevelFormHelperTrait {
    * {@inheritdoc}
    */
   public function submitForm(array &$form, FormStateInterface $form_state) {
+
+    if ($this->shouldSkipThemeSettingsSubmit($form_state)) {
+      return;
+    }
+
+    $this->submitFormThemeSettingsEscapeKeys($form_state);
+
     $this->setSharedFormProperties($form, $form_state);
 
     // Settings for registry configs are read-only.
@@ -33,6 +40,8 @@ trait DevelFormHelperTrait {
 
     $this->saveEnforcedConfigs();
 
+    $this->removeThemeSettingsFormValue($form_state);
+
     return parent::submitForm($this->form(), $this->formState());
   }
 

src/Form/EmbeddedEnforceForm.php

@@ -7,6 +7,7 @@ namespace Drupal\config_enforce_devel\Form;
 use Drupal\config_enforce\Form\AbstractEnforceForm;
 use Drupal\config_enforce_devel\EnforcedConfig;
 use Drupal\config_enforce_devel\Form\DevelFormHelperTrait;
+use Drupal\config_enforce_devel\Form\ThemeSettingsFormTrait;
 use Drupal\Core\Form\FormStateInterface;
 use Drupal\Core\Link;
 use Drupal\Core\Url;
@@ -18,6 +19,8 @@ class EmbeddedEnforceForm extends AbstractEnforceForm {
 
   use DevelFormHelperTrait;
 
+  use ThemeSettingsFormTrait;
+
   const FORM_ID = 'config_enforce_devel_embedded';
 
   /**
@@ -43,6 +46,8 @@ class EmbeddedEnforceForm extends AbstractEnforceForm {
       'Save enforced configuration settings'
     );
 
+    $form = $this->buildFormThemeSettings($form);
+
     return $form;
 
   }

src/Form/ThemeSettingsFormTrait.php

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\config_enforce_devel\Form;
+
+use Drupal\Core\Form\FormStateInterface;
+
+/**
+ * Trait for working around theme settings form fatal error.
+ *
+ * @see https://www.drupal.org/project/config_enforce_devel/issues/3355729
+ *   Issue detailing the error and solution.
+ *
+ * @see https://www.drupal.org/project/config_enforce_devel/issues/3188474
+ *   Issue detailing plan to pull the embedded enforce form out of the parent
+ *   form and into a dialog or off-canvas panel. When this lands, these traits
+ *   and use of them will be removed.
+ *
+ * @see \Drupal\config_enforce_devel\FormHandler\ThemeSettingsFormHandlerTrait
+ *   Counterpart trait.
+ *
+ * @see \Drupal\system\Form\ThemeSettingsForm::submitForm()
+ *   This passes all theme settings to \theme_settings_convert_to_config().
+ *
+ * @see \theme_settings_convert_to_config()
+ *   Unconditionally attempts to save all configuration, thus resulting in our
+ *   embedded enforce form values (which will contain a dot) causing a fatal
+ *   error.
+ *
+ * @todo Remove this and its counterpart trait when we pull the embedded enforce
+ *   form into its own separate form.
+ */
+trait ThemeSettingsFormTrait {
+
+  /**
+   * Alter the embedded enforce form for theme settings form workaround.
+   *
+   * Note that this performs the check for whether the host form is the theme
+   * settings form so no check is needed before calling this.
+   *
+   * @param array $form
+   *   The un-altered embedded enforce form.
+   *
+   * @return array
+   *   The $form parameter with modifications if the host form is the theme
+   *   settings form.
+   */
+  protected function buildFormThemeSettings(array $form): array {
+
+    if ($this->getContext('form_id') !== 'system_theme_settings') {
+      return $form;
+    }
+
+    $form['config_enforce']['actions']['submit']['#config_enforce_devel_submit'] = true;
+
+    return $form;
+
+  }
+
+  /**
+   * Determine if our submit handler should be skipped on a theme settings form.
+   *
+   * @param \Drupal\Core\Form\FormStateInterface $formState
+   *   The form state at the time of our submit handler being invoked.
+   *
+   * @return bool
+   *   True if this is the theme settings form and it was not submitted by our
+   *   submit button; false if this is the theme settings form but our submit
+   *   button was used; false if this is any other form.
+   */
+  protected function shouldSkipThemeSettingsSubmit(
+    FormStateInterface $formState
+  ): bool {
+
+    if (
+      $this->getContext('form_id') === 'system_theme_settings' &&
+      empty($formState->getTriggeringElement()['#config_enforce_devel_submit'])
+    ) {
+      return true;
+    }
+
+    return false;
+
+  }
+
+  /**
+   * Escape any config keys in our container on theme settings form submit.
+   *
+   * This prevents Drupal core throwing an error if it tries to save our keys
+   * as config as they very likely contain a dot which is an invalid character
+   * in config keys.
+   *
+   * @param \Drupal\Core\Form\FormStateInterface $formState
+   *   The form state at the time of our submit handler being invoked.
+   */
+  protected function submitFormThemeSettingsEscapeKeys(
+    FormStateInterface $formState
+  ): void {
+
+    if ($this->getContext('form_id') !== 'system_theme_settings') {
+      return;
+    }
+
+    foreach ($this->getContext('configs') as $configKey) {
+
+      $formState->setValue(
+        ['config_enforce', $configKey],
+        $formState->getValue(
+          ['config_enforce', \str_replace('.', ':', $configKey)]
+        )
+      );
+
+      $formState->unsetValue(
+        ['config_enforce', \str_replace('.', ':', $configKey)]
+      );
+
+    }
+
+  }
+
+  /**
+   * Remove our values on the theme settings form after saving enforce settings.
+   *
+   * @param \Drupal\Core\Form\FormStateInterface $formState
+   *   The form state at the time of our submit handler being invoked.
+   */
+  protected function removeThemeSettingsFormValue(
+    FormStateInterface $formState
+  ): void {
+
+    // Remove our values from the form state at this point to prevent the theme
+    // settings form submit handler including our values in the theme's
+    // configuration.
+    if ($this->getContext('form_id') === 'system_theme_settings') {
+      $formState->unsetValue('config_enforce');
+    }
+
+  }
+
+}

src/FormHandler/DevelEnforceFormHandler.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Drupal\config_enforce_devel\FormHandler;
 
 use Drupal\config_enforce\FormHandler\AbstractEnforceFormHandler;
+use Drupal\config_enforce_devel\FormHandler\ThemeSettingsFormHandlerTrait;
 use Drupal\config_enforce_devel\TargetModuleCollection;
 
 /**
@@ -12,6 +13,8 @@ use Drupal\config_enforce_devel\TargetModuleCollection;
  */
 class DevelEnforceFormHandler extends AbstractEnforceFormHandler {
 
+  use ThemeSettingsFormHandlerTrait;
+
   /**
    * Method to call from implementations of hook_alter().
    */
@@ -22,4 +25,17 @@ class DevelEnforceFormHandler extends AbstractEnforceFormHandler {
     $this->addEnforceForm('Drupal\config_enforce_devel\Form\EmbeddedEnforceForm');
   }
 
+  /**
+   * {@inheritdoc}
+   *
+   * @todo Remove this once the theme settings traits are removed.
+   */
+  protected function addEnforceForm($class) {
+
+    parent::addEnforceForm($class);
+
+    $this->alterThemeSettingsFormSubmitHandlers();
+
+  }
+
 }

src/FormHandler/ThemeSettingsFormHandlerTrait.php

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\config_enforce_devel\FormHandler;
+
+use Drupal\config_enforce_devel\Form\EmbeddedEnforceForm;
+use Drupal\Core\Form\FormStateInterface;
+
+/**
+ * Theme settings form handler trait.
+ *
+ * @see \Drupal\config_enforce_devel\Form\ThemeSettingsFormTrait
+ *   Counterpart trait; see for details.
+ */
+trait ThemeSettingsFormHandlerTrait {
+
+  /**
+   * Alter the theme settings form submit handlers to prevent fatal errors.
+   *
+   * This does two things:
+   *
+   * 1. It re-orders the submit handlers so that our embedded enforce form's
+   *    submit handler runs before core's submit handler, and
+   *
+   * 2. It inserts a new submit handler before both the core and embedded
+   *    enforce form handlers to escape periods in config key names to prevent
+   *    the core handler causing a fatal error if core ends up trying to save
+   *    them along with the theme settings.
+   */
+  protected function alterThemeSettingsFormSubmitHandlers(): void {
+
+    if ($this->getFormId() !== 'system_theme_settings') {
+      return;
+    }
+
+    /** @var mixed[] Array of submit handlers for this form. */
+    $submitHandlers = &$this->form()['#submit'];
+
+    /** @var false|array False if we can't find our handler; an array with handler callable otherwise. */
+    $ourHandler = false;
+
+    foreach ($submitHandlers as $key => $handler) {
+
+      if (
+        !\is_array($handler) ||
+        count($handler) !== 2 ||
+        !\is_object($handler[0]) ||
+        // This should really have an interface.
+        !($handler[0] instanceof EmbeddedEnforceForm)
+      ) {
+        continue;
+      }
+
+      $ourHandler = $handler;
+
+      // Remove our handler from its current position in the array.
+      unset($submitHandlers[$key]);
+
+      break;
+
+    }
+
+    /** @var int|false The core submit handler index, or false if it can't be found. */
+    $coreIndex = \array_search('::submitForm', $submitHandlers);
+
+    // Don't do anything if we could find the core handler or our embedded
+    // enforce form handler.
+    if (!\is_int($coreIndex) || !\is_array($ourHandler)) {
+      return;
+    }
+
+    // Insert our new handler and then the embedded enforce form's handler right
+    // before the core handler, in that order.
+    \array_splice(
+      $submitHandlers, $coreIndex, 0, [
+        [$this, 'submitFormThemeSettings'],
+        $ourHandler,
+      ]
+    );
+
+  }
+
+  /**
+   * Theme settings form submit handler.
+   *
+   * @param array &$form
+   *   The form array.
+   *
+   * @param FormStateInterface $formState
+   *   The form state at the time of our submit handler being invoked.
+   */
+  public function submitFormThemeSettings(
+    array &$form, FormStateInterface $formState
+  ): void {
+
+    // If this submit was not triggered by our sub-form, completely remove our
+    // form value and return here.
+    if (empty($formState->getTriggeringElement()[
+      '#config_enforce_devel_submit'
+    ])) {
+
+      $formState->unsetValue('config_enforce');
+
+      return;
+
+    }
+
+    /** @var array Our form value. */
+    $value = &$formState->getValue('config_enforce');
+
+    // Escape the config key so that they don't contain a period to avoid Drupal
+    // core throwing a fatal error if it tries to save it.
+    $value[
+      \str_replace('.', ':', $formState->getValue('config_key'))
+    ] = $value[$formState->getValue('config_key')];
+
+    // Remove our original, not escaped key.
+    unset($value[$formState->getValue('config_key')]);
+
+  }
+
+}

tests/src/Functional/ThemeSettingsFormTest.php

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\Tests\config_enforce_devel\Functional;
+
+use Drupal\Tests\BrowserTestBase;
+use Drupal\user\UserInterface;
+
+/**
+ * Theme settings form tests.
+ *
+ * @group config_enforce_devel
+ */
+class ThemeSettingsFormTest extends BrowserTestBase {
+
+  /**
+   * The admin user used in this test.
+   *
+   * @var \Drupal\user\UserInterface
+   */
+  protected UserInterface $adminUser;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected $defaultTheme = 'stark';
+
+  /**
+   * {@inheritdoc}
+   */
+  protected static $modules = [
+    'config_enforce', 'config_enforce_devel', 'system',
+  ];
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp(): void {
+
+    parent::setUp();
+
+    $this->adminUser = $this->drupalCreateUser([
+      'administer themes',
+    ]);
+
+  }
+
+  /**
+   * Test that the theme settings form can be submitted without errors.
+   *
+   * @see https://www.drupal.org/project/config_enforce_devel/issues/3355729
+   *   Intended to catch regressions of this issue.
+   *
+   * @see \Drupal\system\Form\ThemeSettingsForm::submitForm()
+   *   This passes most top-level form values to
+   *   \theme_settings_convert_to_config(), which then attempts to set them all
+   *   on the configuration object it's passed. This is a problem for us because
+   *   the dot character ('.') has a special meaning in Drupal's configuration
+   *   system and will throw an error when it tries to set
+   *
+   * @see \theme_settings_convert_to_config()
+   *   Called from \Drupal\system\Form\ThemeSettingsForm::submitForm() and
+   *   triggers fatal error.
+   *
+   * @see \Drupal\config_enforce_devel\Form\EmbeddedEnforceForm::buildForm()
+   *   We add our Config Enforce form elements in this and various methods it
+   *   calls.
+   */
+  public function testThemeSettingsFormSubmit(): void {
+
+    $this->drupalLogin($this->adminUser);
+
+    $this->drupalGet('admin/appearance/settings/stark');
+
+    $this->submitForm([], 'Save configuration');
+
+    $this->assertSession()->pageTextNotContains(
+      'Drupal\Core\Config\ConfigValueException: stark.settings key contains a dot which is not supported.'
+    );
+
+    /** @var int The HTTP status code of the form submission response. */
+    $statusCode = $this->getSession()->getStatusCode();
+
+    // On the off chance that the assert above doesn't find the
+    // ConfigValueException text but a fatal error was nonetheless thrown,
+    // assert that the HTTP status code of the response is expected to be in
+    // the 1-399 range, which is the non-error range; if the response code is
+    // 400 or higher, it's an error. The error code we expect for this
+    // particular bug is 500, but casting a wide net here will allow us to
+    // potentially catch other unexpected errors.
+    //
+    // @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Status
+    $this->assertSession()->assert($statusCode < 400, \sprintf(
+      'Expected a successful HTTP status code but got code "%s".',
+      $statusCode
+    ));
+
+  }
+
+}