$site redundant in theme('page') [#27791]

Comment	File	Size	Author
	phptemplate_redundant_var_patch.txt	815 bytes	robertDouglass

Comment #1

robertDouglass CreditAttribution: robertDouglass commented 29 July 2005 at 18:06

Assigned:

Unassigned

» robertDouglass

Log in or register to post comments

Comment #2

Steven CreditAttribution: Steven commented 29 July 2005 at 21:24

Bluemarine declares $theme as global, but as far as I can tell it doesn't use it anywhere. I think we can safely remove it?

Log in or register to post comments

Comment #3

robertDouglass CreditAttribution: robertDouglass commented 30 July 2005 at 10:07

agree. It can be removed.

Log in or register to post comments

Comment #4

Dries CreditAttribution: Dries commented 30 July 2005 at 14:05

I think it is being used in bluemarine/page.tpl.php:
l(t('edit secondary links'),'admin/themes/settings/' .$theme)

Please double check.

Log in or register to post comments

Comment #5

Steven CreditAttribution: Steven commented 30 July 2005 at 14:53

Status:

Needs review

» Fixed

This is not present in the core bluemarine, I removed it.

Log in or register to post comments

Comment #6

Steven CreditAttribution: Steven commented 30 July 2005 at 15:01

Status:

Fixed

» Needs review

Sorry the two issues ($site and $theme) got mixed up. I probably just pasted in the wrong tab :P.

$site is being used by several contrib modules... $site_name only applies to the main name when displayed as a logo. Contrib usages show "Welcome to $site". It's probably a good idea to keep it, no?

Log in or register to post comments

Comment #7

robertDouglass CreditAttribution: robertDouglass commented 30 July 2005 at 15:36

I still think it is redundant and that the contrib usages should be updated. In fact, the example that you quote is really a bug because it overrides the toggle presented by the theme. $site_name is exactly the same text, but only gets shown if the administrator has toggled it on. It seems to me like PHPTemplate engine should only offer $site_name, and if the theme really *wants* to override the toggle, they can still call variable_get. I think the theme creator in the contrib example was confused by the redundancy.

Log in or register to post comments

Comment #8

Steven CreditAttribution: Steven commented 30 July 2005 at 17:12

Status:

Needs review

» Fixed

Well, it's not really a bug... it's merely a variable that is available for use. $site_name incorporates the wish to display the site_name as a title in the header. $site is used in alt attributes and whatnot.

Still I got rid of it. Please add a note to the theme upgrading guide about this though.