Add switching theme based on browser.

Finally had time to review this patch. I've cleaned-up some minor parts and reworded some strings. Should still work, however, needs testing.

Also, thanks for fixing that stuff we missed in the port to 6.x. :)

Heh... I accidentally committed the administration form in switchtheme.admin.inc already, so I've also committed the rest of this patch now.

Thanks Sun,...

Great Patch,.. it's work,.. nice,... so I can configure for mobile browser, and other browser independently,...
But now,.. can we make some little improvement by detecting java and flash plugin? when the browser doesn't have java installed or flash installed, we can make auto switch for the themes..

Regards,..

Because it's look like the same,.. is it ok to reopen the issue? and add " (+ js, and flash)" on the title?

creating on new issue for it...

It's unlikely that a JS-based detection will be added to SwitchTheme. Anyway, please open a new issue for this, since this issue performs a server-side detection.

Thanks,.. sure,..
I'll make a new issue for it..

Hi,
I'm working on a D5 site so I needed this functionality. Here's my patch, so far it will bring up the settings page and insert the values into the variable table but will give a response like:

Warning: Table 'watchdog' was not locked with LOCK TABLES query: INSERT INTO watchdog (uid, type, message, severity, link, location, referer, hostname, timestamp) VALUES (1, 'php', 'Duplicate entry 'switchtheme_browser_7b74ce3f59567e65d9dc5c2c8743' for key 1\nquery: INSERT INTO variable (name, value) VALUES ('switchtheme_browser_7b74ce3f59567e65d9dc5c2c87432e8e', 's:7:\\"default\\";') in /Applications/MAMP/htdocs/drupal-5.12/includes/database.mysql.inc on line 174.', 2, '', 'http://localhost:8888/drupal-5.12/?q=admin/settings/switchtheme/browser', '', '::1', 1228321625) in /Applications/MAMP/htdocs/drupal-5.12/includes/database.mysql.inc on line 174

whenever I try to save the settings. Also, even though the name/values get put into the variable table, the browser detection code does not work - perhaps because it is in the _menu() hook and not the _init() hook? A good test browser is Safari vs. Firefox. The browser detection was in the _init() hook in the D6 version, but my understanding is that it's supposed to be done in the _menu($maycache) hook in D5. I've commented the code @ the places where I think it's having trouble - look @ lines 119 & 83. Thanks for the help.

Sorry, last attachment wasn't a patch, thanks for the heads up sun.

Sorry, this patch violates pretty much every coding standard mentioned in http://drupal.org/coding-standards

I ran the code through the coder module mentioned in the http://drupal.org/coding-standards site and eliminated all errors. Perhaps this will be better?

@jameselkins: Please click on the patch you attached and have a closer look at it - it contains wrong indentation all over the place, changes a bunch of lines which it does not needs to, aso.

I would like to provide a specific theme for iPhone on my Drupal 6 site.
I already installed Browscap.
Is your patch providing the behavior of switching from Default theme (set for PC) to Mobile theme (designed for iPhone) ?
How to install this patch ?
(I am a newbie in Drupal. I am able to install a module. I have never installed a patch).

Thanks for your help.

This patch works well. Thank you very much. Unfortunately, I've found that the search results (native Drupal as well as Apache Solr) and the profile page using APK pick up the theme marked as 'Default' in admin/build/theme, rather than the theme for that particular browser. Please can someone help?

Thanks!

Thanks for the re-roll! If you don't mind, I'll do a in-depth review here:

-    case 'admin/settings/modules#description':
+  case 'admin/settings/modules#description':

Wrong indentation.

       return t('Set a label for each enabled theme. This is what will be displayed to the user in the selection box.');
+      break;

Duplicate case statement termination. return is sufficient.

-      'title'    => t('switchtheme settings'),
+      'title'    => t('Switchtheme settings'),

We should not change translatable strings in existing versions.

       'callback arguments' => array('switchtheme_admin_settings')
+
     );

Needless blank line.

+  $items[] = array(
+        'path' => 'admin/settings/switchtheme/themes',
+        'title' => 'Themes',
+        'description' => 'Theme display settings for the Switchtheme block.',
+        'type' => MENU_DEFAULT_LOCAL_TASK,
+        'weight' => -10,
+    ); //This part works
+

Wrong indentation. I do not understand the comment after in there; it should go.

+  if (module_exists('browscap')) {
+    $items[] = array(
+      'path' => 'admin/settings/switchtheme/browser',
+      'title' => 'Browser',
+      'description' => "Configuration of switching the theme based on the visitor's browser.",
+      'callback' => 'drupal_get_form',
+      'callback arguments' => array('switchtheme_admin_browser_settings'),
+      'access' => array('administer switch'),
+      'type' => MENU_LOCAL_TASK,
+      'weight' => 2,
+    );
+      }
+    }

Clearly visible: Wrong indentation in closing braces.

+    //the browser code - apparently not working/distinguishing the browser.
+        if (module_exists('browscap') && variable_get('switchtheme_browser_enabled', FALSE)) {
+    $browser = browscap_get_browser();
+    if (isset($browser['parent'])) {
+      $parent = trim($browser['parent']);
+      $browser_theme = variable_get('switchtheme_browser_'. md5($parent), 'default');
+      if ($browser_theme != 'default') {
+        $custom_theme = $browser_theme;
+      }
+    }
+  }
+
   }
   return $items;

Same here.

+ * Form builder function for browser page; menu callback.
+ * This part shows up and retrieves the browsers from the browscap table properly, but chokes when you try to submit.

huh?

For #17, found a fix. Use the Module Weights module. http://drupal.org/project/moduleweight and set the switchtheme weight to -10 so that it runs before the search module and the other modules. That way, no matter what comes up, it will use the theme supplied by switchtheme.

Sun, there were some issues with the patch on here. It didn't work straight out of the box on our D5 install. We'll be uploading the fixed patch shortly. I am sure there will be coding convention issues, as its one of our first attempts at fixing Drupal code. Apologies in advance.

It would be great if the Switchtheme project team leader rolled into an Official 5.x-1.4 release of the Switchtheme module the updated code that enables Switchtheme to support the Browscap module so Drupal can switch themes based on a visitor's browser. (i.e., that incorporates the vetted changes from the final patch code).

NOTE: "Starting with Switchtheme 6.x-1.1, Switchtheme provides support for the Browscap module to switch themes based on the browser of a visitor." Reference: http://drupal.org/project/switchtheme

In light of the successful patch development for the 5.x dev branch, it appears that this functionality could be easily committed to an Official release, i.e., 5.x-1.4. If there is no time for a formal official release, it would be much appreciated if the final 5.x -1.3patched module code were posted here for clarification/convenience of the community.

Yes, this has a chance to be incorporated into the next official release for 5.x. However, I need a patch that applies the (very) same changes to 5.x. Note that I need the code to be as equal and comparable as possible. So, to get this in, the patch from #2 should be used as base. Most of the code for this functionality should be literally the same in 5.x and 6.x.

srinikasturi, please post your patch for Switchtheme 5.x-1.3. Did you use sun's #2 as a baseline? Is the Module Weights module required to make this work in D5?

sun indicates that an Official 5.x - 1.4 release should be feasible (sometime soon hopefully). Your contribution to this endeavor will be appreciated.

sun, this is very good to hear! Please let me know if I can help you further with this effort. Thanks

Resetting version, category, and status to proper values.

My colleague Santhosh Kumar worked on this, and I can request him to detail what exactly needed to change in the patch to make it work. In the meantime, here's the module as it is working on our site today. Visit www.peppervillage.com on IE6, Firefox and mobile to see 3 different themes in action. This uses patch # 2. Hope this helps.

What would be really cool is to have a set of themes available for each browser setting so that the original functionality of switchtheme - to be able to choose a different theme, is still available as a subset of themes enabled for the user's browser. Currently, if the browscap functionality is being used, the theme chooser doesn't work.

Drupal 5 is no longer supported with the release of Drupal 7.