Errors and erroneous warming when admin theme is 'default' [#687210]

Set the admin theme to 'Default theme', then set another theme as default.

You get:

    *  Notice: Undefined index: 0 in system_theme_default() (line 436 of /Users/joachim/Sites/7-drupal/modules/system/system.admin.inc).
    * Notice: Trying to get property of non-object in system_theme_default() (line 436 of /Users/joachim/Sites/7-drupal/modules/system/system.admin.inc).

Status message
Please note that the administration theme is still set to the theme; consequently, the theme on this page remains unchanged. All non-administrative sections of the site, however, will show the selected Bartik theme by default.

Comment	File	Size	Author
#11	687210-3.drupal.admin-theme-error-message.patch	1.31 KB	joachim
#11
#9	687210-2.drupal.admin-theme-error-message.patch	1.31 KB	joachim
#9
#2	687210.drupal.admin-theme-error-message.patch	1.11 KB	joachim
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

aspilicious CreditAttribution: aspilicious commented 17 January 2010 at 12:55

o_O

Cannot reproduce this, I set seven to default, then garland back to default... no notices

Comment #2

joachim CreditAttribution: joachim commented 17 January 2010 at 15:26

Status:

Active

» Needs review

File	Size
687210.drupal.admin-theme-error-message.patch	1.11 KB

You need to have the 'Admin theme' set to 'Current theme'.

Comment #3

aspilicious CreditAttribution: aspilicious commented 17 January 2010 at 15:34

I find this situation strange

       $admin_theme = variable_get('admin_theme', 0);
-      if ($admin_theme != $theme) {
+      if ($admin_theme && $admin_theme != $theme)

Your patch checks if variable_get('admin_theme', 0); returns a good value...
I don't have any doubts that this is working BUT variable_get('admin_theme', 0); should always return a good value doesn't it?
How can it be that there isn't a admin theme o_O?

Comment #4

joachim CreditAttribution: joachim commented 17 January 2010 at 16:18

If the variable is 0 you are then trying to get that key out of the $themes array. There is no 0 key. Hence the error.

Comment #5

aspilicious CreditAttribution: aspilicious commented 17 January 2010 at 16:35

Thats what I mean, how can that variable be 0...
Isn't that plain wrong? Or am I missing something?

Comment #6

joachim CreditAttribution: joachim commented 17 January 2010 at 17:24

It'll be 0 if it is not already set in the DB, or if the option 'Default theme' is selected:

  $form['admin_theme']['admin_theme'] = array(
    '#type' => 'select',
    '#options' => array(0 => t('Default theme')) + $theme_options,
    '#title' => t('Administration theme'),
    '#description' => t('Choose "Default theme" to always use the same theme as the rest of the site.'),
    '#default_value' => variable_get('admin_theme', 0),
  );

Comment #7

aspilicious CreditAttribution: aspilicious commented 17 January 2010 at 17:28

Hmmm, I see...

Comment #8

naxoc CreditAttribution: naxoc commented 15 February 2010 at 13:05

Status:

Needs review

» Needs work

I tested the patch and it makes the notice go away. Even if the '0' is a little confusing that is in fact what it is set to in the db if the default theme is chosen. Maybe throw in a comment to explain the check for 0?

Comment #9

joachim CreditAttribution: joachim commented 15 February 2010 at 13:09

Status:

Needs work

» Needs review

File	Size
687210-2.drupal.admin-theme-error-message.patch	1.31 KB

Ok -- I've made it an explicit check to be not 0, and added a line to explain what the (prior to this patch!) variable_get() is doing for the default value.

Comment #10

naxoc CreditAttribution: naxoc commented 15 February 2010 at 13:24

Status:

Needs review

» Needs work

Looks good :) Now the only nitpicky thing I can think of is that there should either be a period after // The status message depends on whether an admin theme is currently in use or the next line of comment should not have a capital first letter.