Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
While working on #1170362: Install profile is disabled for lots of different reasons and core doesn't allow for that a bug happened due to the fact that simple configuration (eg. system.site) is created and saved without knowing about the original configuration. This means thats events listening to the \Drupal\Core\Config\ConfigEvents::SAVE event. This means that \Drupal\Core\Config\ConfigCrudEvent::isChanged() is completely wrong during config sync. This means that if you only change the system.date:first_day config we'll always set the timezone again in \Drupal\system\TimeZoneResolver::onConfigSave(). [note: this does not matter - it is just wasteful].
The bigger problem is caused by \Drupal\language\EventSubscriber\ConfigSubscriber::onConfigSave() and system.site as this bug can result in bogus rows in language.negotiation:url.prefixes.
Steps to reproduce
Proposed resolution
- Fix \Drupal\Core\Config\ConfigImporter::importConfig() to call ->initWithData() with the old data
- Add test coverage
Remaining tasks
User interface changes
None
API changes
None
Data model changes
None
Release notes snippet
N/a
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 3396197.test-only.patch | 2.07 KB | alexpott |
Issue fork drupal-3396197
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:
changes
Comments
Comment #2
alexpottHere's a test-only patch that shows the problem we have.
Comment #5
borisson_I don't completely understand the underlying issue, and why this code resolves it. But the fix looks very simple and the test coverage proves this works.
Do you think it makes sense to add some kind of documentation in the code so it is is explained why we do the initWithData?
Comment #6
alexpott@borisson_ I debated adding a comment but everywhere in core where initWithData is called it is never documented - including in \Drupal\Core\Config\ConfigImporter::importInvokeOwner() in the same class for configuration entities. So I don't think a comment is necessary.
Comment #7
bramdriesenI think the code and test quite clearly describe the intended working. Also the added if with the initWithData describes what it's doing.
So I would dare to set this to RTBC 🙂
Comment #8
bramdriesenComment #11
catchI also thought about asking for a comment, but when it's there, it's more like 'if there's existing data, initialize with that data', and I'm not sure how to explain that more without referencing the bug being fixed. Since this is blocking, going ahead and committing to 11.x with a cherry-pick to 10.2.x, thanks!
Comment #13
catchAlso cherry-picked to 10.1.x after discussing with @alexpott, worth fixing this bug in 10.1.x in its own right, independently of bigger changes elsewhere.
Comment #14
bircherLate to the party. Thanks for finding and fixing and committing this bug. It is wild that this has gone unnoticed for so long!