Trying to save any theme settings form results in WSoD

After some digging through our code and Drupal core's, it looks like the problem is partially caused by \Drupal\system\Form\ThemeSettingsForm::submitForm() passing the entire theme settings values to theme_settings_convert_to_config() which then attempts to set all top level items in the array it was passed as config keys, which causes Drupal's configuration system to throw an exception because a config key with a dot in it is not valid, due to that character having a special meaning in the configuration system.

We've got a couple of options:

1. We could try and fix just this one form; to do that, we could probably insert our own submit handler so that it runs before ThemeSettingsForm::submitForm() and fix the issue there by escaping the dots in the config name, i.e. replacing them with a colon (:), or:
2. We could do the above but for all forms, to prevent such an error in other places we may not have thought of. This might require a bit more care to ensure we also unescape the configuration name in all cases when/if we retrieve it from the form.

Regardless of which we choose, we should create a basic test to ensure the theme settings form can be submitted successfully and without an error.

I added a test to demonstrate the problem. I've run it locally to verify that it does indeed detect the fatal error. The test is currently queuing and is expected/intended to fail.

This occurs whether we're trying to enforce the theme config or not. It seems odd that CED would be triggered at all.

@ergonlogic Yeah, it looks like the theme form is passing our form elements over to the configuration system, so that would be why. Those form elements are always added even when not enforcing.

Part of the goal for ✨ Improve usability of Config Enforce Devel's UI Postponed is to move the Config Enforce form elements into their own form. Accomplishing that ought to resolve this issue, and any similar ones.

It would solve a number of problems for sure and I'd like to get that done, but it'll probably take a while to do so maybe we can implement a workaround for now with a clear @deprecated and/or @todo tag that the workaround will be removed before 2.0.0, which is when we'll totally rework the UI. How does that sound?

Sure, that makes sense. But let's limit the scope to solving the problem with theme settings, rather than trying to anticipate similar issues.

We could, for example, just add `system_theme_settings` to the form deny-list, which resolves this issue. It can then be properly resolved as part of ✨ Support adding enforced configs to themes Active , for v2.0.0

Yup, we can limit the fix to this form. I think I'd prefer to not add the form to the deny list just because it feels a bit weird from a UX point of view, in that this might become an annoyance if we can't quickly go to the form to enforce something that might turn out to be common in the future.

Alright, so I might have done a deep dive while sleep deprived and a bit delirious but finally figured out how to work around this nasty issue. I pushed a commit that isn't even remotely ready (it's a mess and has commented out debug stuff), but wanted to get this up here so you can see what I'm doing. I'll revisit this tomorrow when I'm less sleep deprived and hopefully refine it and fix it up.

Alright, so it's still a mess but I fixed the remaining issue which was that the system theme settings form submit handler was still saving our enforced config form values because it's dumb so I made sure our submit handler runs before core's and removes our values from the form state after it's done with them.

It's still a mangled mess that needs to be refactored as a trait or something to contain this so it's easy to remove once we don't need it anymore, either because core reworks the form or we pull our enforce form out of the parent form and into a standalone form when ✨ Improve usability of Config Enforce Devel's UI Postponed is completed, whichever occurs first.

Uh, whoops, I broke every non-theme settings form embedded form. Working on it.

I probably spent more time on this than I should have - and I still need to properly document/comment the trait code - but on the plus side, I got a bit more familiar with the form and form handler architecture which will come in handy later.

I have to concur, this looks like it may have ended up being more trouble than just doing the form separation work, but I am fully aware how much easier it can be to see that in hindsight. That said, now that it's done and passing tests, I see no reason *not* to merge it, really. I appreciate that it's nicely segmented off, and if we do end up implementing the "proper" fix (separating forms) on the 2.x branch, at least this gives a workaround for folks who (unfathomably) are still using our 1.x branch and trying to enforce theme settings without throwing 500s :)

Agreed on all points!

Automatically closed - issue fixed for 2 weeks with no activity.