Thank Acquia and Top Noch Themes you for this excellent theme. It has not only lot of features, but an excellent support (thx jwolf!!)

I would like to request Page Title module integration. Acquia Marina's own page titles is good, but having the possibility to set the title tag apart from the node title is sometimes a very useful feature, specially regarding SEO, and Page Title does a great job in this field.

Apart from that, this integration is quite easy to achieve. A simple "if" in template.php and modify theme_settings.php in a similar way with nodewords case to warn the user this module is active.

I have done the first part, this is the patch from ver. 6.x-1.5 in case you want implement it

--- template.php        2008-12-22 00:38:55.000000000 +0100
+++ template.php_patch  2008-12-26 12:15:04.906250000 +0100
@@ -236,7 +236,11 @@ function phptemplate_preprocess_page(&$v
   $mission = t(variable_get('site_mission', ''));
   $page_title = t(drupal_get_title());
   $title_separator = theme_get_setting('configurable_separator');
-  if (drupal_is_front_page()) {                                                // Front page title settings
+
+  if (module_exists('page_title')) {
+     $vars['head_title'] = page_title_page_get_title();
+   } else {
+   if (drupal_is_front_page()) {                                                // Front page title settings
     switch (theme_get_setting('front_page_title_display')) {
       case 'title_slogan':
         $vars['head_title'] = drupal_set_title($title . $title_separator . $slogan);
@@ -275,6 +279,9 @@ function phptemplate_preprocess_page(&$v
         }
     }
   }
+  }
+
+
   $vars['head_title'] = strip_tags($vars['head_title']);                       // Remove any potential html tags

   if (!module_exists('nodewords')) {

Comments

andrewfn’s picture

This is a greatly needed patch. I will have a go at testing it.

andrewfn’s picture

Status: Active » Needs work

patch no-longer applies

stephthegeek’s picture

Assigned: Unassigned » jwolf

We'll be adding this fix in shortly.

jwolf’s picture

jbrauer’s picture

Any update on this? Is there anything I could do to help? Would like to see this get committed if possible.

niklp’s picture

I gotta be honest, I apply this sort of thing manually, in general. I would deem it too specific to be of actual merit to the theme.

In Zen et al, this is normally catered for by allowing the theme to assign the page title, and then doing an additional check in the _preprocess_page to check for module_exists on page_title, then reassigning it there.

Adding lots of module specific code to themes is of little benefit in my opinion - it just makes it less user friendly.

Wouldn't a more sensible option be to just describe how to add the page_title functionality into the theme manually? It's pretty trivial.

jbrauer’s picture

I'm not sure that removing the page title functionality already in Acquia Marina would help. It has some very nice features. This issue is about adding the conditionals, similar to Zen et al. that defers the page title options to the Page Title module if it's i use. Otherwise it continues to use the settings available in the theme.

jwolf’s picture

Status: Needs work » Needs review
StatusFileSize
new5.92 KB

The attached patch has been committed to CVS.

jwolf’s picture

Status: Needs review » Fixed

Committed and will be in new dev snapshot and 1.6 release

Status: Fixed » Closed (fixed)

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