jenssegers / agent

👮 A PHP desktop/mobile user agent parser with support for Laravel, based on Mobiledetect
https://jenssegers.com
MIT License
4.53k stars 473 forks source link

fix - package broken due to dependecy code change #208

Closed driade closed 11 months ago

driade commented 11 months ago

Hi.

Surely due to this change

https://github.com/serbanghita/Mobile-Detect/commit/ba9281b06937a01afdb72ae90064bd6ba4d35435

the package is broken as we add to "getOperatingSystems" a list of not really mobile ones.

Hope this helps.

cc https://github.com/jenssegers/agent/issues/207

driade commented 11 months ago

cc @https://github.com/ash-jc-allen/short-url/issues/222

driade commented 11 months ago

A temporary solution for those affected would be sticking mobiledetect/mobiledetectlib to version 2.8.41

composer require mobiledetect/mobiledetectlib 2.8.41

driade commented 11 months ago

Long explanation

This code prints "1" with mobiledetect 2.8.43, and "" with 2.8.41

use Jenssegers\Agent\Agent;

$agent = new Agent();
$agent->setUserAgent('Mozilla/5.0 (Macintosh; Intel Mac OS X 10_6_8) AppleWebKit/537.13+ (KHTML, like Gecko) Version/5.1.7 Safari/534.57.2');
echo $agent->isPhone();

This is due to a change in mobiledetect, that instead of using an internal list of mobile operating systems, uses the function getOperatingSystems().

public static function getOperatingSystems()
{
    return self::$operatingSystems;
}

This function is overriden by jenssegers/agent, which adds a few more operating systems

public static function getOperatingSystems()
{
        return static::mergeRules(
            static::$operatingSystems,
            static::$additionalOperatingSystems
        );
}

These additionalOperatingSystems on jenssen are not mobile ones.

 protected static $additionalOperatingSystems = [
        'Windows' => 'Windows',
        'Windows NT' => 'Windows NT',
        'OS X' => 'Mac OS X',
        'Debian' => 'Debian',
        'Ubuntu' => 'Ubuntu',
        'Macintosh' => 'PPC',
        'OpenBSD' => 'OpenBSD',
        'Linux' => 'Linux',
        'ChromeOS' => 'CrOS',
    ];
derekmd commented 11 months ago

The same issue also exists for the getBrowsers() method override. e.g., any user agent string containing "Firefox" or "Chrome" returns Agent::isMobile() === true and Agent::isDesktop() === false, despite not having a mobile device indicator in the string.

The Agent class getter method overrides would need to be renamed and/or removed.

For this package:

  1. 1.x likely just needs to pin composer require mobiledetect/mobiledetectlib:2.8.41 and archive that branch.
  2. 2.x could remove the method overrides and upgrade mobiledetect/mobiledetectlib that is currently on v4.

This package hasn't seen maintenance since 2021 so a community initiative is likely needed.

serbanghita commented 11 months ago

today: I'm reverting the change in Mobile Detect, will keep you posted.

driade commented 11 months ago

@serbanghita Thank you very much, this is really appreciated.

serbanghita commented 11 months ago

Should be fixed without intervention to jenssegers/agent

https://github.com/jenssegers/agent/issues/207#issuecomment-1800253853

driade commented 11 months ago

Thanks again @serbanghita , I'm closing this pull request