Problem/Motivation
Use-case 1:
When a profile installs a block for a theme, it is created for all enabled themes.
See e.g. the Multilingual Demo profile: https://www.drupal.org/project/multilingual_demo
For reproducing:
- Install the multilingual_demo profile (simplytest.me works perfectly for reproducing this bug)
- Go to "Single Export":
admin/config/development/configuration/single/export
- Select "Block" for "Configuration type"
- In "Configuration name" you will see "duplicated" names. They are the blocks created for the different enabled themes.
I set component other as I cannot find the culprit yet.
Use-case 2:
The Thunder distribution wants to adopt Olivero as our default theme.
The problem is Olivero ships some blocks in config/install. They get installed first. After that, our Thunder Admin Theme gets enabled. We have some block configuration for that in config/optional of Thunder but that gets ignored because profile configuration gets imported at the end of the installation. That means for the Admin Theme block_theme_initialize runs during the installation and creates copies of oliveros blocks.
Proposed resolution
Remaining tasks
- Attach a patch fixing it.
- Review it manually.
- Add tests for it.
User interface changes
None.
API changes
None.
Comment | File | Size | Author |
---|---|---|---|
#32 | 2404105-32.patch | 8.45 KB | chr.fritsch |
#32 | interdiff-2404105-29-32.txt | 1.22 KB | chr.fritsch |
#29 | 2404105-29.patch | 8.27 KB | chr.fritsch |
#29 | interdiff-2404105-28-29.txt | 2.25 KB | chr.fritsch |
#28 | 2404105-28.patch | 8.21 KB | chr.fritsch |
Comments
Comment #1
alexpottThe culprit :)
Comment #2
penyaskitoAlexpott suggested this fix on IRC and worked for me.
Comment #3
alexpottLet's move this to block_themes_installed() so we save queries and loops during the installer.
Comment #4
alexpottWrong culprit... well it's kind of both of them so this seems only fair :)
Comment #5
jlbellidoI'm going to work on this during the SprintWeekend2015.
Comment #6
jlbellidoChanged #2 with the suggested changes in #3 and running tests.
I've tested it manually and it works for me. Tests still needed.
Comment #7
penyaskitoNeeds works, tests missing yet.
Comment #8
Anonymous (not verified) CreditAttribution: Anonymous commentedI have just stumbled upon this thing.
I would welcome a setting for the block module to disable this feature because themes are now able to provide their own block instances via configuration so if a theme does that and out of nothing there are blocks that have no place to be there the developer get's really confused.
I know this can be easily turned off by hook_module_implements_alter but I don't like it and I'd rather have a setting that I can turn on/off globally or per theme.
My issue is for the post-installation state but this issue covers it so I won't be creating a new one.
Comment #17
chr.fritschSo this is really annoying. We are currently working on activating Olivero as the default theme for Thunder.
Olivero ships some blocks in config/install. They get installed first. After that the seven theme gets enabled. We have some block configuration for that in config/install of Thunder but that get's ignored because profile configuration get's imported at the end. That means for the seven theme
block_theme_initialize
runs and creates random blocks...So here is a rerolled patch.
Comment #19
chr.fritschHere is a test.
Comment #21
alexpottShould this also check if config is syncing for the same reasons?
Hmmm... I see why this is not really causing an issue... it's because we only do the random block creation when there are no blocks.
I guess this is a super super rare situation during a config sync. Why is this not protected us during an install?
Comment #22
chr.fritschIt depends on the themes that get installed. For themes with blocks in config/install it's not a problem, because this config get's directly imported during the installation. But for themes that only have blocks in config/optional or only have block config in the profile, these blocks get imported at the end of the installation and then
block_theme_initialize
runs before.Comment #23
alexpott@chr.fritsch - thanks for answering the q. So adding a follow-up to deal with enabling such a theme via config sync seems something we should do.
Comment #24
chr.fritschSo you are talking about the following use-case:
I have an installed site with some themes enabled. Then I add for example claro to the
core.extensions
file and execute adrush cim
. Then I could image that the following two things could happen:- the optional config of claro is installed
- block_theme_initialize creates blocks for claro
But instead, nothing of that happens. I think block_theme_initialize creates some blocks, but they get immediately deleted.
I think that is ok.
Comment #25
alexpott@chr.fritsch yeah - but it'd be nice to not do the unnecessary creations and deletions.
Comment #26
chr.fritschHere is the follow-up
Comment #27
alexpottSo this then prompts the question what if blocks are not installed as optional config?!?!?
Do we have to check if the theme does have optional config?
Don't we want to maintain this behaviour for themes with no blocks...
I think we need to call this from install_install_profile() if the block module is present for all themes that have been installed...
Comment #28
chr.fritschRight, I added it install_install_profile and also added a test for it.
Comment #29
chr.fritschAdded some comments and found a way we don't have to change the installer.
Comment #30
alexpottLet's flesh out this comment a bit.
Missing () - should be @see block_modules_installed()
@see block_themes_installed()
Comment #31
alexpottI created #3182738: Delay calling hook_modules_installed() and hook_themes_installed() during site install for a larger discussion about
hook_EXTENSION_installed()
and site install.Comment #32
chr.fritschAddressed #30
Thanks for the reviews, @alexpott
Comment #33
chr.fritschAdded our use case to the IS
Comment #34
daniel.bosenI tested the issue by simply configuring the minimal theme to have olivero as the default theme. Without the patch, minimals admin theme (stark) gets some blocks added from Olivero on installation. For example the "Powered by Drupal" Block is added.
With the patch only the stark blocks, that are shipped in minimals config/install folder are created. This looks good!
Comment #35
alexpottCommitted 4a4770271b and pushed to 9.2.x. Thanks!
Going to ask other committers for a +1 to backport. We can't go as far as 8.9.x because we're missing
Fixed coding standards on commit.
Comment #37
alexpottDiscussed with @catch and we agree that this can be backported.