There seems to be an issue when using Drupal's page caching.

When enabled the $is_mobile and $is_tablet vars only work intermittently on mobiles/tablets and even intermittently gives a false positive resulting in mobile style sheets etc being loaded for desktop browsers.

Would be great to be able to use this module with caching enabled. Let me know if you need any more info.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mpdonadio’s picture

Assigned: Unassigned » mpdonadio

Yeah, the standard way Drupal caching works doesn't take things like this into consideration. I'll see what I can do.

RaphaelBriskie’s picture

Awesome!

progzy’s picture

IMHO the module is not the problem. But maybe you could post a snippet here to show how you are "doing things".

Having said that, I guess you're "doing things" in a hook function that is cached. AFAIK the only available hook when caching is enabled is the "hook_boot" one. The main problem here is that very few things are loaded...

Then from here it depends of what you want to do. I think the "dynamic cache" module can help in some cases.

mpdonadio’s picture

I originally wasn't planning on tacking anything related to the caching issue with this module. But, since I added the sub-module which adds variables to the preprocess hooks, this could be more of an issue for people.

If I do this (leaning towards yes), I will add something similar to what authcache does. Basically, I would create a new cache backend that would take into account the mobile/tablet values and use these for the page cache key for anonymous users.

RaphaelBriskie’s picture

@progzy I'm using it as per the instructions in the module:

<?php
$detect = mobile_detect_get_object();
$is_mobile = $detect->isMobile();
$is_tablet = $detect->isTablet();
?>

So in page.tpl.php I have the above at the top of the file and then I'm simply using something like:

<?php if ($is_mobile) { // do something } ?>

And it works as long as Drupal cache is disabled, as soon as I enable Drupal caching the result is pretty much random.

Jason Dean’s picture

@mpdonadio thanks for this great module :)

Picking up your thoughts in #4, I wondered whether you've been able to make any progress addressing the cache problem... or are likely to have time for that?

I'm a fan of this module's lightweight approach and Ctools support. If there are any opportunities for co-sponsorship or support to help resolve this issue, I'd be happy to discuss!

mpdonadio’s picture

OK, here is the status of this request.

-- I haven't decided if this should hold up a proper alpha release, or if I should do an alpha release, and then work on this in the dev branch. I am leaning toward the second.

-- I haven't decided which of the MD predicates this should really take into account. Mobile/tablet is easy (well, in theory), but some people use the other functions. These functions can vary based on the MD version you are using. Also, taking too many predicates into account would mean way less cache efficiency

-- I have an option that would address the address these, but would probably be best split out into a separate module. This would also have the same cache efficiency problem, though.

-- I haven't had time to really dedicate to really think all of this through.

mpdonadio’s picture

Category: bug » feature
Priority: Normal » Major

Reclassifying this as a feature request.

I am tentatively starting work on this in July, and this will be a sub-module.

progzy’s picture

@RaphaelBriskie

@progzy I'm using it as per the instructions in the module:

$detect = mobile_detect_get_object();
$is_mobile = $detect->isMobile();
$is_tablet = $detect->isTablet();

So in page.tpl.php I have the above at the top of the file and then I'm simply using something like:

if ($is_mobile) { // do something }

And it works as long as Drupal cache is disabled, as soon as I enable Drupal caching the result is pretty much random.

Actually, the question is more what you want to do than how you do things. What "//do something" is supposed to do? What do you want to achieve, context? I am just saying that there may be a workaround/alternative for your use case.

Hope it is clearer now :)

EDIT: it has nothing to do with the issue but is it a normal behaviour that clicking reply just bellow a comment does not place the incoming comment just below it (nested)

JulienF’s picture

Hello Folks,

I'm wondering what is going on in here as I'm encountering this problem when the cache is enabled.

My situation is as follows:

I'm conditionally adding javascript and css files (Drupal add js / add css) according to the device detection. All this logic was happening in a hook_init and I moved it to the template.php of the theme but it didn't help.

The behaviour is not random though, what I could observe is that the first device to visit the website will be processed correctly, afterward the result is cached and the code is never hit again so whatever the next device to visit the site be it will see the very first version that have been generated.

Any help here would be greatly appreciated as I would like to finally deploy a website that needs only this issue to be fixed in order to get a GO.

Thanks ahead.

PS: I'm ready to help on this, simply provide me with guidelines to not blindly hit the issue.

mpdonadio’s picture

The problem has to do with the way page caching works in Drupal.

Cached pages get served up between hook_boot() and hook_init() (search core for drupal_serve_page_from_cache() and start tracing backwards through the bootstrap process).

Pages are stored in the cache by drupal_page_set_cache() , and the $cid is not changable.

The hurdle is that neither of these two actions are alterable/hookable.

My first attempt at getting around this was to implement an alternate page cache, similar to how the Authcache module works. The second attempt (I stalled out on this) was to mess with $_SERVER so that request_uri() would get back mobile/tablet info so that drupal_page_set_cache would have a better $cid to work with.

The second approach seems like the only thing that is feasible, but I don't know how safe it really is.

progzy’s picture

A possible quick fix for users that should work (can not test from where I am):
- Separate URLs for each targeted devices like www.ex.com for desktop, m.ex.com for mobile ... This makes a different $cid I think
- Make detection in hook_boot and redirect to the URL matching the device

A cookie could be used to avoid detection on each requests.

Feedbacks are welcome.

.

dgtlmoon’s picture

Category: feature » bug

Isnt this a bug?

If mobile detect is used to detect mobile's, but has an issue where it incorrectly detects mobile's due to an inability to function with Drupal caching, then I think this should be classed as a bug?

ransomweaver’s picture

There is a cookie-based database caching module: https://drupal.org/project/cookie_aware_page_cache

The author has written it to work with context_breakpoints, like so:

$conf['cache_backends'][] = 'sites/all/modules/cookie_aware_page_cache/CookieAwarePageCache.inc';
$conf['cache_class_cache_page'] = 'CookieAwarePageCache';
$conf['cookie_aware_page_cache_cookies'][] = 'context_breakpoints';

which is how I'm using it, but it could probably be made to work with anything that sets cookies; it introduces the cookie into the cache bin cid.

The problem I had with it is that the normal cache clearing mechanism doesn't work; i.e. performance page "clear cache", cache_actions clear cache bin, and admin menu flush caches doesn't do it, though "drush cc all" will. I didn't dig into why this is, as I figure why make it more complicated than a rule triggered by new content that executes db_query("TRUNCATE {cache_page}"); Of course that might be a problem with a high traffic site. Cache actions allows for clearing specific CIDs, so if you examine how cookie aware page cache constructs CIDs you could probably target very specific cache table entries with cache_actions.

I recommend context_breakpoints for doing things like not rendering blocks for certain screen sizes within the same theme, as you would want with response theming.

Hi Matt!

rooby’s picture

Issue summary: View changes

You might be interested in #1768556: Context Mobile Detect does not work with page cache enabled, where this problem was tackled for the Context Mobile Detect module.

deepfriedbits’s picture

Any movement on this? I might be interested in sponsoring this module if we can get the cache conflict figured out.

mpdonadio’s picture

Right now, this stalled out, both due to me becoming distracted by other things, and lack of a clear solution that works well.

The proper thing to do would be to patch core to make the default page cache $cid alterable. The chances of this happening in 7 are pretty much nil. See #1303010: Page cache only uses URL as cache ID, not HTTP Accept headers or language and #1855260: Page caching broken by accept header-based routing, and some of the linked issues.

I took a stab based on the lead from @ransomweaver but that didn't work.

To be honest, the motivation for this module was to support Panels based sites, and leverage those caching solutions. I never really wanted to tackle this as a general issue, and that is why this was originally a feature request and not a bug...

I read through the patch from the link @rooby provided, and wasn't convinced that it was a good solution in this case. However, I will take another look. I am also worried about the chain reaction of possible Boost and Varnish bugs that will ensue.

It also raises a large issue. What criteria do we want to use? Just $is_mobile and $is_table? Or do we want to delve into all of the tests (which vary library version to version)?

Ideas and patches welcome.

mpdonadio’s picture

Status: Active » Needs review
mpdonadio’s picture

OK, for those playing along at home, here is our starting point. Patchfile only, as I don't know if this is dangerous.

The patch adds a new sub-module, mobile_detect_caching. The module itself currently does nothing. There is an include file. Add it like you would a new cache backend:

$conf['cache_backends'][] = 'sites/all/modules/mobile_detect/mobile_detect_caching/mobile_detect_caching.inc';

It will only work with Apache(?). I doubt it will work with Varnish, Boost, or similar. I suspect it will work with {cache_page} in alternate caches, but I didn't test this.

It assumes the library is in sites/all/libraries/Mobile_Detect/Mobile_Detect.php, and can't use the autoloader which was one of the points of the module :(

Read the patch to see what it does. Yes, I know the hook_boot() and hook_init() are empty. They are there for debug purposes so I don't need to clear caches when I need them.

I am seeing X-Drupal-Cache:HIT in response headers. I added the request time to the debug blocks to aid with seeing whether the page changes.

Feedback appreciated. If enough people say this doesn't suck, I will push to 7.x-1.x-dev, and then we can figure out a long term solution.

Flimmer1970’s picture

It looks like I successfully applied the patch.
I have a new module Mobile Detect Caching, which I activated without problems.
Everything is working well like bfore the paatatch was applied.

I just don´t have a subfolder mobile_detect_caching in the modules folder so I included it so:
$conf['cache_backends'][] = 'sites/all/modules/mobile_detect/mobile_detect_caching.inc';

Should there be something else in the subfolder?

We soon will start caching tests and I wil give a response.
Thanks for your great work!

Maurice M.’s picture

Is there any update on this issue / patch ?

mpdonadio’s picture

@Maurice-Burst, can you try out the patch and let me know if it works for you?

Maurice M.’s picture

We fixed the issue by using the Context Mobile Detect module.

cutesquirrel’s picture

Another way :

using the memcache module, you can specify a prefix_key (memcache_storage_key_prefix), so you can distinguish the caches between the different devices :

if ($mobileDetect->isTablet()) {
  $conf['memcache_storage_key_prefix'] = 'mycache_tablet';
}
else if ($mobileDetect->isMobile()) {
  $conf['memcache_storage_key_prefix'] = 'mycache_mobile';
}
else{
  $conf['memcache_storage_key_prefix'] = 'mycache_global';
}
dianacastillo’s picture

Will installing the cache exclude module and not caching the particular pages where the mobile detection is necessary fix this problem?

mpdonadio’s picture

I haven't tried the two together, but yes, CacheExclude should prevent cache related problems if they are due to page level caching, and not something else (like block caching).

quotesBro’s picture

@cutesquirrel, good idea!

mpdonadio’s picture

Has anyone tried this patch yet? I am hesitant to push it w/o feedback from users.

darol100’s picture

@mponadio,

I enable the core caching (Caching for anonymous users.) on a Pantheon dev server and your patch does not seem to be working. I'm having still have problems with anonymous user.

mpdonadio’s picture

@darol100, I assume you also enabled the new submodule and added

$conf['cache_backends'][] = 'sites/all/modules/mobile_detect/mobile_detect_caching/mobile_detect_caching.inc';

to your settings?

darol100’s picture

@mpdonadio,

I'm sorry I forgot to add $conf['cache_backends'][] = 'sites/all/modules/mobile_detect/mobile_detect_caching/mobile_detect_caching.inc'; to the settings.php.

Once I did that this works like a charm.

I think this patch is ready to be submitted.

darol100’s picture

Status: Needs review » Reviewed & tested by the community
mpdonadio’s picture

Awesome; thanks. I will merge this into -dev and tag an alpha release later today (or tomorrow).

SocialNicheGuru’s picture

So should I use either #24 method or add the cache_backend?
I am using Redis as the default cache backend.

If I set the mobile_detect cache bin, should I use that bin for page storage only if $mobile_detect returns mobile or tablet?

mpdonadio’s picture

I am not sure if the method in #24 is a good idea. When you clear caches, it will only clear the one you have active, and not the others.

This patch isn't a true cache backend. It just uses the mechanism to get some code executed during bootstrap before a page gets served up from cache. It alters the request string to add on new variables to. This is to get around the fact that the page $cid isn't alterable.

This may not be the final version of the patch, as I just thought of an additional complication that I need to mull over.

izmeez’s picture

Is it really necessary to require a line in the settings.php file when enabling the sub-module? Or can it be available in the drupal configuration UI?

mpdonadio’s picture

@izmeez, AFAIK, cache backends are always configured through settings.php. Do know of any that can handle {cache_page} that don't?

markusgerlach’s picture

---

mpdonadio’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
6.3 KB

One more version. Can someone double check this? Seems to work for me. Thanks.

darol100’s picture

Status: Needs review » Reviewed & tested by the community

@mpdonadio

I tested out with the Drupal Core cache and works like charm. I think this is ready to be submitted.

  • mpdonadio committed 3b6f625 on 7.x-1.x
    Issue #1889824 by mpdonadio: First pass at workaround for using the...
  • mpdonadio committed 5d7ccc2 on 7.x-1.x
    Issue #1889824 by mpdonadio: Fixed caching logic to prevent query...
mpdonadio’s picture

Assigned: mpdonadio » Unassigned
Status: Reviewed & tested by the community » Fixed

It's in. Please continue to report bugs here instead of opening new issues.

SocialNicheGuru’s picture

I am still a little unsure about how to properly implement.

I want to enable prefix cache tables for mobile and tablet and desktop devices like in #24.
But do I have to:
1. include the library in my settings.php (I think I do)
2. recreate a new Mobile_Detect object on each page load. Is that necessary? If it is created once, can it be reused?

in my settings.php file:

  $mobileDetect = NULL;
  include_once 'sites/all/libraries/Mobile_Detect/Mobile_Detect.php';
 
  if (class_exists('Mobile_Detect')) {
      $mobileDetect = new Mobile_Detect();
      }

  if ($mobileDetect->isTablet()) {
        $conf['cache_prefix'] = $conf['cache_prefix'].'-tablet';
     
    }
     else if ($mobileDetect->isMobile()) {
        $conf['cache_prefix'] = $conf['cache_prefix'].'-mobile';
        
     }
     

 # Mobile_detect
 $conf['cache_backends'][] = 'sites/all/modules/mobile_detect/mobile_detect_caching/mobile_detect_caching.inc';

When I add the cache_backend I don't have to do anything else? For example with redis, apc, or memcache I then set an actual cache bin like the cache_pages to use it.

Do I have to do the same with the mobile_detect cache_backend?

mpdonadio’s picture

I don't recommend the approach in #24. This will create three caches for everything.

When you add the cache backend line, you don't need to do anything else. It is not a true backend. It just uses (well, kinda abuses) the cache backend to alter the $cid for {cache_page} by altering $_SERVER['REQUEST_URI'] to add on query variables based on what the Mobile_Detect object returns.

SocialNicheGuru’s picture

So I can remove prefixing.
I still add the mobile_detect cache backend
But do I then have to assign a bin like cache_page or cache_views, etc to the mobile_detect cache backend?

I think from above only cache_page will be modified.

mpdonadio’s picture

No, there is no configuration like with memcache or redis. This patch is only to force the $cid for {cache_page} to take mobile and tablet into account, which makes it work with page caching for anonymous users.

SocialNicheGuru’s picture

I am connecting to external services and it looks like the url is appended which causes some of these external services to fail validation. Here is an example of Linkedin.

Mar  7 21:25:48 dev drupal: 
http://testlocal/linkedin/token/1?mobile_detect_caching_notmobile&mobile_detect_caching_nottablet|
Warning: Illegal offset type in drupal_set_message() (line 1815 of d7/includes/bootstrap.inc).

Mar  7 21:25:48 dev drupal: 
http://testlocal/linkedin/token/1?mobile_detect_caching_notmobile&mobile_detect_caching_nottablet|
Warning: Illegal offset type in drupal_set_message() (line 1819 of d7/includes/bootstrap.inc).

Mar  7 21:25:48 dev drupal: 
http://testlocal/user/1/social_media/linkedin|1||Linkedin reported the following response : timestamp_refused

In #42 we were asked to continue to report bugs here.

If you would like me to create another issue, I will.

SocialNicheGuru’s picture

Status: Fixed » Needs work
markusgerlach’s picture

I see a blank page after i added this row
$conf['cache_backends'][] = 'sites/all/modules/mobile_detect/mobile_detect_caching/mobile_detect_caching.inc';
to the end in sites\default\settings.php.

On a other homepage i got a redirection error after a added this code.

I'm also can't see a new submodule caching in the Backend /admin/modules

What's wrong here?

mpdonadio’s picture

@markusgerlach, you are probably getting a WSOD: https://www.drupal.org/node/158043 I can't really debug w/o an error message.

Not sure why you are getting redirects and can't see the module in the module list.

SocialNicheGuru’s picture

drush cron --debug

I get the following notice:

Undefined index: QUERY_STRING mobile_detect_caching.inc:19 [0.47 sec, 20.07 MB]

markusgerlach’s picture

Ok after i reinstall this modul i could see the sub Module "Mobile Detect Caching".

I still got the redirection error after i added this code ($conf['cache_backends'][] =... ) :

from nginx/error.log
2015/03/16 12:07:17 [error] 2550#0: *266954 FastCGI sent in stderr:
"PHP message: PHP Fatal error: Cannot redeclare class Mobile_Detect in /var/www/www.xxx.de/web/sites/all/modules/context_mobile_detect/library/mobile_de... on line 16"
while reading response header from upstream, client: zzz, server: xxx, request: "GET /inhalt/yyy HTTP/1.1",
upstream: "fastcgi://127.0.0.1:8090", host: "xxx"

I could solve this problem. I added the following line to settings.php:
include DRUPAL_ROOT . '/sites/all/modules/context_mobile_detect/settings.inc';

For more see(sites\all\modules\context_mobile_detect\README.txt)

Now i get a blank site on mobile device.
I'll check the error messages...

Now i delete the line ($conf['cache_backends'][] =... ) in settings.php and it works else i got a blank site with error_reporting.

thx

mpdonadio’s picture

OK, a few things going on. One, it looks like FastCGI is behaving in a different manner that my test setup, and it also looks like nginx is also behaving in a different manner.

Just being able to alter the page cache cid would make life so much easier, but it looks like that will never happen with Drupal 7...

darol100’s picture

@mpdonadio,

Do you know if module would ever work with Boost ? Someone at Drupal Answer (Themekey with mobile detect not working properly) was having some issues with the Mobile Detect module and I believe it was because of the boost module.

Jieyyal’s picture

The latest module still have issue with page cache.

Finally I found the Mobile_Detect.php library path is hard coded at mobile_detect_caching.inc.

My library path is DRUPAL_ROOT . '/sites/default/libraries/Mobile_Detect/Mobile_Detect.php'.

When changed to mine, it works.

/**
 * Alters $_SERVER['REQUEST_URI'] to add in the mobile detect results, so that
 * the Drupal page caching system will use them as part of the $cid.
 */
function mobile_detect_caching_alter_cid() {
  @include_once DRUPAL_ROOT . '/sites/all/libraries/Mobile_Detect/Mobile_Detect.php';

  if (class_exists('Mobile_Detect')) {
    try {
      $detect = new Mobile_Detect();

      parse_str($_SERVER['QUERY_STRING'], $query);
mpdonadio’s picture

#55, the general problem is that is a somewhat non-standard location for libraries (I could possible check there, too, though), and the Libraries module isn't available at this point in the bootstrap.

mrP’s picture

I was getting infinite redirect loop errors similar to #49

For me, the cause was globalredirect module's 'Non-clean to Clean' option which is enabled by default

If enabled, this option will redirect from non-clean to clean URL (if Clean URL's are enabled). This will stop, for example, node 1 existing on both example.com/node/1 AND example.com?q=node/1

Not sure if there is anything that can be done in this module to prevent this conflict or not. Or if this reveals other areas where we may see problems.

VVVi’s picture

Status: Needs work » Needs review
FileSize
5.55 KB

Here is the patch for more "native" way to change the $cid depending from device type. Please, don't forget add the strings to settings.php:

  $conf['cache_backends'][] = 'sites/all/modules/mobile_detect/mobile_detect_caching/mobile_detect_caching.inc';
  $conf['cache_class_cache_page'] = 'MobileDetectCache';
  $conf['mobile_detect_library'] = 'sites/all/libraries/Mobile_Detect/Mobile_Detect.php';

And you may need to correct the paths if you installed the module and the library in the different location.

mpdonadio’s picture

I think #58 may be a good approach, but I need to look at it closer tonight. Would like to get others to test it, too. Personally, I think I would leave in the experimental language until we know it works well for a large number of users.

  • mpdonadio committed 2a14809 on 7.x-1.x
    Issue #1889824 by mpdonadio: Restored experimental warnings.
    
  • mpdonadio committed a5e4470 on 7.x-1.x authored by VVVi
    Issue #1889824 by VVVi: Rework mobile_detect_caching as a...
  • mpdonadio committed f3e4aaf on 7.x-1.x
    Issue #1889824 by mpdonadio: Class cleanup per Drupal coding standards;...
mpdonadio’s picture

Just committed #58 w/ a little cleanup.

Please test. I think we could have a winner here.

VVVi’s picture

It works fine for me. Thanks for the fast commit and for the great module.

izmeez’s picture

Have the required strings for settings.php been added to the README.txt ?

mpdonadio’s picture

#63, yes, that was part of a5e4470e and I just visually verified a fresh clone from git and the download on the project page (which I also updated last night).

mpdonadio’s picture

Status: Needs review » Closed (fixed)

No comments in three weeks, so I think we can call this closed. We ca re-open or file followups if further bugs surface.

Sora_tm’s picture

Hello, kamrads!

When i tested my site with cache enabled, I found this issue - when i used iphone, in cache_page table rows look like:

mobile:http://site/
mobile:http://site/news

Everything OK. But when I using Ipad, I found that new cache bins not created. I edit mobile_detect_caching_alter_cid() in mobile_detect_caching.inc like this:

  protected function mobile_detect_caching_alter_cid() {
    $device = '';

    if (class_exists('Mobile_Detect')) {
      try {
        $detect = new Mobile_Detect();

        if (($detect->isMobile()) && ($detect->isTablet() == FALSE)) {
          $device = 'mobile:';
        }
        elseif ($detect->isTablet()) {
          $device = 'tablet:';
        }

      } catch (Exception $e) {
        // nop
      }
    }
    return $device;
  }

And now cache_page table has this rows:

tablet:http://site/
tablet:http://site/news
mobile:http://site/
mobile:http://site/test

Now it work like I expected.

Sorry for my English :)

derMatze’s picture

Sora_tm is absolutely right. Thank you :)

mpdonadio’s picture

Status: Closed (fixed) » Needs review

Just pushed out the fix in #66.

Vincent_Jo’s picture

Hi,

Just pushed out the fix in #66.

What does it mean?
Is there an updated module version already available?
Or is the patch from #58 fixing this issue?

mpdonadio’s picture

#69, the -dev version should include the latest version with the patch in #66.

Vincent_Jo’s picture

#70, Thanks for pointing this out.

Unfortunately with no success here.
Installed latest dev.
Installed latest library.
Put the code in into settings.php. ($conf['cache_backends'].....)
Leading to strange behaviour on the whole site. Nodes not saving after edit. WSOD after flushing caches ... .
And mainly, the mobile-detect function didn´t start working.

Very likely: Is it another issue when having boost-caching activated?

regards
Vincent

mpdonadio’s picture

#71, I am not sure if anyone has tested this along with Boost, but I suspect that could be the problem.

jim.shreds’s picture

All is well when enabling modules.
put:
$conf['cache_backends'][] = 'sites/all/modules/mobile_detect/mobile_detect_caching/mobile_detect_caching.inc';
in settings.php and get WSOD. comment it out and it all works again

logs say:
[Fri Jan 29 15:41:56.062769 2016] [:error] [pid 2959] [client 127.0.0.1:49777] PHP Fatal error: require_once(): Failed opening required '/sites/xxxx/sites/all/modules/mobile_detect/mobile_detect_caching/mobile_detect_caching.inc' (include_path='.:') in /sites/xxxx/includes/bootstrap.inc on line 2400

derMatze’s picture

Looks like you did not install the module to /sites/all/modules/mobile_detect/mobile_detect_caching/ or at least the file "/sites/all/modules/mobile_detect/mobile_detect_caching/mobile_detect_caching.inc" does not exist...

jim.shreds’s picture

yup. its in a contrib folder.
d-uh
thanks.sorry

cxc891’s picture

just wanted to confirm it does create two caches for every page? thanks!

mpdonadio’s picture

#76, it will potentially create a cache entry for three versions: desktop, tablet, and mobile.

Have enough people tested this to consider the issue done?

Ben Greenberg’s picture

I am using the memcache module (version 7.x-1.5) on my site for the default cache. If I add the lines for mobile detect caching to my settings.php, the pages are cached to the database instead of memcache. Am I doing something wrong, or are the two mutually exclusive? Here are the lines configuring the cache in my settings.php file:

$conf['cache_backends'][] = 'sites/all/modules/memcache/memcache.inc';
$conf['cache_default_class'] = 'MemCacheDrupal';
$conf['cache_class_cache_form'] = 'DrupalDatabaseCache';
$conf['memcache_stampede_protection'] = TRUE;
$conf['lock_inc'] = 'sites/all/modules/memcache/memcache-lock.inc';
$conf['memcache_key_prefix'] = 'development';

$conf['cache_backends'][] = 'sites/all/modules/mobile_detect/mobile_detect_caching/mobile_detect_caching.inc';
$conf['cache_class_cache_page'] = 'MobileDetectCache';
$conf['mobile_detect_library'] = 'sites/all/libraries/Mobile_Detect/Mobile_Detect.php';

edit:
I should have looked at the code before posting. Anyway, to answer my own question, "MobileDetectCache" extends "DrupalDatabaseCache", so it doesn't look like it would work with other cache backends.

I copied the code to a mobile_detect_memcache.inc file. I created a "MobileDetectMemCache" class that extends Memcache's "MemCacheDrupal" class (after a "require_once" for memcache.inc). I think I have setting/getting working. The only issue was needing to add the get($cid) function. I still need to check if flushing works, but that will have to wait until tomorrow.

I have no idea how kosher it is to extend a non-core module's class like this. I'd definitely like to know if there is a more Drupaly way to do this.

Ben Greenberg’s picture

Here is a patch with my implementations for memcache and memcache_storage support. I put instructions in patched README.TXT

I also changed the mobile_detect_caching_alter_cid() function so that instead of:

if (($detect->isMobile()) && ($detect->isTablet() == FALSE)) {
  $device = 'mobile:';
}
elseif ($detect->isTablet()) {
  $device = 'tablet:';
}

it is now

if ($detect->isTablet()) {
  $device = 'tablet:';
}
elseif ($detect->isMobile()) {
  $device = 'mobile:';
}

It's cleaner and reads better to me.

mpdonadio’s picture

Status: Needs review » Needs work

Thanks for both reporting the problem and for the patch! Some comments before I look at this before I apply it for closer examination.

  1. +++ b/README.txt
    @@ -93,6 +93,54 @@ the module and library in a different location.
    +    ¶
    +    # MEMCACHE API AND ITEGRATION MODULE
    +    $conf['cache_backends'][] = 'sites/all/modules/memcache/memcache.inc';
    +    $conf['cache_default_class'] = 'MemCacheDrupal';
    +    $conf['cache_class_cache_form'] = 'DrupalDatabaseCache';
    +    ¶
    +    # MOBILE DETECT
    

    Looks like some stray tabs/spaces.

  2. +++ b/mobile_detect_caching/mobile_detect_caching.inc
    @@ -60,12 +60,12 @@ class MobileDetectCache extends DrupalDatabaseCache implements DrupalCacheInterf
    -        if (($detect->isMobile()) && ($detect->isTablet() == FALSE)) {
    -          $device = 'mobile:';
    -        }
    -        elseif ($detect->isTablet()) {
    +        if ($detect->isTablet()) {
               $device = 'tablet:';
             }
    +        elseif ($detect->isMobile()) {
    +          $device = 'mobile:';
    +        }
     
    

    This is an out of scope change regarding the problem you identified (especially since we don't have tests to make sure this isn't a regression). We really try to keep commits isolated to fixing a particular problem. Cleanups can get done in their own issues.

  3. +++ b/mobile_detect_caching/mobile_detect_memcache.inc
    @@ -0,0 +1,83 @@
    +require_once DRUPAL_ROOT . '/sites/all/modules/memcache/memcache.inc';
    +class MobileDetectMemCache extends MemCacheDrupal {
    

    Memcache may not be installed in this file location. I think it is safe to assume that the end user has already done this in settings.php. We can add that to the README.

  4. +++ b/mobile_detect_caching/mobile_detect_memcache.inc
    @@ -0,0 +1,83 @@
    +  ¶
    +  public function get($cid) {
    +    $cid = $this->mobile_detect_caching_alter_cid() . $cid;
    +    return parent::get($cid);
    +  }
    +  ¶
    +  /**
    

    Stray spaces.

  5. +++ b/mobile_detect_caching/mobile_detect_memcache_storage.page_cache.inc
    @@ -0,0 +1,87 @@
    +require_once DRUPAL_ROOT . '/sites/all/modules/memcache_storage/memcache_storage.api.inc';
    +require_once DRUPAL_ROOT . '/sites/all/modules/memcache_storage/memcache_storage.page_cache.inc';
    

    Same as above. Memcache may not be installed here.

  6. +++ b/mobile_detect_caching/mobile_detect_memcache_storage.page_cache.inc
    @@ -0,0 +1,87 @@
    +  ¶
    +  public function get($cid) {
    +//    dpm('md get cid:' . $cid);
    +    $cid = $this->mobile_detect_caching_alter_cid() . $cid;
    +    return parent::get($cid);
    +  }
    +  ¶
    +  /**
    

    Stray spaces.

  7. +++ b/mobile_detect_caching/mobile_detect_memcache_storage.page_cache.inc
    @@ -0,0 +1,87 @@
    +//        dpm('md getMultiple array loop cid:' . $cid);
    +        $cids[$key] = $this->mobile_detect_caching_alter_cid() . $cid;
    

    We try not to commit debug and commented out code.

  8. +++ b/mobile_detect_caching/mobile_detect_memcache_storage.page_cache.inc
    @@ -0,0 +1,87 @@
    +        if ($detect->isTablet()) {
    +          $device = 'tablet:';
    +        }
    +        elseif ($detect->isMobile()) {
    +          $device = 'mobile:';
    +        }
    +
    

    As with the above, we want to try to be consistent with how we implement this.

  9. +++ b/mobile_detect_caching/mobile_detect_memcachie_storage.info
    @@ -0,0 +1,14 @@
    +; Information added by Drupal.org packaging script on 2015-11-01
    +version = "7.x-1.0-alpha1"
    +core = "7.x"
    +project = "mobile_detect"
    +datestamp = "1446408241"
    +
    

    Don't need this bit; packager will add it.

In general, I think this is great, but I wonder how this will scale for Redis, MongoDB, APCu, and other cache backends. Not sure what the answer is here; we may be running into a limitation of how caching is implemented in D7.

Ben Greenberg’s picture

Some comments before I look at this before I apply it for closer examination.

Thanks for taking the time to spell everything out for me. Here is a clean version of the patch. Let me know if I missed anything, or if you want other changes.

Memcache may not be installed in this file location. I think it is safe to assume that the end user has already done this in settings.php.

I kept getting Class not found exceptions, and they went away when I added those include statements. When I was debugging another problem, I figured out that I needed to add the mobile detect cache settings after the memcache settings. But, I didn't make the connection that adding memcache to the cache_backends array before mobile detect, caused the memcache classes to be loaded first, and available for my mobile detect class to extend it. Thanks for clearing that up for me, I knew I was missing something obvious about how the Drupal cache system worked.

patbranch’s picture

I'm having this issue as well. I enabled the included cache module which seemed to solve it. Today, I noticed the site thinking my laptop is a mobile device. Clearing the caches fixed it for now.

What's the best fix at this at this point? I'm using 7.x-1.0-alpha1. Did the patch in #79 do it?

vdanielpop’s picture

#82 Try using the dev version, several fixes have been applied to that branch and, from what I read, it should have the fix for your issue as well.

suman.abc’s picture

If anybody is using varnish, there is another solution using varnish vcl script which worked properly for my project.
In vcl_recv, you put a condition to detect mobile device using user agent - req.http.user-agent. Inside the if block add an extra parameter for mobile version e.g. set req.url = req.url + "&mobile=true";. This extra parameter is only visible to drupal backend.
So when varnish calculates the hash using vcl_hash, it includes URL by default. Two separate URLs for desktop and mobile version make it happen to generate two cache entries for two versions whereas browser won't show these two separate URLs.

tcnolan7’s picture

Hi all, I am having this issue with Mobile Detect and Drupal 7 caching. I have the dev version of the module installed and have the latest version of Mobile_Detect.php. I did enable Mobile_Detect_Caching and entered the 3 lines in the settings.php.

But still if I go to the site on a mobile device first and then visit on a desktop, I see the mobile version. Or vise versa.

Is there anything else I can try to make drupal caching work with Mobile Detect?

Thank you.

Edit: I figured this out. I had not made the changes to the settings.php file. Once those lines were added, it now works with page caching turned on.

abhishek.kumar’s picture

@sumanchalki can you please provide code snippet for varnish vcl file ?

abhishek.kumar’s picture

here is the solution for varnish which I did and it's working fine.

Create a file called "detect-device.vcl" and copy the following vcl logic.
Ensure this file is included in your own vcl using, for example;

include "/etc/varnish/detect-device.vcl";

This is all you need to do as each function is then appended to the functions
in your own vcl file.

################################################################################

sub vcl_recv {
  call identify_device;
}

// Set a device grouping based on the accessing user-agent.
sub identify_device {
  // Set a default of "NA" which will be used if the user-agent isn't one of the
  // mobile groups.
  set req.http.X-Device = "NA";

  // Check user-agents
  if (req.http.User-Agent ~ "iPad") {
    set req.http.X-Device = "mobile-tablet";
  }
  elsif (req.http.User-Agent ~ "iP(hone|od)" || req.http.User-Agent ~ "Android" ) {
    set req.http.X-Device = "mobile-smart";
  }
  elsif (req.http.User-Agent ~ "SymbianOS" || req.http.User-Agent ~ "^(BlackBerry|LG|Nokia|SonyEricsson|SAMSUNG)") {
    set req.http.X-Device = "mobile-other";
  }
}

sub vcl_hash {
  if (req.http.X-Device ~ "^mobile") {
    // Varnish 2.x => 3.x compatibility.
    // Swap the lines below if you're using Varnish 2.x.
    // set req.hash += req.http.X-Device;
    hash_data(req.http.X-Device);
  }
}

SocialNicheGuru’s picture

@abhishek.kumar in #87 are you saying that the module is not needed at all?
and that a cache entry would be created for each type of device: NA ie. desktop, mobile, or tablet?
Would I even need to specify the cache bin in settings.php?
And then how would I test it?

When a cache flush or purge is done would it clear all of three caches for a page?

leymannx’s picture

Following the issue out of interest while having a similar problem.

To me it seems a reasonable decision to switch to JS to solve that issue.

Exposing an uncached route like /mobile-detect that returns a JSON response depending on the mobile detection. And check that route via JS and add the outcome to DrupalSettings, no?

euk’s picture

Hello!

Patch in #81 seems not to be able to patch README.txt on top of the dev branch.
Patches another file instead.

nonom’s picture

Assigned: Unassigned » nonom
nonom’s picture

Status: Needs work » Closed (outdated)

I'm my own experiments the mobile_detect along 7, 8, 9 the results could vary depending what cache type are you using.

Reading this thread I found some interesting examples but surely it doesn't fit your current needs, so I would like to know the state of this issue and get some feedback to improve this.

I'm focused in Drupal 8 / 9 caching contexts but would nice to improve mobile_detect 7.x. if you need it.

Feel free to reopen the issue if you have any related question.

leymannx’s picture

Assigned: nonom » Unassigned