Use Admin theme in installer

Moving this to the admin theme and bumping priority.

I've started with an MR and covered the installer, the default settings, tests, and also a couple of default_admin methods that worked under the assumption that a database or the config system would be available, which is not during the installation of Drupal.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

I've addressed and fixed the test failure and also the bot report. Back to NR.

How does this look like? Claro has special styling for this with the arrow steps, etc.

Comparing this with the installer on main, it definitely needs work to address the styling issues.

Thanks for the super quick technical turnaround @jurgenhaas. Looking at the screenshot, while its intended to look new and "default admin-like" (ie potentially not the arrows Claro had), it should still look not broken. :)

Sure thing. There is an explicit installer css file already available in default_admin, we need to adjust this to the latest markup.

I'll put in some time on this today

This is caused by an incorrect root CSS selector within the variables files. I was using .root instead of :root

I'll do this in a separate issue, though, so I can backport.

Opened #3619253: Admin theme's CSS variable selector incorrectly uses .root instead of :root to get this resolved and pinged a few people to make the change & RTBC so I can get it committed (and backport it) ASAP

The CSS is fixed and a manual test shows that it now looks as expected. Further down in the install process, it fails with this:

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "user" entity type does not exist. in Drupal\Core\Entity\EntityTypeManager->getDefinition() (line 171 of /var/www/html/web/core/lib/Drupal/Core/Entity/EntityTypeManager.php)

Drupal\Core\Config\UnmetDependenciesException: Configuration objects provided by <em class="placeholder">standard</em> have unmet dependencies: <em class="placeholder">core.entity_form_display.user.user.default (image.style.thumbnail, image), core.entity_view_display.user.user.compact (core.entity_view_mode.user.compact, image.style.thumbnail, image), field.field.user.user.user_picture (image), user.role.content_editor (contextual, navigation, node, path)</em> in Drupal\Core\Config\UnmetDependenciesException::create() (line 100 of core/lib/Drupal/Core/Config/UnmetDependenciesException.php).

But that's not from this MR. Main currently fails with the same exception.

@jurgenhaas can you provide more details or a screenshot? I just tried an install via UI on main and did not see that exception.

@godotislate sure. I'm composer reinstall drupal/core and get Installing drupal/core (dev-main 223bbbb): Extracting archive.

Then I install the site in the UI or with drush si, and get the same error as posted in #17. This happens with or without the MR applied. Here is how that looks:

Just saw there is also the stack trace, if that helps:

Drupal\Core\Config\UnmetDependenciesException::create() (Line: 587)
Drupal\Core\Config\ConfigInstaller->checkConfigurationToInstall() (Line: 186)
Drupal\Core\Extension\ModuleInstaller->install() (Line: 1726)
install_install_profile() (Line: 691)
install_run_task() (Line: 568)
install_run_tasks() (Line: 126)
install_drupal() (Line: 53)

@jurgenhaas is ECA in your codebase? Could it be related or similar to https://git.drupalcode.org/project/eca/-/work_items/3588862?

@godotislate good call, and yes ECA was there. But after removing it, deleting the database, and rebuilding all docker containers to flush APC et al, the problem is still the same. Let me quickly try by removing all contrib modules ...

No luck, removed all contrib code and still get the same issue.

I'm still unable to reproduce. But to get back on-track, if anyone else can reproduce, let's open a separate issue.

@godotislate OK, must be something else at my end. I've started on a fresh environment and there I can't reproduce the error. As soon as I add a bunch of composer packages from our dev environment, it starts failing again. I'll investigate what exactly is going on. Nothing to do with this one here.

So, I've now manually tested the install and it looks good end-2-end. Just the nightwatch test in the pipeline failed. Shall we get that green first or do you want to review this first?

Looks like I missed the references in the install profiles. Should they be switched from Claro to Default Admin as well? I guess so. But waiting for a confirmation before getting into this.

@jurgenhaas which install profiles do you mean?

We need an issue to switch umami's admin theme to default_admin, and another one for standard - although standard probably needs a stable or at least release candidate default_admin before we switch that.

Just merged #3619253: Admin theme's CSS variable selector incorrectly uses .root instead of :root. Verified it looks good.

One thing that happened to me is after the install process was complete, it landed me on a 404 page.

When I went to /admin, I received the following error

The website encountered an unexpected error. Try again later.

InvalidArgumentException: No check has been registered for access_check.permission in Drupal\Core\Access\CheckProvider->loadCheck() (line 110 of core/lib/Drupal/Core/Access/CheckProvider.php).
Drupal\Core\Access\AccessManager->performCheck() (Line: 136)
Drupal\Core\Access\AccessManager->check() (Line: 113)
Drupal\Core\Access\AccessManager->checkRequest() (Line: 106)
Drupal\Core\Routing\AccessAwareRouter->checkAccess() (Line: 91)
Drupal\Core\Routing\AccessAwareRouter->matchRequest() (Line: 89)
Drupal\Core\Http\EventListener\RouterListener->onKernelRequest() (Line: 246)
Symfony\Component\EventDispatcher\EventDispatcher::{closure:Symfony\Component\EventDispatcher\EventDispatcher::optimizeListeners():241}() (Line: 206)
Symfony\Component\EventDispatcher\EventDispatcher->callListeners() (Line: 56)
Symfony\Component\EventDispatcher\EventDispatcher->dispatch() (Line: 162)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 79)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 30)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 71)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 54)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 753)
Drupal\Core\DrupalKernel->handle() (Line: 34)
Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run() (Line: 32)
require('/var/www/html/vendor/autoload_runtime.php') (Line: 20)
require_once('/var/www/html/autoload_runtime.php') (Line: 13)

I this related? Setting to NW just in case.

Tested the install screen after pulling the latest changes and it's looking great.

The requirements screen doesn't look great, but it's basically the same as Claro:

The batch process screen looks good.

After finishing the install I was successfully taken to the welcome page, no errors or anything on my end.

@quietone - yeah that looks like what happened to me.

@jurgenhaas - let me know if you want to troubleshoot at some point.

I'm in a chat with @catch over in #core-maintainers related to another issue and he reported about a 404 when he tried to uninstall the profile, which is what happens at the end of the site install nowadays. So, that sounds like the same issue. If so, then it's unrelated to this MR. I'll try to verify this.

I've just tested this and can't reproduce. It works for me without 404. That's both in plain main, and with this MR applied. Only if I manually uninstall the profile do I get a 404 when going to the frontpage which is because the profile provides that page. But that's unrelated.

Only if I manually uninstall the profile do I get a 404 when going to the frontpage which is because the profile provides that page. But that's unrelated.

Just to confirm this was the problem I had and nothing to do with this issue...

Disclosure: I have used AI to fix the problem.
The site install functionality now works again. I know, a nice to have feature ;-)

Comments on the MR.

Thanks for the changes, @quietone. This looks good.

I tested locally, and the installer is using default_admin:

After install, it's back to claro:

No errors on /admin:

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Reviewing the MR there were a couple of bugfixes here I wouldn't have expected were necessary, but the installer theme stuff is due to us using a theme in the installer that's not in the install profile, which we didn't really do before, and the font preloading is because claro wasn't preloading its fonts yet so we weren't running into that edge case.

Everything else looks good. Committed/pushed to main, thanks! Will need an 11.x backport MR.

Just documenting here for posterity: I was previously opposed to committing this before Admin were stable, but the complexities of proceeding with the Claro deprecation without this were slowing us down on that goal and leading to chicken-and-egg problems, so I think we can allow this for the installer only even if Admin remains beta. (It definitely should not be backported to stable branches, though, because while Admin is beta and can change in minors, the installer itself is not.)

Thanks!

Compared the 11.x MR diff to the main commit diff and just about everything looks good except one thing.

The 11.x MR has to add core/modules/system/tests/themes/test_installer_theme/src/Hook/TestInstallerThemeHooks.php, because the issue where the file was introduced (#3579903: Eliminate Olivero and three test .theme files), was not backported to 11.x. But the MR here does not also remove the core/modules/system/tests/themes/test_installer_theme/test_installer_theme.theme that was deleted in that issue.

Relatedly, in the 11.x backport of #3581218: Deprecate .theme file extension, it looks like the hook in test_installer_theme.theme had the #[ExtensionFileIsConverted] applied instead of being converted to an OOP hook.

tl;dr: I think we should delete the test_installer_theme.theme file here as well.

Based on #50 should this be backported?