Yuck.

The following code is in system_install():

  // Enable the default theme.
  variable_set('theme_default', 'bartik');
  db_update('system')
    ->fields(array('status' => 1))
    ->condition('type', 'theme')
    ->condition('name', 'bartik')
    ->execute();

That's not right. That's going to force the Bartik theme to be enabled for every single site out there, even if their install profile uses a different theme. I ran into this while reviewing #632100: Use Stark as default theme for Minimal install profile.

This is kind of a tricky problem to solve, however. I don't think we can variable_set('default_theme', 'something_else'); before system_install() runs, because that's what's going to create the variable table in the first place. Hmmm.

Comments

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Status: Active » Postponed (maintainer needs more info)

Hm? And the bug is?

The installation profile just need to un-enable it.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: Bartik is force-enabled in system_install() » Don't force install profiles to undo system_install()'s setting of a default theme
Category: bug » feature
Priority: Major » Normal
Status: Postponed (maintainer needs more info) » Active

The bug, at least to me, is hard-coded assumptions in system module, of all places. While it's true that every single install profile could *undo* what system_install() does, that seems rather sub-optimal. :P

However, given that the D6 version of system_installI() does exactly the same thing:

db_query("INSERT INTO {variable} (name, value) VALUES ('%s', '%s')", 'theme_default', 's:7:"garland";');

I guess you're right that this isn't a bug so much as a feature request for us not to be so janky.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Version: 7.x-dev » 8.x-dev
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

A possible idea for implementation: a new theme = xxx in the install profile's .info file which system module would check and variable_set() it to that. If left unspecified, it would default to whatever variable_get('default_theme')'s default is (currently, bartik).

mlncn’s picture

mlncn CreditAttribution: mlncn commented

I think theme_enable() / theme_disable() work here (see #719814: Add tests for default templates (like node.tpl.php)), but it still seems installation profiles should be able to just have a theme = xxx.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

The approach I'm advocating in #1181776: Change theme_default variable to Stark would require something like this, so I would like to work on it, but I think we should agree on a name of the property we're adding first.
@webchick proposed: theme = bartik
That is definitely the easiest, but I wonder if it is weird for profiles that enable multiple themes. They have to set one as default, so the property still makes sense for them, but it would be named default theme = bartik more ideally for them. So I would lean more towards default theme but I'm not quite sure.
Thoughts?

Dustin@PI’s picture

Dustin@PI CreditAttribution: Dustin@PI commented

My suggestion:

;Turn on example1, example2 and example3; set the default theme to example1; and set the admin theme to example2.

themes[] = example1
themes[] = example2
themes[] = example3
default theme = example1 
admin theme = example2

If admin theme is not set, it is set to the value of default theme. If default theme is not set it should be set to themes[0].

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Component: system.module » install system
Issue tags: +Framework Initiative, +installation profiles, +API addition, +API clean-up, +Platform Initiative

The proposal in #7 makes sense to me.

That said, I actually think that those properties are plain configuration, so I'm playing with the idea of doing:
#1608842: Replace list of bootstrap modules, enabled modules, enabled themes, and default theme with config

However, I'm not 100% sure whether we'll be able to pull that off for D8 (I'd surely love to), so I don't think it would be a good idea to block/postpone this issue on that effort.

Closely related: Someone pointed to a Commerce Kickstart hack on Twitter:
http://drupalcode.org/project/commerce_kickstart.git/commitdiff/df21a9f9...

Essentially, an installation profile needs to be able to choose the (maintenance) theme to be used within the installer itself.

DuaelFr’s picture

DuaelFr
he/him
Primary language French
Location Montpellier, France
CreditAttribution: DuaelFr as a volunteer and at Happyculture commented
Issue summary: View changes
Status: Active » Fixed

This issue have been fixed by the CMI.
There is no theme definition in system_install() anymore. Everything is now in a system.theme.yml file that can be overriden by your profile.

Status: Fixed » Closed (fixed)

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