Port functionality from jenssegers/agent

[francislavoie]

We've been using https://github.com/jenssegers/agent for quite a long time, but it seems it's fallen out of maintenance, and hasn't been updated to use Mobile-Detect v3 ☹️

We can't really switch away from it and onto Mobile-Detect though, because Agent adds functionality that's critical to our usecase,

For example, Agent has best-effort OS detection, and a few helper methods like ->isDesktop(), ->browser() and ->platform(). We also chain the output of those methods into ->version() (using the property name returned by those) to get specific browser/OS versions.

Would you be willing to review and port some of that functionality, so we can do away with that library, allowing us to remove a link in the dependency chain?

[serbanghita]

@francislavoie I have explored the versioning situation in the past and I had to stop because it became complex, that's why I decided to focus MobileDetect only on detecting "mobile" and "tablets".

I'm still thinking if getting back to this is viable. I don't have time to port anything from jenssegers/agent especially with the BS proposals from Google that can impact the way User-Agents will look in the future #942

I'm still trying to find additional motivation. Maybe ask @jenssegers if he can do an update ...

[serbanghita]

@francislavoie can you briefly discuss your business case for using platform and version?

[francislavoie]

We use platform and version to keep track of what device the user last used for support purposes.

Our system is complex and uses a mix of modern and legacy browser APIs, and having a best-guess for what kind of client they're using is very helpful. Sometimes support staff can say "you're using too old a browser, please make sure to update". You'd be surprised how often we see old desktop Chrome versions because the user found themselves in a way that prevented the automatic updates etc.

It also helps for keeping tabs on the market share usage, like the % of mobile vs desktop users which helps tell us what experience is more important to target.

It can also be useful as a security measure to help verify that the user's session wasn't hijacked; if their user agent changes during their session, we can close the session and force them to re-login (though we don't strictly need this lib for that, but we rather use the derived agent data rather than the raw user agent string).

[francislavoie]

I think this has become more urgent. See #936 (comment), users are now stuck pinning to a specific older version of Mobile-Detect (2.8.41) because of a breaking change in 2.8.42.

It would be awesome if we could drop Agent directly, but it needs to have feature parity before we can do that.

[serbanghita]

@francislavoie thanks for pointing that, I've fixed it last night. Luckily we don't have to change anything in jenssegers/agent

back to the topic at hand, it looks like you need a stable solution to extract the following data from every User-Agent:

$md->getOs(); // "Mac OS"
$md->getOsVersion(); // "10.15.7"
$md->getBrowser(); // "Chrome"
$md->getBrowserVersion(); // "117.0.0.0"

I believe I can include this in MobileDetect or a sister class called UserAgent or smth like that.

What about: Browser engine, browser engine version, Device model, device brand ?

[JayBizzle]

Not forgetting isRobot() - https://github.com/jenssegers/agent#robot-detection

Which relies on my package - https://github.com/JayBizzle/Crawler-Detect

👍

[serbanghita]

@JayBizzle I was just checking your project because I saw it as a dep of agent, I'll put it in the README.md since there are a lot of ppl asking about detecting bots 👍

[francislavoie]

IMO it's not necessary for Mobile-Detect to require Crawler-Detect because it works standalone. Agent just includes it as a passthrough, it doesn't do anything special with it, it just hands it $this->userAgent which can be grabbed from Mobile-Detect with $mb->getUserAgent() if you need.

FWIW, this is the code my app currently uses for pulling out the information we keep track of for support purposes:

Details

    private static $os_names = [
        1  => 'AndroidOS',
        2  => 'Apple', // Deprecated, too ambiguous
        3  => 'iOS',
        4  => 'iPadOS',
        5  => 'OS X', // NOTE: Should be "macOS" but Agent doesn't support that yet
        6  => 'Windows',
        7  => 'Linux',
        8  => 'Ubuntu',
        9  => 'Debian',
        10 => 'OpenBSD',
        11 => 'ChromeOS',
        99 => 'Unknown',
    ];

    private static $browser_names = [
        1  => 'Chrome',
        2  => 'Safari',
        3  => 'Firefox',
        4  => 'Android Internet Browser', // Deprecated, Agent doesn't return this value
        5  => 'IE',
        6  => 'Edge',
        7  => 'UCBrowser',
        8  => 'Opera',
        99 => 'Unknown',
    ];

    /**
     * Uses the Agent library to attempt to determine the user client info
     *
     * @param stdClass|null $extraDeviceDetails
     * @param string|null $userAgent Override the user agent from the current request
     * @return array
     */
    public static function detect($extraDeviceDetails = null, $userAgent = null)
    {
        $agent = new Agent();

        if (isset($userAgent)) {
            $agent->setUserAgent($userAgent);
        }

        // Simply determine if we think it's a mobile device
        $isMobile = $agent->isMobile() ? 1 : 0;

        // Determine the operating system
        $osName = $agent->platform();
        $osByNames = array_flip(self::$os_names);
        $os = $osByNames[$osName] ?? $osByNames['Unknown'];
        $osVer = isset($osByNames[$osName])
            ? $agent->version($osName, Agent::VERSION_TYPE_FLOAT)
            : false;

        // Shim to override Mac to iPadOS if touch support is detected
        // Newer iPads report themselves as Mac/desktop, which is problematic.
        // We won't know the iPadOS version either unfortunately.
        if (($extraDeviceDetails->maxTouchPoints ?? 0) > 1) {
            if ($os === $osByNames['OS X']) {
                $os = $osByNames['iPadOS'];
                $osVer = -1;
                $isMobile = 1;
            }
        }

        // Determine the web browser
        $browserName = $agent->browser();
        $browsersByNames = array_flip(self::$browser_names);
        $browser = $browsersByNames[$browserName] ?? $browsersByNames['Unknown'];
        $browserVer = isset($browsersByNames[$browserName])
            ? $agent->version($browserName, Agent::VERSION_TYPE_FLOAT)
            : false;

        return [
            'osName'         => $os,
            'osVersion'      => $osVer !== false ? $osVer : -1,
            'browserName'    => $browser,
            'browserVersion' => $browserVer !== false ? $browserVer : -1,
            'isMobile'       => $isMobile,
        ];
    }

Elsewhere, we also do this, to allow our JS client to check its own platform for some platform-specific fixes:

Details

            $agent = new Agent;
            $agent->setUserAgent($request->useragent);
            $response->device = $agent->isDesktop() ? 'desktop' : 'mobile';

            // Add some device-specific OS checks
            $response->isIOS = $agent->is('iOS');
            $response->isAndroid = $agent->is('AndroidOS');
            $response->isWindowsPhone = $agent->is('WindowsPhoneOS');

            // Shim to override Mac detection to iPad if touch support is detected
            // Newer iPads report themselves as Mac/desktop, which is problematic.
            if (($request->extraDeviceDetails->maxTouchPoints ?? 0) > 1) {
                if ($agent->is('OS X')) {
                    $response->device = 'mobile';
                    $response->isIOS = true;
                }
            }

(Also our JS client may send extraDeviceDetails: { maxTouchPoints: navigator.maxTouchPoints ?? null } which helps with detecting iPad vs desktop Safari, in the above code)

Hopefully that helps give you a better picture of my needs. Though of course I'm not everyone, and others might use more of Agent's feature set.

[JayBizzle]

@francislavoie I actually do agree with you about what functionality should be ported to MobileDetect. With Crawler Detect, we have been vary wary about feature creep. We have tried to keep it focussed on doing one job, and doing that job very well.

Porting features from agent I feel may be out of scope of this library, but there is definitely some functionality that could crossover.

For instance, should MobileDetect be able to determine desktop browser versions? Seems out of scope for a library aimed at detecting mobile user agents.

I think a new library that includes MobileDetect, and CrawlerDetect would be the way to that adds the functionality of agent

[francislavoie]

For instance, should MobileDetect be able to determine desktop browser versions? Seems out of scope for a library aimed at detecting mobile user agents.

I think it's very related. "Mobile" seems like a weird line to draw IMO because you now have (originally) desktop browsers running on mobile devices. Being able to distinguish one from the other requires the full dataset, you can't easily check if a user agent is desktop without also having the dataset of all mobile devices to cross-ref.

For example, in Agent, isDesktop actually means: !isMobile && !isTablet && !isRobot. Basically it's easier to guess it's desktop when you rule out that it's mobile.

I think a new library that includes MobileDetect, and CrawlerDetect would be the way to that adds the functionality of agent

Well that's the whole problem here, Agent was that, and is no longer maintained. I'm trying to help avoid this problem in the future by consolidating the little bit of added functionality into the parent library which is maintained.

[JayBizzle]

Like I said, I'm not totally disagreeing with you here, I guess I'm playing devil advocate, and highlighting how we may need to be careful with what feature get ported.

So, if you want isDesktop() ported to here, and you say !isMobile && !isTablet && !isRobot is basically what happens under the hood, then wouldn't that mean MobileDetect should require CrawlerDetect to handle the isRobot part?

Just to be clear, I'm not for, or against any pathway here, I just want to highlight these things as discussion points 👍