Contao loads desktop cache file if no mobile cache file is present

[fritzmg]

Problem description

If you use the server cache and a mobile page layout and there is a cache file present for the desktop version for a specific page, but not for the mobile version, then Contao will load the desktop version for mobile devices as well.

Reproduction

• Activate the page cache
• Use different page layouts for desktop and mobile
• In your desktop browser, open a specific page in the frontend (without being logged into the backend of course) other than the index page (see Seitencache und Spracherkennung #7618#issuecomment-83171643), so that its cache file will be generated
• Now open the same page on your smartphone - it will load the desktop version

Cause

The problem is the mobile check in FrontendIndex.php::outputFromCache():

// Check for a mobile layout
if (\Input::cookie('TL_VIEW') == 'mobile' || (\Environment::get('agent')->mobile && \Input::cookie('TL_VIEW') != 'desktop'))
{
    $strMd5CacheKey = md5($strCacheKey . '.mobile');
    $strCacheFile = TL_ROOT . '/system/cache/html/' . substr($strMd5CacheKey, 0, 1) . '/' . $strMd5CacheKey . '.html';
    if (file_exists($strCacheFile))
    {
        $blnFound = true;
    }
}
// Check for a regular layout
if (!$blnFound)
{
    $strMd5CacheKey = md5($strCacheKey);
    $strCacheFile = TL_ROOT . '/system/cache/html/' . substr($strMd5CacheKey, 0, 1) . '/' . $strMd5CacheKey . '.html';
    if (file_exists($strCacheFile))
    {
        $blnFound = true;
    }
}

The first condition will set $blnFound to true, if there is a cache file present for the mobile version. However, if no cache file was found for the mobile version, the second condition will execute and thus the desktop cache file will be loaded, if there is one present, even if the requested page has a mobile layout defined.

I don't know of an easy fix, since we probably do not want to query whether the requested page needs to have a mobile layout or not.

Reference: https://community.contao.org/de/showthread.php?57144-Contao-l%E4dt-Desktop-Version-aus-Cache

[dmolineus]

The simplest solution would be to cache always a mobile and desktop version of a page. If the condition \Input::cookie('TL_VIEW') == 'mobile' || (\Environment::get('agent')->mobile && \Input::cookie('TL_VIEW') != 'desktop') is true, the page cache key should add the .mobile flag no matter if a separate mobile layout is used or not.

Then I would drop the fallback to the desktop page by changing the mentioned if (!$blnFound) to a simple else.

The only drawback I see is that the page cache get blown up no matter if a mobile layout is used or not.

Another solution could be to add also a .desktop flag for desktop layouts if a separate mobile layout is used. And only for pages without a defined mobile layout the page is cached without the layout flag.

[fritzmg]

The only drawback I see is that the page cache get blown up no matter if a mobile layout is used or not.

Also, (first) page generation time could double this way. Your second solutions sounds better to me :)

[leofeyer]

Shouldn't it suffice to replace the second if (!$blnFound) with else?

// Check for a mobile layout
if (\Input::cookie('TL_VIEW') == 'mobile' || (\Environment::get('agent')->mobile && \Input::cookie('TL_VIEW') != 'desktop'))
{
    $strMd5CacheKey = md5($strCacheKey . '.mobile');
    $strCacheFile = TL_ROOT . '/system/cache/html/' . substr($strMd5CacheKey, 0, 1) . '/' . $strMd5CacheKey . '.html';

    if (file_exists($strCacheFile))
    {
        $blnFound = true;
    }
}

// Check for a regular layout
else
{
    $strMd5CacheKey = md5($strCacheKey);
    $strCacheFile = TL_ROOT . '/system/cache/html/' . substr($strMd5CacheKey, 0, 1) . '/' . $strMd5CacheKey . '.html';

    if (file_exists($strCacheFile))
    {
        $blnFound = true;
    }
}

[dmolineus]

Shouldn't it suffice to replace the second if (!$blnFound) with else?

Unfortunately no. Then mobile pages which does not have a mobile layout wouldn't be loaded from the cache.

[leofeyer]

You are right.

[leofeyer]

I have created a PR here: #7859

@dmolineus Can you please checkout the branch and see if the change fixes the issue?

[dmolineus]

It does not work yet. See the comments in the pull request, please.

[leofeyer]

I'm closing the ticket in favor of #7859.