serbanghita / Mobile-Detect

Mobile_Detect is a lightweight PHP class for detecting mobile devices (including tablets). It uses the User-Agent string combined with specific HTTP headers to detect the mobile environment.
http://mobiledetect.net
MIT License
10.52k stars 2.67k forks source link

Port functionality from `jenssegers/agent` #940

Open francislavoie opened 11 months ago

francislavoie commented 11 months ago

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 :frowning_face:

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 commented 10 months ago

@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 https://github.com/serbanghita/Mobile-Detect/issues/942

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

serbanghita commented 10 months ago

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

francislavoie commented 10 months ago

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 commented 10 months ago

I think this has become more urgent. See https://github.com/serbanghita/Mobile-Detect/pull/936#issuecomment-1796469285, 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 commented 10 months ago

@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 commented 10 months ago

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

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

👍

serbanghita commented 10 months ago

@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 commented 10 months ago

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

```php 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

```php $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 commented 10 months ago

@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 commented 10 months ago

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 commented 10 months ago

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 👍