The error:

Notice: Undefined variable: site_name in include() (line 32 of /Users/jbeach/code/acquia/repos/engineering/gardens/trunk/docroot/themes/bartik/templates/maintenance-page.tpl.php).

Steps to reproduce.
1. Run update.php on a site or install.php
2. On the site configuration page, a DSM error will appearing complaining that $site_name is undefined.
3. Check the dblog after installation and note that a watchdog error exists for the undefined variable $site_name in maintenance-page.tpl.php

In bartik_preprocess_maintenance_page, $site_name is unset on site install:
https://skitch.com/jesse.beach/ggwpi/acquia-gardens-trunk-netbeans-ide-7...

CommentFileSizeAuthor
#23 bartik_maintenance_page-1334344-23.patch797 bytesDevin Carlson
#21 bartik_maintenance_page-1334344-21-d7.patch797 bytesjessebeach
#17 bartik_maintenance_page-1334344-16.patch797 bytesmstef
#9 bartik_maintenance_page-1334344-9.patch1.25 KBjessebeach
#5 bartik_maintenance_page-1334344-5.patch979 bytesjessebeach
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

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

Posting on behalf of Jesse since she is behind some sort of evil anti-Drupal.org force-field atm:

The regression was introduced in http://drupalcode.org/project/drupal.git/commitdiff/ea1bd8becea0a4b214cb... (#790556: Make the Maintenance Page Kick Ass)

webchick’s picture

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

And reviewing that issue, this was introduced at #790556-65: Make the Maintenance Page Kick Ass. Quoting:

I think it's totally weird to display the "site-name" when the db is offline, and there is no site name / logo coming from the db. What's "Drupal" doing there instead?

So I think we need to set this to an empty string, rather than unsetting it, and a comment that explains why the heck we are doing that would be wonderful.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Issue tags: +Novice

This should be pretty straight-forward.

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented
Version: 7.x-dev » 8.x-dev
Assigned: Unassigned » jessebeach

Setting to 8.x dev. The patch will need to be ported to 7.x

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented
FileSize
bartik_maintenance_page-1334344-5.patch979 bytes

changed 

function bartik_preprocess_maintenance_page(&$variables) {
  if (!$variables['db_is_active']) {
    unset($variables['site_name']);
  }
  drupal_add_css(drupal_get_path('theme', 'bartik') . '/css/maintenance-page.css');
}

to

function bartik_preprocess_maintenance_page(&$variables) {
  if (!$variables['db_is_active']) {
    $variables['site_name'] = '';
  }
  drupal_add_css(drupal_get_path('theme', 'bartik') . '/css/maintenance-page.css');
}

$variables['site_name'] is still set, but it now empty, so the check for it in maintenance-page.tpl.php returns FALSE instead of failing and throwing an error.

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented
Status: Active » Needs review

marking as need review

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs review » Needs work

Yep, that looks right. can you toss a one-liner comment in there too above if (!$variables['db_is_active']) { about why we're doing that?

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented

Adding tag, see #1334342: During update, Bartik throws an error that $site_name is not defined

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented
Status: Needs work » Needs review
FileSize
bartik_maintenance_page-1334344-9.patch1.25 KB

Comments added. I think this is why this code exists, but tim plunkett would be the person to ask for sure.

Kisugi Ai’s picture

Kisugi Ai CreditAttribution: Kisugi Ai commented

Thanks Works for me

but i have a anothe question. i have two drupal installations the one use bartik as maintenece and the other uses seven as maintenance page where is this settet?

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

The last submitted patch, bartik_maintenance_page-1334344-9.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Needs work » Needs review
Issue tags: +Novice

#9: bartik_maintenance_page-1334344-9.patch queued for re-testing.

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented

Should I just load the 7.x version of the patch here?

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented

Alyx Vance, this patch only affects Bartik, not Seven.

Kisugi Ai’s picture

Kisugi Ai CreditAttribution: Kisugi Ai commented

thats i know its has not to do with seven.
my confusion is thats two Drupal7 installationen use two diffrent maintanance themes.
thats only an question why. i have found nothing to setup this.
eg. the xy.com use bartik and the yx.com use seven.

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented

Let's make sure we don't hijack this issue with unrelated questions.

But just to answer this one, you can set your admin theme at the bottom of the Appearance tab form:

https://skitch.com/jesse.beach/gedhf/rockmelt-appearance-d8.drupal.dev

mstef’s picture

mstef CreditAttribution: mstef commented
FileSize
bartik_maintenance_page-1334344-16.patch797 bytes

Rerolled patch #9 to apply from a standard Drupal installation root - needed for drush make. (7.x)

Status: Needs review » Needs work

The last submitted patch, bartik_maintenance_page-1334344-16.patch, failed testing.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs work » Needs review

#9 still needs review for D8.

#17 is for D7

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Reviewed & tested by the community

Heh, I remember rerolling that original patch to unset it. Good catch.

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented
FileSize
bartik_maintenance_page-1334344-21-d7.patch797 bytes

Rolled for D7. I just added '-d7.patch' to the patch in 17 to befuddle Testbot.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Needs review

Works for me and doesn't look testable. Committed/pushed to 8.x, moving back to 7.x

Devin Carlson’s picture

Devin Carlson CreditAttribution: Devin Carlson commented
FileSize
bartik_maintenance_page-1334344-23.patch797 bytes

The patch in #21 applied cleanly and made the same change as the original patch in #9.

I've uploaded the same patch as in #21 but with a different file name in order to trigger the test bot.

Devin Carlson’s picture

Devin Carlson CreditAttribution: Devin Carlson commented
Status: Needs review » Reviewed & tested by the community
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x. Thanks!

Status: Fixed » Closed (fixed)

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

cweagans’s picture

cweagans
He/Him
Primary language English
Location Boise, ID, USA
CreditAttribution: cweagans commented
Issue tags: +Needs backport to D7

Updating tags per http://drupal.org/node/1517250