Support from Acquia helps fund testing for Drupal Acquia logo

Comments

wildflower_0002’s picture

Issue summary: View changes

fixed version we are using, i had dev instead of alpha1

imonemus’s picture

The problem is that Drupal caches pages by URL, which obviously remains the same when the theme is switched. Modules such as Mobile Tools seem to be attempting to override the caching and implement their own version.

It seems (IMHO) the easiest thing is to include the theme name in the CID (cache id). I tried hacking cache_set and cache_get in the core (!) and adding:

global $theme;
$cid .= '_' . $theme;

This appears to do the trick, but obviously you can't hack the core for a production site.

I read somewhere that it is possible to copy the cache.inc file from /include into the module directory, making the modifications there and then telling Drupal to use those functions by adding variable_set('cache_inc', 'sites/all/modules/mobile_switch/cache.inc') to the installation.

However, I can get it to pick up the new file! If someone feels like trying to beat me to a solution.........

acy76’s picture

I believe your cache modification may work if you switch cache backends in settings.php, as discussed in a slightly different context here.

Also, I have not tested this, but the approach to this same problem taken by the Context Mobile Detect module looks promising - if you dissect their code, they add a "preboot" function to the module and include/call it directly from settings.php (that file has to be modified for this to work, of course) which does device detection and modifies the cache hash string accordingly. The function to take a close look at in that module is called context_mobile_detect_preboot().

I have not decided whether I will attempt to build a single, responsive theme and use two Varnish cache bins or employ mobile theme switching, but if I go the latter route and a patch implementing the above method is in order, I will post here.

imonemus’s picture

Thanks, acy76. I have now tried switching cache backends in settings.php and extending DrupalDatabaseCache to accommodate my own modifications. The function overriding works fine, but in testing I have discovered that DrupalDatabaseCache::get and DrupalDatabaseCache::getMultiple appear to be called before global $theme is set, leaving my solution pretty much dead in the water. It's probably therefore necessary to use some sort of preboot method. However, I'm beginning to feel it would be easier to get in the habit of using adaptive/responsive themes!

Caching should in general be specific to theme, language, and sometimes user or role. I wonder if this is really a core fault. For anyone who wants to persue this, there is a function drupal_render_cid_parts in core (common.inc) which looks like it should do the necessary but appears not to get called. I'm way out of my depth.

KarlKedrovsky’s picture

I've run into this problem on a recent site build and I thought I'd share a partial solution I've come up with just in case it might help someone else. In my case I needed to cache pages in three different categories - on the desktop using the main theme, on mobile using the mobile theme and on mobile using the main theme. The solution I've come up with is to subclass the DrupalDatabaseCache class and append theme and device information to the cid. I just whacked this together this morning and I've only tested it a small amount so I'm not sure it's 100% correct or the best implementation. If anyone has any feedback or opinions I'd love to here it.

The solution involves creating a new caching provider that I put in sites/all/modules/custom/mysite_misc/cache.inc:

<?php

/**
 * @file
 * Provides a cache implementation to be used in conjunction with the
 * mobile_switch module.
 *
 * This is only intended to be used as a replacement for the page cache.
 * It appends the theme and whether we're on a mobile device or not to
 * the cache id.
 */

class MysiteCache extends DrupalDatabaseCache implements DrupalCacheInterface {

  /**
   * Overrides DrupalDatabaseCache::get().
   */
  function get($cid) {
    $cid = $cid . $this->_get_extra_cid();
    return parent::get($cid);
  }

  /**
   * Overrides DrupalDatabaseCache::getMultiple().
   */
  function getMultiple(&$cids) {
    $new_cids = array();
    if (is_array($cids)) {
      foreach ($cids as $cid) {
        $new_cids[] = $cid . $this->_get_extra_cid();
      }
    }
    return parent::getMultiple($new_cids);
  }

  /**
   * Overrides DrupalDatabaseCache::set().
   */
  function set($cid, $data, $expire = CACHE_PERMANENT) {
    $cid = $cid . $this->_get_extra_cid();
    return parent::set($cid, $data, $expire);
  }

  /**
   * Overrides DrupalDatabaseCache::clear().
   */
  function clear($cid = NULL, $wildcard = FALSE) {
    if ($cid != NULL) {
      $cid = $cid . $this->_get_extra_cid();
    }
    return parent::clear($cid, $wildcard);
  }

  function _get_extra_cid() {
    global $theme_key;
    $browser = mobile_switch_mobile_detect();
    $extra = '';
    if (!empty($theme_key)) {
      $extra .= ':' . $theme_key;
    } else {
      $extra .= ':notheme';
    }
    if (is_array($browser) && $browser['ismobiledevice']) {
      $extra .= ':mobile';
    } else {
      $extra .= ':notmobile';
    }
    return $extra;
  }

}

To use the new caching provider (for pages only) add this to the bottom of your settings.php file:

$conf['cache_backends'][] = 'sites/all/modules/custom/mysite_misc/cache.inc';
$conf['cache_class_cache_page'] = 'MysiteCache';

Like I said, I've just put this together and I'm still testing but I thought others might benefit from the solution and I'd love some feedback. Obviously this will only work if you're using the database cache that comes in core but it might be a good start at something more generic.

KarlKedrovsky’s picture

Status: Active » Needs review
FileSize
2.92 KB

I did a bit more testing and created a patch to add a new database cache provider.

rfc2460’s picture

#5 works very well for me. Thanks !

KarlKedrovsky’s picture

I found an issue with the patch I included in #5, the theme information isn't available when the call to get the cache value is made, although it is when you set it. I've modified the way I added the extra information to the cache id and cleaned up the rest of the cache provider quite a bit. I'll be doing some additional testing but I'd love any feedback folks might have.

rfc2460’s picture

For me it works (but #5 was also ok :)). Thanks.

imonemus’s picture

parent::get($cid) calls getMultiple, so aren't you adding the extra cid information twice?

Just a thought.

KarlKedrovsky’s picture

Ya, the "solution" in #4 was more than a little naive. The patch in #7 is quite a bit simpler and takes into account the limitations of being so early in the request handling process.

imonemus’s picture

Yes, I see. Sorry! As you say #4/#5 don't work.

#7 does indeed work and gets around the problem that caused me to abandon in #3

Thank you!

It is a shame to have to test for mobile/non-mobile rather than just add the theme key since this would provide a more generic solution that could be used by a number of other theme-switching modules which appear to be exhibiting the same shortcomings. But this will do.

jfama’s picture

Thank you for #7 my good sir!

bkenro’s picture

Thank you for the valuable patch.

When I set the 'Tablet device' setting 'no' in 'BASIC SETTINGS' conf page and clear the cache, anonymous access with tablet device seems to create the cache entry with the desktop content value for the ':mobile' key (cache id).

I think the getExtraCid() method should take account of the 'Tablet device' setting.

KarlKedrovsky’s picture

Here's an updated patch that takes tablets into account, caching pages for "mobile", "tablet" and "standard" devices independently. I think this will take care of the issue raised in comment #13 but let me know if I've misunderstood something.

jl.images’s picture

Just chiming in to confirm that #14 works brilliantly for me.

Rodlangh’s picture

Are you all sure this works?
I've got a similar issue where I need different cache for desktop and mobile devices for another module. Stumbling on this page I took the solution from #7 and altered it to my needs.

Checking the cache_page table, I saw that the different caches were created, so far so good. However, whenever I reloaded a page, the browser didn't get the cached page and just loaded everything from scratch and saved a new cache (cache created timestamps changed).

Seems like the getMultiple(&$cids) function gets called so early, that it cant find the method that handles the mobile devices check. Because of this it will search for a cache with a cid that's formatted like a normal url (which doesn't exist), in stead of [url]:mobile / [url]:notmobile.

I can get the cache to work with a required_once to the files that will handle the mobile checking, but it's not the most elegant fix.

Anyone else noticed this with perhaps a better fix?

KarlKedrovsky’s picture

I just re-tested the patch, making sure that the create dates in the cache table didn't change, and it's working fine for me (doesn't mean that I'm not missing something though). I had a very similar issue to the one you're describing in an earlier version of my patch that tried to use the theme name as part of the cache key. That worked fine when creating the cache entry but at the point in the request handling process when it was checking the cache (very early) the theme name was not available. If you take a look into the core code that calls the function to check the cache you'll find that it's so early that you really can't depend on much of anything being available from a Drupal perspective. As I thought about it for a bit it made sense, the whole idea behind using a cache was to avoid all the processing Drupal does.

What I ended up doing is depending only on information that the Mobile_Detect library provided and the cooking information that this module sets. Any time I tried to use any Drupal specific information that's created during the request processing I saw the behavior you described in comment #16.

Rodlangh’s picture

Thanks for re-testing! I think I figured it out: the Mobile Switch module implements the hook_boot. Because of this the module will be loaded early enough for the mobile_switch_mobile_detect() function to be available when the cache gets loaded.

My implementation doesn't define the boot hook, so my module won't be loaded that early. A simple drupal_load() in the getExtraCid() resolves that problem.

Thanks again for your patch, it helped me a lot!

bradjones1’s picture

Version: 7.x-2.0-alpha1 » 7.x-2.x-dev
acy76’s picture

Not to pollute the issue queue, but I just wrote a module that seems to address the caching issue discussed above without the need to replace the cache backend (useful for people already running memcached or similar).

I think my module would enable caching in concert with other mobile modules that lack it (in other words, it *should* fix the caching issue discussed here).

I also added some simple mobile theme switching capability, although it can be disabled.

It's available for review etc. in my sandbox if anyone is looking for an alternate approach.

Please test and give feedback if you do - keep in mind, though, it's experimental.

lorenz’s picture

I have the same problem with the 6.x-1.0-beta1+1-dev version of mobile switch. Will there be a patch for the 6er version of this otherwise great module?

Thanks

imoreno’s picture

#14 works for me with one exception, it does not work / recognize the front page for some of the mobile devices.

works very well on all other pages.

bradjones1’s picture

@imoreno - can you be more specific and post steps to reproduce that error/behaviour?

imoreno’s picture

what i can think of that might be difernt is the fact that i"m using panels, with override for the path

www.mysite.com/frontpage

Is this might be the problem as the cache is aiming to drupal native frontage url only like www.mysite.com/ ?

any thoughts will be helpful.

BR
Itzhak

majusz’s picture

Thanks for the patch in #14! Works fine for me.

Mr.Echo’s picture

The instructions should include a warning to users of the Domain Access module.

If you have this line in your settings.php:

include DRUPAL_ROOT . '/sites/all/modules/domain/settings.inc';

Be sure to put $conf['cache_backends'][] = 'sites/all/modules/contrib/mobile_switch/mobile_switch_database_cache.inc'; and $conf['cache_class_cache_page'] = 'MobileSwitchDatabaseCache'; ABOVE the include DRUPAL_ROOT...

Otherwise, Drupal won't be able to find the "MobileSwitchDatabaseCache" class.

Mr.Echo’s picture

Issue summary: View changes

spelling errors

AlfTheCat’s picture

Issue summary: View changes

Same as #22. It's not working for my homepage, which is a panels page.
I'm not getting any errors at all. Just not working :(

ntigh52’s picture

I all,
I have homepage build and declared by panels module.
when enabling cache pages for anon users, -
Just on this front page and for anon users I have the responsive theme also on desktop.
Do you have any idea for how to solve it?!
Thanks a lot

ntigh52’s picture

Status: Needs review » Active
MakeOnlineShop’s picture

Hello,

Can you confirm that the solution is reliable and stable ?

Thank you.

Rhicreate’s picture

Brilliant, the patch in #14 has saved the day for me - My site's been hit by a lot of bots recently and I really really REALLY needed to have page caching turned on for anon users. Seems to work perfectly at enabling the switching and not confusing the cache!

Anyone who tries this, please remember to check that the path given to your new cache file in settings.php is correct, as my mobile_switch module does not sit in a /contrib directory.

AlfTheCat’s picture

Yes, I've been using this solution for many months.

bsztreha’s picture

#7 Works for me, maybe #14 is good too

mikeytown2’s picture

A better option is to modify `$_SERVER['REQUEST_URI']`; no need to implement you own cache backend.

At the bottom of settings.php add this

drupal_bootstrap(DRUPAL_BOOTSTRAP_VARIABLES);
include_once DRUPAL_ROOT . '/sites/all/modules/mobile_switch/mobile_switch.module'
$mobile_switch = '';
if (function_exists('_mobile_switch_block_get_cookie') && _mobile_switch_block_get_cookie()) {
  $mobile_switch .= ':' . _mobile_switch_block_get_cookie();
}
if (function_exists('mobile_switch_mobile_detect')) {
  $browser = mobile_switch_mobile_detect();
  if (is_array($browser)) {
    if ($browser['istablet']) {
      $mobile_switch .= ':tablet';
    }
    elseif ($browser['ismobiledevice']) {
      $mobile_switch .= ':mobile';
    }
    else {
      $mobile_switch .= ':standard';
    }
  }
}

$_SERVER['REQUEST_URI'] .= '#' . $mobile_switch;

Where is _mobile_switch_block_get_cookie() coming from?

If all the variables are hardcoded in the settings.php file via $conf then the drupal_bootstrap line could be ignored.

alex.87’s picture

#14 working fine, but i have needs for more caching modules, i tried Boost but i have similar issue, actually im getting wrong theme on both desktop and mobile...

foreverfun’s picture

I applied #14.

My result is

User 1 enters using desktop’s browser
User 2 enters using mobile’s browser

When user 1 enters: example.com/:standard is cached in cid.
When user 2 enters: example.com/:mobile is cached in cid.
At this point, pages are shown using the correct themes.
For example, desktop user sees desktop theme, and mobile user sees mobile theme.

The problems come
1)
When user 1 clicks on pageA, example.com/pagea/:standard is cached.
Now user 2 clicks on pageA, the desktop theme, example.com/pageA/:standard, shows on the mobile device.
This is incorrect theme for user 2.

2)
When user clicks pageB, example.com/pageb/:mobile is cached.
Now user 1 clicks on pageB, example.com/pageb/:mobile, shows on the desktop.
This is again incorrect.

Why?

Please help! Your suggestions are invaluable to me. Please help!

rozh’s picture

Status: Active » Reviewed & tested by the community

#14 works perfectly.

I guess we can mark this as RTBC.