Problem/Motivation

#3516264: CKEditor 5 loads all plugin translations on AJAX operations surfaced a bug in the site installer UI when installing either Standard or Umami.
Screenshot of error after installing standard in browser

This is occurring because when the site configure form is submitted and Drupal\Core\Installer\Form\SiteConfigureForm::submitForm() runs, the update module is installed, which leads to Drupal building a new container. However, the entityTypeManager property on the form object references an object on the old container, and when it is used to load user 1 in order to update the user's field values, the outdated container references result in an exception.

Steps to reproduce

Install Standard or Umami via the UI at /core/install.php, and observe the error in the screenshot in Problem/Motivation after submitting the site configuration form with "Check for updates automatically" checked.

Proposed resolution

Reset SiteConfigureForm properties after the update module is installed in submitForm().

Remaining tasks

Figure out why this does not surface in StandardInstallerTest or UmamiMultilingualInstallTest.

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3573856

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

godotislate created an issue. See original summary.

godotislate opened merge request !14776

godotislate’s picture

godotislate
he/him
commented
Status: Active » Needs review
Issue tags: +Needs test

MR: https://git.drupalcode.org/project/drupal/-/merge_requests/14776

This needs manual testing, because so far I have not been able to identify why the installer tests don't run into this issue.

godotislate’s picture

godotislate
he/him
commented

@smustgrave should be credited for discovering this and @nicxvan also for investigating on Slack.

godotislate’s picture

godotislate
he/him
commented
Issue tags: -Needs test

OK, figured out that the tests weren't running into the exception because they were all not set to install the update module. Updated one of the tests to do that and it fails as expected: https://git.drupalcode.org/issue/drupal-3573856/-/jobs/8527043

nicxvan’s picture

nicxvan commented

This is a great find and a simple test.

It makes sense given a bunch of other fixes needed.

I think this is pretty close to ready, but want to manually test for my own peace of mind.

I took a pass at credit.

catch’s picture

catch
he/him
Primary language English
commented

One question is whether we should be injecting the entity type manager etc. at all in this case rather than using \Drupal directly, but we could discuss that in a follow-up issue given this fixes a regression.

godotislate’s picture

godotislate
he/him
commented

Re #7: I had thought about doing that or various other ideas, but I saw that FormBase has a resetConfigFactory() method and decided to emulate that pattern.

I don't really know or have a preference on what approach is best.

nicxvan’s picture

nicxvan commented
Status: Needs review » Reviewed & tested by the community

I had a chance to test manually too and that works.

A previous test only failed and this makes sense related to other similar issues.

godotislate’s picture

godotislate
he/him
commented
Issue summary: View changes

  • longwave committed 5108015e on 11.x 
    fix: #3573856 SiteConfigureForm properties need to be reset after module...

longwave closed merge request !14776

  • longwave committed 31b727dc on main 
    fix: #3573856 SiteConfigureForm properties need to be reset after module...
godotislate’s picture

godotislate
he/him
commented
Issue summary: View changes
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 31b727dc4f6 to main and 5108015e82c to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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