Add switching theme based on browser.

Rob Loach - May 29, 2008 - 02:53
Project:Switchtheme
Version:5.x-1.x-dev
Component:Code
Category:feature request
Priority:normal
Assigned:Unassigned
Status:needs work
Description

This patch uses Browscap to detect the browser, and then change the theme based on what browser they're using. It only allows the functionality if the Browscap module is enabled.....

You'll need browscap for Drupal 6.

#1

Rob Loach - May 29, 2008 - 02:53

Oops, forgot the patch.

AttachmentSize
switchtheme_browser.patch 7.05 KB

#2

sun - September 18, 2008 - 23:10
Title:Add Switching Theme based on Browser» 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. :)

AttachmentSize
switchtheme-DRUPAL-6--1.browscap.patch 7.01 KB

#3

sun - September 18, 2008 - 23:22
Status:needs review» fixed

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

#4

Anonymous (not verified) - October 2, 2008 - 23:32
Status:fixed» closed

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

#5

panji - November 11, 2008 - 05:16
Title:Add switching theme based on browser» Add switching theme based on browser,

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?

#6

panji - November 27, 2008 - 21:15
Title:Add switching theme based on browser,» Add switching theme based on browser, Screen Resolution, Java, Flash and other plugin
Status:closed» active

creating on new issue for it...

#7

sun - November 18, 2008 - 21:32
Status:active» closed

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.

#8

panji - November 27, 2008 - 21:19
Title:Add switching theme based on browser, Screen Resolution, Java, Flash and other plugin» Add switching theme based on browser.

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

#9

jameselkins - December 3, 2008 - 16:55
Version:6.x-1.x-dev» 5.x-1.x-dev
Status:closed» needs work

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.

AttachmentSize
switchtheme_D5_browser.patch 8.08 KB

#10

jameselkins - December 3, 2008 - 17:31

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

AttachmentSize
switchtheme_D5_browscap.patch 4.74 KB

#11

sun - December 3, 2008 - 18:11

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

#12

jameselkins - December 3, 2008 - 20:04

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?

AttachmentSize
switchtheme_D5_browscap.patch 5.97 KB

#13

sun - December 3, 2008 - 21:32

@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.

#14

Rob Loach - February 22, 2009 - 08:05
Version:5.x-1.x-dev» 6.x-1.x-dev
Status:needs work» patch (to be ported)

Wheeeee!

#15

sun - February 22, 2009 - 18:39
Version:6.x-1.x-dev» 5.x-1.x-dev
Status:patch (to be ported)» needs work

#16

pyctures - April 28, 2009 - 07:58

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.

#17

srinikasturi - July 16, 2009 - 16:26

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!

#18

sun - July 17, 2009 - 00:22

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?

#19

srinikasturi - July 17, 2009 - 11:20

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.

#20

MJL - July 17, 2009 - 17:19
Version:5.x-1.x-dev» 5.x-1.3

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.

#21

sun - July 17, 2009 - 17:30
Version:5.x-1.3» 5.x-1.x-dev

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.

#22

MJL - July 17, 2009 - 17:53
Version:5.x-1.x-dev» 5.x-1.3
Category:feature request» support request
Status:needs work» patch (to be ported)

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.

#23

MJL - July 17, 2009 - 17:49

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

#24

sun - July 17, 2009 - 17:56
Version:5.x-1.3» 5.x-1.x-dev
Category:support request» feature request
Status:patch (to be ported)» needs work

Resetting version, category, and status to proper values.

#25

srinikasturi - July 18, 2009 - 12:08

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.

AttachmentSize
switchtheme_with_browscap_patch.zip 14.01 KB

#26

srinikasturi - July 24, 2009 - 11:42

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 is a registered trademark of Dries Buytaert.