Hi,

The function at_get_browser(), which the mobile blocks/regions feature depends on, does not seem to be compatible with the current return format of the browscap_get_browser() function it relies on.

As it is now, in adaptivetheme 7.x-3.1, it does:

<?php
$browser
= browscap_get_browser();
if (
$browser['ismobiledevice'] == 1) {
 
$is_mobile = TRUE;
}
?>

But browscap 7.x-2.0 seem to set $browser['ismobiledevice'] to a string value 'true' rather than an actual boolean value.

Since you are checking if it's equal to 1, not if it's true, the expression returns false.

Proposed solution will be attached as patch.

Files: 

Comments

Status:Active» Closed (fixed)

Actually, never mind. Upon preparing the patch, and grabbing the latest git version, I see you've already changed it to:

<?php
if ($browscap['ismobiledevice'] == TRUE) {
 
// ...
}
?>

Which would have been my proposed solution. In other words; if anyone else experience 7.x-3.1 mobile functionality being borked... download the dev version.

Status:Closed (fixed)» Needs review
StatusFileSize
new511 bytes

On a closer look (I should have done this sooner, but... well... it's been a long week) browscap 7.x-2.0 also seems to return 'false' as a string when it is not a mobile browser. Unfortunately this means that the updated expression in your dev version would also behave unexpectedly... because PHP does not cast 'false' to the boolean false. Unlike for an example the string '0'.

There's a few ways this could be handled if you need to keep backwards compatibility for an earlier version which returned 1 or 0 instead of the current string format. The following method would achieve that, while making it more robust against changes made to browscap in the future:

<?php
if (filter_var($browscap['ismobiledevice'], FILTER_VALIDATE_BOOLEAN) == TRUE){
 
// ...
}
?>

The FILTER_VALIDATE_BOOLEAN filter means that it'll return TRUE for "1", "true", "on" and "yes". It'll return FALSE for all other values.

If you wanted to check more explicitly for TRUE/FALSE you could add the flag FILTER_NULL_ON_FAILURE to have it only return FALSE on "0", "false", "off", "no", and "" - and NULL for all "non-boolean" values.

php > var_dump(filter_var('true', FILTER_VALIDATE_BOOLEAN));
bool(true)
php > var_dump(filter_var('false', FILTER_VALIDATE_BOOLEAN));
bool(false)
php > var_dump(filter_var(1, FILTER_VALIDATE_BOOLEAN));
bool(true)
php > var_dump(filter_var(0, FILTER_VALIDATE_BOOLEAN));
bool(false)
php > var_dump(filter_var(true, FILTER_VALIDATE_BOOLEAN));
bool(true)
php > var_dump(filter_var(false, FILTER_VALIDATE_BOOLEAN));
bool(false)

To my knowledge there are no significant performance hits in using filter_var this way, but it will mean a dependency of PHP 5.2 or later.

Patch attached.

Issue summary:View changes

Removed junk

Version:7.x-3.1» 7.x-3.x-dev

I'm going to test with this solution, I have no issues depending on PHP 5.2.

Status:Needs review» Fixed

OK, I committed this, hooray, I will call this fixed.

Some of my clients urgently required this so I have kind of run out of patience waiting for Browscap to fix its own issues: #1868808: False/true not converted to 0/1 during browscap.ini import

Great! :)

(PS: It's no biggie, but git author attribution would have been nice. :P Every patch, even as small as this one, helps.. and people like being given credit where credit is due. You can find a ready formated command for the commit in peoples profiles, in case you were not aware. :-))

Ah shit, I did but I think I did not leave a space, so it went as a string in the commit message:

[7.x-3.x 45bd4bd] add workaround for browscap ismobiledevice unefined index issue--author=git <git@81265.no-reply.drupal.org>

http://drupal.org/node/519586/commits

Sorry man, I will be more careful next time.

Ah... nothing to worry about, shit happens. :-)

Thanks for keeping up the good work with AT!

Status:Fixed» Closed (fixed)

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

; Information added by drupal.org packaging script on 2013-04-17
version = "7.x-3.1+68-dev"

Notice: Undefined index: ismobiledevice в функции at_get_browser() (строка 228 в файле .\sites\all\themes\adaptivetheme\at_core\inc\get.inc).

Issue summary:View changes

Spelling/rephrasing