Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Posted by xjm
Problem/Motivation
The color module currently only saves a theme's color configuration to config when the color schema does not match any of the default color schemes provided by the theme, going out of its way to return early in color_scheme_form_submit() if the palette matches one of the default palettes. I'm not sure if this is due to it being in some weird interim limbo until #1712250: Convert theme settings to configuration system is in, but there doesn't seem to be any reason to do this, and the result is that you cannot rely on the configuration existing.
I discovered this when I tried to provide a local config override in a settings.php file:
$conf['color.bartik'] = array(
'palette' => array(
'top' => '#096d17',
'bottom' => '#151e16',
)
);
It worked fine until the main configuration got switched to a default bartik palette, whereupon the config file disappeared from disk, and as a result these two colors were the only ones loaded into the palette, so all the colors that were supposed to be inherited from the active configuration were lost.
Proposed resolution
The attached patch might be all that's needed to fix this--simply removing the weird exemption for the default color scheme.
Remaining tasks
This probably needs test coverage, and also some manual testing to make sure it works as intended. Just want to check the test suite before I go any further with it.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | color-1963922-3.patch | 1.08 KB | mgifford |
| #3 | color-1963922-3.patch | 1.08 KB | xjm |
| color.patch | 675 bytes | xjm | |
Comments
Comment #1
xjmComment #2
xjmComment #3
xjmLooks like we still need the file to be created initially. I have no idea how CMI works with themes; does this work?
Comment #4
xjmSo, hm. #3 does result in a color palette being installed, but since all the stylesheet and image generation logic happens within the color module submit handler (and not on cache clear), one has to submit the form before any overrides specified in
settings.phpwill take effect. Rewriting that is a bit more than I've bargained for here. :)This also means that if you stage (e.g)
color.bartik.yml, your theme is way broken until you submit the color form to regenerate the files locally on the site where you imported it.Also, unrelated, the outside hue wheel of the color picker seems to be broken in Safari.
Comment #5
xjmI guess the next step is to factor out the file generation logic into something we can call during installation and config synch (maybe on cache clear?).
Comment #6
alexpott#1890784: Refactor configuration import and sync functions adds the necessary symfony events to do this.
Comment #7
markhalliwellI'll have to take a look at this more (in depth) when I have time.
Comment #8
xjmThis actually has pretty serious implications for deploying color configuration, so bumping to major.
Comment #8.0
xjmRemoving myself from the author field so I can unfollow. --xjm
Comment #9
BTMash CreditAttribution: BTMash commentedFollowing up on this issue. It seems like a part of the issue was fixed (getting the palette in place). The color configuration itself seems to come over just fine. However, the part that doesn't work is where files need to be regenerated when moving from one environment to another (like local to stage or stage to prod / vice versa). These files are actually part of configuration so the yml file looks like:
Which is problematic since the other environment is not going to contain those files. Resaving the theme settings page brings everything up again correctly but you are then left with a difference in configuration. It seems like the logo, stylesheets, files should be in the key_value storage instead of CMI (and regenerate if they do not exist).
Comment #10
catchThis will need an upgrade path and probably configuration schema change, and yes key/value somewhere seems like the right place.
Comment #11
mgiffordJust re-uploading patching #3 for the bots.
Comment #13
heddnThis is related to #2675234: Color Module doesn't regenerate files on deploy or cache clear. This issue seems (based on its title) to only focus on predefined color schemes. But another part of the issue is that when the config is generated, the color scheme isn't used until you save a /form/. But if you use config_readonly, then you cannot save the admin form to generate the css/js/images. This is especially bad if you use config_installer to install the site. The site just looks bad and you cannot fix it until you disable readonly, save the color scheme, then re-enable readonly.
Comment #14
heddnComment #18
geek-merlinUpdated title as i understand the patch. Feel free to correct if i got it wrong.
I agree that this impacts deployability thus has high prio.
Comment #27
quietone CreditAttribution: quietone at PreviousNext commentedColor has been removed from core, #3270899: Remove Color module from core.