Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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:
$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.
Comment | File | Size | Author |
---|---|---|---|
#2 | adaptivetheme-at_get_browser_broken-1898936-2.patch | 511 bytes | sigveio |
Comments
Comment #1
sigveio CreditAttribution: sigveio commentedActually, never mind. Upon preparing the patch, and grabbing the latest git version, I see you've already changed it to:
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.
Comment #2
sigveio CreditAttribution: sigveio commentedOn 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:
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.
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.
Comment #2.0
sigveio CreditAttribution: sigveio commentedRemoved junk
Comment #3
Jeff Burnz CreditAttribution: Jeff Burnz commentedI'm going to test with this solution, I have no issues depending on PHP 5.2.
Comment #4
Jeff Burnz CreditAttribution: Jeff Burnz commentedOK, 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
Comment #5
sigveio CreditAttribution: sigveio commentedGreat! :)
(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. :-))
Comment #6
Jeff Burnz CreditAttribution: Jeff Burnz commentedAh 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.
Comment #7
sigveio CreditAttribution: sigveio commentedAh... nothing to worry about, shit happens. :-)
Thanks for keeping up the good work with AT!
Comment #9
DD 85 CreditAttribution: DD 85 commented; 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).
Comment #9.0
DD 85 CreditAttribution: DD 85 commentedSpelling/rephrasing