Problem/Motivation

The default theme for the Minimal installation profile, Bartik, does not suit the emphasis of the profile.

Proposed resolution

Use the Stark theme as default for Minimal (and disable an alternate administration theme), to emphasize the profile's visual branding. Patch in #11 implements this change.

Remaining tasks

#11 is RTBC.

Use of Stark exacerbates the bug in #728702: Visiting index.php should redirect to install.php if settings.php already has database credentials but database is empty., so that issue should be addressed as a followup.

User interface changes

None other than the default theme change.

API changes

None.

Original report by tstoeckler

Now it might be possible that I'm insane, but it seems that there is no reason to have 'bartik' be Drupal's default theme. We want Drupal to look nice out of the box, but out of the box experience is the realm of our standard install profile. So for a typical user there would be no visible change at all.

One small, but subtle benefit from this is that we don't have to go through all of core whenever we change the default theme.

Question:
- Should minimal profile set 'bartik' as default theme, too?

Files: 
CommentFileSizeAuthor
#58 drupal8.theme-default-stark.58.patch12.82 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,322 pass(es).
[ View ]
#55 1181776_55_drupal8.theme-default-stark.patch10.98 KBscor
PASSED: [[SimpleTest]]: [MySQL] 33,768 pass(es).
[ View ]
#50 drupal8.theme-default-stark-rerolled.50.patch9.16 KBmallezie
PASSED: [[SimpleTest]]: [MySQL] 33,747 pass(es).
[ View ]
#48 drupal8.theme-default-stark.48.patch9 KBsun
PASSED: [[SimpleTest]]: [MySQL] 33,651 pass(es).
[ View ]
#37 bartik.png34.59 KBDavid_Rothstein
#37 stark.png38.25 KBDavid_Rothstein
#35 minimal.png12.61 KBsun
#35 drupal8.theme-default-stark.35.patch11.6 KBsun
PASSED: [[SimpleTest]]: [MySQL] 33,281 pass(es).
[ View ]
#11 drupal-framework.theme-default-stark.11.patch11.22 KBsun
PASSED: [[SimpleTest]]: [MySQL] 33,055 pass(es).
[ View ]
#1 makestarkdefault-1181776-1.patch710 bytesPsikik
PASSED: [[SimpleTest]]: [MySQL] 30,034 pass(es).
[ View ]

Comments

Psikik’s picture

Title:Make Stark the default theme» Make Stark the default theme in minimal profile
StatusFileSize
new710 bytes
PASSED: [[SimpleTest]]: [MySQL] 30,034 pass(es).
[ View ]

I see no need to have Bartik enabled when we're doing a minimal install. Even though we'll probably install another theme soon after installing, we need at least one theme enabled as the default. So to keep with the minimal theme, let's use a minimal theme.

Here's a patch to the minimal install profile that enables Stark and sets it as the default theme and disables Bartik.

Psikik’s picture

Status:Active» Needs review

Forgot to set it to needs review.

tstoeckler’s picture

Title:Make Stark the default theme in minimal profile» Make Stark the default theme

What you're trying to do is covered by #719814: Add tests for default templates (like node.tpl.php). There's no patch there yet, so I think you can just upload the patch there.

Restoring title.

kika’s picture

Issue tags:+d8ux

In a weird way this makes sense. Could UX peeps chime in?

joachim’s picture

Status:Needs review» Needs work

Just so I'm clear -- this is just the minimal install we'd be changing, not the standard install which would still have bartik as default?

+++ b/profiles/minimal/minimal.install
@@ -6,8 +6,15 @@
+  // Disable bartik theme.
+  theme_disable(array('bartik'));

Why do we need to disable bartik if we're in minimal install? What would have enabled it previously?

Powered by Dreditor.

Bojhan’s picture

Shouldn't there be some reasoning to a decision - there is none for this one as far as I read it.

joachim’s picture

I guess to make the minimal install profile look really basic and unencumbered?

tstoeckler’s picture

The point of opening this issue, was the realization on my part, that the theme of a Drupal install really is part of its branding. Just like OpenAtrium and ManagingNews, etc. have dedicated themes tailored to their functionality, their target audience, etc., Drupal core - the distribution/product (vs. the framework) - comes with the Seven theme. That's why the setting of a theme belongs in an install profile, in our case standard profile.
(What to use for the minimal profile, e.g. #7, is, strictly speaking, a different question, although it could/should be discussed in this issue as well.)
Drupal core - the framework, i.e. with no install profile - should, hence, ideally make no assumption on a default theme as that is the realm of Drupal core - the distribution/product - or whatever other distribution/product you make out of it. Since Drupal needs a default theme to function, we can't just use no theme at all, by default, but Stark is basically the theme, that is no theme.

A side-effect of this would be that the testing profile would automatically use Stark, which is a good thing in its own right. But since that could also be achieved quite simply by setting the default theme in testing.install that shouldn't be part of the discussion here and rather be discussed at #719814: Add tests for default templates (like node.tpl.php).

Another positive side-effect would be that a patch to change the default theme would then only touch standard.install (and/or minimal.install). But since that, even now, is a trivial search and replace, this shouldn't be reason enough for this on its own.

Please note that I'm not at all married to this issue and I realize it's not of very big importance, but I do think it would be the right thing to do.

Bojhan’s picture

There we go :) I think that makes a lot of sense, I agree it would be good to have a install profile that could be considered "without branding". Bare in mind there are still some visual elements that are very drupalistic.

I am OK with this, but this needs feedback from our "install profile maintainers" - catch can you chime in?

yoroy’s picture

Interesting. I can't tell from the discussion if Stark would be the admin theme as well? I think that might be pushing it a bit too far, but as the front-end theme it makes sense.

sun’s picture

Assigned:Unassigned» sun
Status:Needs work» Needs review
StatusFileSize
new11.22 KB
PASSED: [[SimpleTest]]: [MySQL] 33,055 pass(es).
[ View ]

Attached patch changes the default theme to Stark.

Only the Standard installation profile uses Bartik (and Seven as administration theme).

The Minimal installation profile uses the new default (Stark) and does not use an administration theme ('cos it's minimal).

The hidden Testing installation profile automatically uses also the new default (Stark).

Drupal's installer and update.php remains to use Seven.

tstoeckler’s picture

Status:Needs review» Reviewed & tested by the community

Patch looks great from review.
I also tried it and it works as advertised. Standard profile shows no difference and minimal comes with Stark (which is incredibly usable by the way).

This would make #728702: Visiting index.php should redirect to install.php if settings.php already has database credentials but database is empty. at least major, though. If you hit that bug with current 8.x, you get to a page with Seven theme. With the patch, you get a page in Stark, which is not really nice.

cweagans’s picture

Title:Make Stark the default theme» Make Stark the default theme for Minimal profile

Fixing the title for clarity. Also, +1.

Jacine’s picture

Love this idea. +1.

yoroy’s picture

No further objections here. I'd say go ahead.

Caligan’s picture

Issue summary:View changes

Updated issue summary.

Bojhan’s picture

Assigned:sun» Unassigned
sun’s picture

Assigned:Unassigned» sun

erm?

sun’s picture

catch’s picture

Assigned:sun» Dries

I'm fine with this but I'm sticking it in Dries' queue since it's a major user facing change for anyone who happens to select that option.

chx’s picture

Leaving this on Dries' plate but. The goal of minimal was not 'debranding' or whatnot. It was part of the agreement that got D7UX into core. We got minimal so I never need to see that. Never. This patch would make minimal barely useable and so it'd hinder my work. Stark is a theme developer feature, a base to start from.

catch’s picture

Just discussed this with chx in irc. I hadn't properly chimed in on this but I'm neutral on the change - we don't actually remove anything from minimal here, it's just a theme switch - so it's down to aesthetics rather than an architectural change (i.e. I'd be more excited about minimal having less modules enabled or something). But I can see arguments both ways and I'm fine either way.

chx’s picture

Right. I am reinstalling core N times a day and now you would make me add a theme switch every time I do that. Hence "hinder".

Bojhan’s picture

I am neutral on this too, I think it makes sense from a branding perspective - but can understand that Stark might be less usable. Whatever the maintainer of minimal profile wants to do :)

sun’s picture

I'm primarily interested in the default value change of the variable, which has the primary effect of 1) making the variable more explicit, 2) not imposing a certain fully-fledged theme on all sites, and lastly, and probably most importantly, 3) automatically using Stark as theme during testing, without having the testing profile install Drupal with a whack default theme X and having to revert that to the plain module output afterwards.

webchick’s picture

I think this is in the spirit of the "product vs. framework" distinction which is where I see "default vs. minimal" at the moment. (Or the best approximation of that we currently have.) It's truly a "blank slate" theme, with only the bare minimal CSS to make sidebars show up in the right place. That's what you want when starting from scratch.

I genuinely don't understand what about Stark makes Drupal "barely usable". Are there specific bugs with Stark not showing certain page elements, or particular accessibility issues? If so, we should fix them. But apart from Stark making Drupal look like a website built in 1992 (which is a good thing when you're starting from scratch), I'm not clear on how it impacts the actual usability of the minimal profile.

webchick’s picture

I mean, look at it this way. The D8 Design Initiative is going to want full reign to put whatever whizz-bangy stuff they want in the new default theme. It might load lots of JS effects, it might have little help pop-ups, it might have animated snowflakes that come down during the entire month of December. Whatever. It might also have none of these things. But I don't want the imagination of designers molding Drupal's default branding to be held back by "Well this would piss off people who use the minimal profile." :P~ And sticking with Bartik on Minimal might work for one release, but that won't help us in Drupal 9, when it would likely be sunsetted just as Garland is headed in Drupal 8.

When the default theme is "opinionated" about what it looks like and does (which a strong theme is), this makes yet another assumption to work around for all the people out there who just want to use "core" as a baseline and work from there. So let's plan ahead for where we want to go, and make this change now, IMO.

Everett Zufelt’s picture

AFAIK, no accessibility issues w/ Stark that would impact this issue.

chx’s picture

Bojhan’s picture

Status:Reviewed & tested by the community» Needs work

I have no favor for any road (I purposely try to avoid any involvement with minimal profile)- seems like not everyone is in agreement so switching status for more discussion. I can understand the barely usable point, because stark as it stands has no visual cues and especially if used for administration - there is little direction for the eye.

sun’s picture

Status:Needs work» Needs review

It actually looks like there is some need for a plain CSS-only theme in core like Stark (without any tpl.phps), which merely has the purpose of being "usable", but not necessarily "ready-to-go".

I don't think there's any theme that uses Stark as base theme, 'cos that wouldn't really make sense. ;)

In light of that, I wonder whether it would be acceptable to just simply minimally tweak Stark's already existing CSS a little more to achieve just that.

I can only guess that would make all sides happy.

webchick’s picture

I still have no idea what the delta is though between Stark and "usable" Stark. What exactly is the problem with Stark that makes it unusable? That it's too plain? Well, that's rather the point. People building web applications/distributions on top of Drupal want any and all default assumptions taken out of their way. Stark is the minimal amount of assumptions needed for things not to be absolutely broken.

Jacine’s picture

Totally agree with webchick in #31. If something is inherently wrong with Stark, other than it being too plain for some people's taste, it's likely that it's a problem that should be fixed in one of the core modules.

The reason this makes sense to me is that if we are truly going to call it a "minimal" profile, we should stay true to what that means, and not make exceptions when it comes to the theme layer.

I don't support making changes to Stark, because it goes against what the theme was created for, and would negatively impact the way we use it for testing. If we make changes to Stark, we loose that true base. It's not worth it IMO, just to have it be the default for a minimal profile, so if it comes down to one or the other, I'd go with using a different theme for the minimal profile. That's my 2 cents.

chx’s picture

Status:Needs review» Reviewed & tested by the community

As I said: go for it. I still maintain that Stark is not useable but there is no reason to use it either. You use stark as a base theme, that's it. If you develop a site starting from minimal you certainly will add a lot of modules and a theme too so you wont use it. For core development I have came up with a good compromise above.

Dries’s picture

Assigned:Dries» Unassigned
Status:Reviewed & tested by the community» Needs work

For me, this boils down to the question: who is the primary user and what are their expectations?

If the target audience is web developers looking to build a Drupal website from scratch, this make sense. (Pretty much all the people in this issue.)

If most users are evaluators and site builders trying out Drupal to see if it is a good fit, it may not make sense. (The people are not represented in this issue.)

Right now, the minimal profile is advertised as "Start with only a few modules enabled.". This doesn't avoid evaluators from using it -- as an evaluator this looks like a perfectly valid choice. Selecting the minimal theme would result in a disappointment as my expectations are not met.

In other words, I'm not comfortable with this change unless we decide to change the description for the "Minimal install profile".

sun’s picture

Assigned:Unassigned» sun
Status:Needs work» Needs review
StatusFileSize
new11.6 KB
PASSED: [[SimpleTest]]: [MySQL] 33,281 pass(es).
[ View ]
new12.61 KB

Discussed in IRC:

- the common perception of the Minimal profile is that it's a base building block for site builders.

- "minimal is what Stark is for our templates, except for our CMS." -- and the goal of Stark is that a designer can start whipping up some CSS, with no default assumptions.

- a benefit of this change is that there is more actual exposure for Stark, because right now, that "but hey, you can also have nothing!" feature is deeply hidden in the theme/appearance administration page.

- of course, there's still a difference between Stark and nothing, as Stark at least tries to get the sidebars minimally aligned, but that's rather off-topic nitpicking in light of this issue. ;)

Based on that, we want to adjust the description of Minimal to manage expectations accordingly:

minimal.png

(which reads: "Minimal: Start with almost no functionality and no design enabled.")

David_Rothstein’s picture

Component:Stark theme» install system

I agree with Dries and chx. Unless we rename the minimal profile to "theme developer" or similar, I can't see why we would do this. That's the audience Stark is aiming for. Even site builders who want to build a site from scratch still won't want Stark here unless they are also planning to build a theme from scratch (which many won't).

Bartik actually seems like a perfect fit for minimal to me. It's nice-looking and easy to use, but also designed to be simple, clean, and not too in-your-face (hence minimal). And regardless of what other fancy themes get added later, we'll always want to have a theme along the lines of Bartik in core, since it's a very common need. So the minimal profile can always use that.

I'm also moving this issue to the install system queue, since that's where people who care about the initial installation experience tend to hang out (yes, we really should have install system and install profiles as separate components in the issue queue, but oh well....)

David_Rothstein’s picture

Status:Needs review» Needs work
StatusFileSize
new38.25 KB
new34.59 KB

As others have said above, though, 95% of this patch may make sense either way (we could still change the default to Stark but not have the minimal profile use it).

However, there are some issues with that:

  1.      // We use the default theme as the maintenance theme. If a default theme
    -    // isn't specified in the database or in settings.php, we use Bartik.
    -    $custom_theme = variable_get('maintenance_theme', variable_get('theme_default', 'bartik'));
    +    // isn't specified in the database or in settings.php, we use Stark.
    +    $custom_theme = variable_get('maintenance_theme', variable_get('theme_default', 'stark'));

    This is problematic because when the database is down, it changes the maintenance theme for all Drupal sites (only sites that have added code to settings.php will be immune from the switch from Bartik to Stark here).

    We can't do that as things currently stand, because whereas Bartik's maintenance theme looks reasonable for this situation (and I believe was even designed to be), Stark's currently does not. See the attached screenshots; which would you rather have users see when your database is down? (Screenshots show that the site name "Drupal" and the default Druplicon logo are displayed to end users by Stark in this situation, again because we don't have database access.)

    So changing this part of the code would require either changes to Stark's maintenance page template to make it look more reasonable, or maybe the configuration management initiative will solve the problem of variables persisting even when the database is down, etc...

  2. I think this patch would need an update function (to explicitly set the 'theme_default' variable for sites that were previously relying on Bartik as the fallback)?
Jacine’s picture

Hmm, I must completely misunderstand what the "minimal" profile is really about. Either that, or people completely disregard the front end when it comes to the "framework" discussion, which I don't get at all. In fact, when developing themes, I never use the minimal profile and wouldn't recommend dong so to any theme developer because you will miss too many critical aspects, that others can enable and then your theme looks like crap. :)

Anyway, it seems like what people are looking for here is a base theme (Stark is not a base theme) that has minimal styling, sort of like a wireframe theme, for this. That would actually be cool, but no such theme has been proposed or discussed for core AFAIK, so maybe it's best to leave this be.

@David_Rothstein I think I'm missing something. What would be the point of doing #37 if no profile in core is going to use it? For custom install profiles and stuff?

cweagans’s picture

It makes sense to me that the default would be Stark and that install profiles could override it as needed.

David_Rothstein’s picture

In fact, when developing themes, I never use the minimal profile and wouldn't recommend dong so to any theme developer because you will miss too many critical aspects, that others can enable and then your theme looks like crap. :)

That's a very good point. In that case, I'm starting to think that the proposed change to the minimal profile actually benefits almost nobody?

@David_Rothstein I think I'm missing something. What would be the point of doing #37 if no profile in core is going to use it? For custom install profiles and stuff?

See comments #8 and #24. Basically, a combination of making testing easier and theoretical purity :)

sun’s picture

when developing themes, I never use the minimal profile

I think the focus and purpose of the Minimal profile is to be the basement/minimum for building a custom site with Drupal, where you not only want a clean slate with regard to modules, but also regarding the visual output/design.

That's great, because when developing a new theme for this custom site, you only want to care for the features that actually exist, and without getting biased in your design decisions just because you constantly look at the suggested visualization of a core theme.

If you still want to build out your site with Bartik, Seven, or whatever other theme, then you can make that an active decision as a site builder. But the essential difference is that we're turning the default behavior around - we do not impose any kind of decision on you. As with modules, you get the (almost) bare minimum you can work off from.

This is problematic because when the database is down, it changes the maintenance theme for all Drupal sites

You make that sound as if it was a feature, whereas I've heard many complaints from site builders that they were shocked and confused to see their site output in a theme they never enabled and didn't want to appear at all. (Kind of an easter-egg experience.) Drupal has a long history with this topic, and while I certainly agree that we made some progress for the better in D7, the disadvantage you've outlined actually points to a baked-in assumption in core that is a problem in itself for me.

In fact, it looks like this nicely revealed that our default maintenance-page.tpl.php in System module could use some tweaking.

a combination of making testing easier

The testing facet of this change is much more than that. Bartik is a fully-fledged theme that contains plenty of overrides. Right now, lots of our tests are verifying the behavior and output of core functionality based on Bartik's overrides. And not even tests using the Testing profile are excluded from this. That's obviously very bad from a unit testing perspective.

But it's also bad from the counter-perspective: Make one change to Bartik, and you possibly break thousands of module tests.

Note that this issue originally was about changing the default theme variable only. Essentially, the 'theme_default' variable change could be a different issue separated from the Minimal profile default theme change, but as this discussion shows, there are many interdependencies that make it hard to properly handle the changes independently. (That said, if anyone sees a way to handle them independently, I'd be more than happy to remove the change to Minimal profile from this patch.)

pillarsdotnet’s picture

Jacine’s picture

Ugh... So, I just learned that the default testing theme is Bartik - in the minimal profile - and that the HTML5 patches we are doing cannot be tested unless we apply the changes to Bartik as well. We are working on the core default templates - not Bartik. So....#fail

mlncn’s picture

Here's the fix for #43, the interim solution for testing (use Stark for the Testing profile): http://drupal.org/node/719814#comment-5147204

Note the duplicate issue #632100: Use Stark as default theme for Minimal install profile

mlncn’s picture

Recommend that this issue be retitled "Make Stark the default theme" and leave decision on making it the default theme to #632100: Use Stark as default theme for Minimal install profile (i am +1 on both).

tstoeckler’s picture

Title:Make Stark the default theme for Minimal profile» Make Stark the default theme
Issue tags:-d8ux

re mlncn: I totally agree with you. I would have advocated combining the two issues here, if it was non-controversial, but since it obviously isn't, let's have that discussion where it belongs (#632100: Use Stark as default theme for Minimal install profile). Hence, changing title.

I tried to roll an updated patch which doesn't change the minimal profile, but I came across a problem regarding the maintenance theme. I think we should avoid showing the maintenance page in Stark for regular users. So what I thought would be nice, if we would set the maintenance theme to the default theme of the install profile when we write settings.php. That is, I think the default use-case for most profiles, and it can always be overriden in settings.php. That requires #1022386: Don't force install profiles to undo system_install()'s setting of a default theme though, because we currently have no way of figuring out what theme a profile sets.
Thoughts?

David_Rothstein’s picture

Regarding the maintenance theme, for the purpose of this issue couldn't we just solve it by removing this from the patch?

     // We use the default theme as the maintenance theme. If a default theme
-    // isn't specified in the database or in settings.php, we use Bartik.
-    $custom_theme = variable_get('maintenance_theme', variable_get('theme_default', 'bartik'));
+    // isn't specified in the database or in settings.php, we use Stark.
+    $custom_theme = variable_get('maintenance_theme', variable_get('theme_default', 'stark'));

In other words, leave the default value as 'bartik' in this one location, even though it would be 'stark' everywhere else. There could be a comment/@todo explaining why.

I don't know about using the install profile's default theme as the maintenance theme default.... A lot of themes aren't well-suited for this (Bartik intentionally cripples its own maintenance page theming in order to be suitable). It would be better to solve the root cause and use the current site theme always. This is something that will hopefully come out of the Configuration Management Initiative naturally, although I was thinking the other day that maybe it's possible to do it without waiting for that whole initiative to complete... If I wind up writing a patch for that, I'll cross-link it here.

sun’s picture

Status:Needs work» Needs review
StatusFileSize
new9 KB
PASSED: [[SimpleTest]]: [MySQL] 33,651 pass(es).
[ View ]

Just discussed with @Jacine at BADcamp, and while @David_Rothstein's suggestion would introduce an inconsistency in terms of the variable's default value, that sounds acceptable in light of the upcoming CMI improvements (that will allow us to use site's default theme for the maintenance page even if there is no database) and we can certainly live with that compromise.

Also agreeing with spinning of the Minimal profile theme discussion into an own issue.

Removed the corresponding hunks accordingly.

xjm’s picture

Status:Needs review» Needs work
Issue tags:+Novice

Tagging as novice for the task of rerolling the Drupal 8.x patch on account of #22336: Move all core Drupal files under a /core folder to improve usability and upgrades.

If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.

mallezie’s picture

Status:Needs work» Needs review
StatusFileSize
new9.16 KB
PASSED: [[SimpleTest]]: [MySQL] 33,747 pass(es).
[ View ]

I've rerolled the patch and tested it. Everything seems all-right.
I hope i did everything right.

This rerolling of patches should be promoted more as first time core patches.
It was easy and fun to make a first core patch, which i understand ;-)

sun’s picture

Same patch, different directories, thanks :) So, reviews, anyone?

xjm’s picture

Issue tags:-Novice

Thanks @mallezie! I agree that rerolling patches is a great way to learn the ropes. We'll be focusing on it tomorrow during core office hours as well. :)

David_Rothstein’s picture

Status:Needs review» Needs work
  1. Since the patch doesn't change any code in the Minimal profile, it makes minimal fall back on Stark as the default theme, which is the exact thing we just decided not to do here :)
  2. I still see a few other random instances of variable_get('theme_default', 'bartik') in the codebase after applying this patch.
  3. Per my question in #37, do we need an update function? I guess most sites are expected to have their chosen 'theme_default' stored in the database, but there's no real reason they absolutely must, and if they don't this patch will currently wind up switching their site's theme from Bartik to Stark.
tstoeckler’s picture

+++ b/profiles/standard/standard.install
@@ -62,8 +62,22 @@ function standard_install() {
+  db_update('system')
+    ->fields(array('status' => 1))
+    ->condition('type', 'theme')
+    ->condition('name', $default_theme)
+    ->execute();
+  db_update('system')
+    ->fields(array('status' => 0))
+    ->condition('type', 'theme')
+    ->condition('name', 'stark')
+    ->execute();

Is there any reason this doesn't use theme_enable() theme_disable() ?

28 days to next Drupal core point release.

scor’s picture

Status:Needs work» Needs review
StatusFileSize
new10.98 KB
PASSED: [[SimpleTest]]: [MySQL] 33,768 pass(es).
[ View ]

@David_Rothstein #53:
1. I think that's what was suggested in #48 (separate issue)
2. fixed in attached patch
3. still open

@tstoeckler good point. this was pointed out during BADCamp but we forgot to file it. fixed in this patch.

David_Rothstein’s picture

1. I think that's what was suggested in #48 (separate issue)

Yes, separate issue... but I interpreted that to mean this issue would result in Minimal continuing to use Bartik. Otherwise when this patch is committed, Minimal will have switched to Stark whether we wanted it to or not.

On the other hand it's less code to do it that way and we have lots of time before D8 is released to sort out a final decision...

sun’s picture

Status:Needs review» Needs work
+++ b/core/includes/theme.maintenance.inc
@@ -44,7 +44,7 @@ function _drupal_maintenance_theme() {
-    $custom_theme = variable_get('maintenance_theme', variable_get('theme_default', 'bartik'));
+    $custom_theme = variable_get('maintenance_theme', variable_get('theme_default', 'stark'));

This change for the maintenance page was intentionally omitted as per previous discussion on this issue.

To make sure it isn't changed again, we probably should add an inline comment explaining that we intentionally want to use Bartik as default maintenance theme.

+++ b/profiles/standard/standard.install
@@ -62,8 +62,14 @@ function standard_install() {
+  // Enable Bartik theme and set it as default theme instead of Stark.
+  // @see system_install()
+  $default_theme = 'bartik';
+  variable_set('theme_default', $default_theme);
+  theme_enable(array($default_theme));
+  theme_disable(array('stark'));

The same code needs to be copied into minimal.install.

Lastly, we're still missing a module update for system.module that ensures Bartik is configured as default theme if not configured; e.g.:

/**
* Set Bartik as default theme if no default theme is configured.
*/
function system_update_800x() {
  if (!variable_get('theme_default', FALSE)) {
    variable_set('theme_default', 'bartik');
  }
}
sun’s picture

Status:Needs work» Needs review
StatusFileSize
new12.82 KB
PASSED: [[SimpleTest]]: [MySQL] 34,322 pass(es).
[ View ]

Attached patch resolves all remaining issues.

sun’s picture

Final reviews, anyone?

Jacine’s picture

Status:Needs review» Reviewed & tested by the community

I just tested this in the following ways:

Fresh install of each of the following combos:

  1. Minimal: Bartik was the default theme and the admin theme.
  2. Standard: Bartik was the default theme and Seven the admin theme.
  3. Testing (Manually removed hidden = TRUE removed from the testing profile and installed with that): Stark was both default and admin theme.

With each install I did the following:

  • Ran Form API: Element processing, System: Block functionality & Theme: Theme API tests locally
  • All tests passed.
  • Verbose output of tests showed Stark was used.

My only question, which should not hold this up is, does the follow-up issue for this already exist?

+++ b/core/includes/theme.maintenance.incundefined
@@ -44,6 +44,12 @@ function _drupal_maintenance_theme() {
+    // @todo Should use the actual default theme configured, but that depends on
+    //   configuration being available while possibly not having a working
+    //   database connection (yet). And only if that fails, should fall back to
+    //   Stark otherwise. Since there is no low-level access to configuration
+    //   currently, we only consult settings.php and fall back to Bartik
+    //   otherwise, as it looks generic enough and way more user-friendly.
Dries’s picture

I'd like to review this before this gets committed. I'm still not convinced we want to make Stark the default theme.

Specifically, I want to make sure this is only for the minimal profile. Based on the issue description that is the case, but based on the title not necessarily.

sun’s picture

Title:Make Stark the default theme» Change theme_default variable to Stark

@Dries: This patch no longer touches the profiles, neither Standard nor Minimal.

This patch essentially changes the value of the theme_default variable to Stark.

The most important impact of this change is that the Testing profile uses Stark, which in turn means that functional tests no longer implicitly test the Bartik theme as that is fundamentally wrong (That is, as long as tests use the Testing profile - of which we badly need more tests).

yoroy’s picture

Yeah, my understanding is that change does not put Stark in use in the core profiles. Only if all else fails, Stark is defined as the fallback (which helps with testing). Standard and Minimal still define their respective defaults Bartik/Seven and Bartik as their default themes.

Dries’s picture

Status:Reviewed & tested by the community» Fixed

OK, I had a chance to review this patch and everything looks good. I originally misunderstood the intent of the patch. Committed to 8.x! :)

xjm’s picture

Note that the summary for this issue does not reflect the actual change. Less of a concern now that the patch has been reviewed and committed, but we might want to correct it for posterity.

sun’s picture

Issue tags:+Testing system

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

Anonymous’s picture

Issue summary:View changes

Updated issue summary.