I just added a mobile property to the themekey so I can switch my theme for mobile phone users.

That works great, no problem there.

But at some point I had a suspicion... and it was correct. I added a drupal_set_message() in my implementation of hook_themekey_global() and it gets called every time I access the website. That means each single hook is called whether or not the variables it includes are required.

It's not too bad in my case since I can skip the installation of my sub-module. However, that means all your sub-modules (book, comment, locale, node, system, taxonomy, user) hook is called and all the code there is executed. I'm not too sure how you could avoid calling all of those. My idea is that if none of the attributes defined in a module are used in the list of theme switching rule chain, then its hook shouldn't be called. I guess that would make the module a tad bit more complicated, but it could be a great optimization.

I say that because the test for mobile user agents is quite a bit of data to load each time. Although, really I save the data in a cookie so it's not so bad (it won't reload everything each time.)

Thank you.
Alexis Wilke

Comments

mkalkbrenner’s picture

It's true that implementations of hook_themekey_global() are called on every page request.

But the optimization you describe, to only run some code if the related property is used in a rule, already exists.
Instead of implementing hook_themekey_global() you can define a callback function in hook_themekey_properties(). That's the way most properties in ThemeKey and ThemeKey Properties are implemented.

ThemeKey 7.x-1.x contains an example module. Please have a look at this example.

Nevertheless, maybe we should improve the documentation.

AlexisWilke’s picture

That would be an idea... 8-)

I checked the contents of the README.txt file and cannot see what you are talking about. Also, if that's the case, shouldn't the node, user, etc. make use of that optimization?

And note that I work with version D6.x 3.2, not 7.x... (and yes, I will upgrade to 3.3)

Thank you.
Alexis Wilke

P.S. My extension module is here: http://drupal.org/project/mobilekey
Dunno if you'd want to do so, but maybe you could mention it on your project page?

mkalkbrenner’s picture

I checked the contents of the README.txt file and cannot see what you are talking about. Also, if that's the case, shouldn't the node, user, etc. make use of that optimization?

That's what they do! The properties are not set using hook_themekey_global() but through callback functions. Please have a look at the implementations at modules/themekey.ENTITY.inc.

I had a look at your MobileKey module. I'm convinced that the functionality could be achieved in an easier and more efficient way:

function mobilekey_themekey_properties() {
  // Attributes for properties
  $attributes = array();
  $attributes['mobile:device'] = array(
    'description' => t('Mobile: Device - the name of device, "desktop", "mobile", or a specific group such as "blackberry" or "ipad". All desktops are mark "desktop".'),
    'validator' => '',
    'page cache' => THEMEKEY_PAGECACHE_SUPPORTED, // FIXME I'm convinced that page caching won't be supported
  );

  // Mapping functions
  $maps = array();
  $maps[] = array(
    'src' => 'system:user_agent',
    'dst' => 'mobile:device',
    'callback' => 'mobilekey_user_agent2device');

  return array('attributes' => $attributes, 'maps' => $maps);
}

Therefore you have to add a dependency to the module ThemeKey Properties as well.
BTW I see a lot of redundancy to some properties provided by this module. Maybe you should consider to make some use of them to reduce your code. We already detect Operating Sytems and Browsers. So the device is not that far away.

AlexisWilke’s picture

Yeah, actually the cache is a huge problem. I use boost and this is just not working at all... that is, I want the front page to be cached at all time. If cached by boost, mobile devices will see the cached version and thus not the expected version. For this to work right, I need to add the test directly in Apache2, which sucks since I'm running with a multi-site install and that means I cannot do said change...

Thank you for the feedback, much appreciated. I guess I need to test your 3.3 version 8-)
Alexis Wilke

mkalkbrenner’s picture

Status: Active » Closed (fixed)