This theme does not output the $head variable like every phptemplate theme should. From the changelog, it appears this was a heavy-handed attempt at dealing with drupal.css problems. It appears that all the drupal.css styles were copied into modules.css and modified.

Drupal.css should never, never, never be removed in this way, as it destroys the functionality and styling of a variety of modules. Here's a short list: htmlarea, fckeditor, tinymce, quote, img_assist. It also prevents forum, blog, and taxonomy RSS entries from being properly reported in the head section. In short, anything that any Drupal module wants to place in the head section is now impossible to accomplish. The assumption that the $head variable contains only drupal.css is a very dangerous one to make.

If you need to override various styles in drupal.css (which I'm assuming you do), you should do so by modifying your own CSS, which you should reference AFTER the $head variable.

In short, please add:
<?php print $head ?>

BTW: This is comparable to this issue from the SpreadFirefox theme.

P.S. If this was an accidental mistake, sorry for being so long-winded about it. I was trying to be persuasive. ;-)

Comments

kbahey’s picture

kbahey commented
Assigned: Unassigned » kbahey

Hi TDobes.

THanks for pointing this out. I remember fixing something similar in SpreadFireFox after you pointed it out.

If you have a patch, that would be appreciated.

Thanks

kbahey’s picture

kbahey commented

I did some tidying up in all three Chris Messina themes (SpreadFireFox, Democratica and Lincoln's Revenge), as per TDobes advice.

TDobes, please take a look and let me know if something is still missing.

TDobes’s picture

TDobes commented

kbahey: Thanks for your work on this. I took a quick look... I noticed a few things:
* Democratica and Lincoln's Revenge: <meta http-equiv="Content-Type" content="text/html; charset=utf-8" /> is already displayed from the "print $head;" line... it can be safely removed from the page.tpl.php files.
* All Three: the base href tag is already output by "print $head;"... it is unnecessary to explicitly place it in page.tpl.php. I believe this should remove the requirement for accessing the $base_url global too.
* Democratica and Lincoln's Revenge: The style.css file is referenced by the "print $styles;" line. style.css should not be referenced manually in the theme, as this effectively defeats Drupal's ability to style themes. Drupal will normally output the style.css file for the theme on the "print $styles" line, but can also reference a custom stylesheet instead for sites that want to do things like customize the color scheme or make minor CSS modifications.
* All Three: For the reason stated above, it is also perferrable that "print $styles" come AFTER all other stylesheets which are referenced by the theme. This way, if a site designer wants to further customize their site's style by overriding some of the theme's other CSS (i.e. rules in modules.css), they can do so without needing to manually edit the theme.
Aside: The ability to create a style rather than directly modifying the theme makes the upgrade path much simpler for site admins when new versions of Drupal and/or these themes are released. That is why this method of customization is perferrable.
* Democratica: There seems to be a "styles.css". I recommend you rename this to style.css and remove all mention of either file in page.tpl.php... as mentioned above, Drupal will include the style.css automatically with "print $styles;"
* Democratica: When Drupal references the style.css file, it does so with media="all", so the media="print" lines which just reference style.css can be removed as well.
* You might want to check with Chris on the CDATA stuff you removed from Democratica... I'm not really sure why that was there.
* All Three: I don't think you need to manually reference the favicon. It looks like the latest versions of the phptemplate engine automatically check for the existence of a favicon.ico file in the theme directory and, if it exists, references it in the "print $head" output. However, phptemplate.engine doesn't seem to prepend the favicon with $base_url like you're doing... maybe this is a problem? If so, let me know and I'll file a bug (with patch) for phptemplate. The exact code phptemplate uses is: "<link rel=\"shortcut icon\" href=\"" . path_to_theme() . "/favicon.ico\" />\n"

Thank you for spending the time to standardize these themes and extend their potential use to a wider variety of situations. If anything I wrote above is confusing or you'd like me to provide a patch or two, just ask.

kbahey’s picture

kbahey commented

Thanks TDobes for all the detailed comments.

I fixed them, and commited them to the repository. They have been tagged for 4.5, so they are available to those who need them.

Take a look and check if any other thing need to be done.

Anonymous’s picture

(not verified) commented