mikemaccana / outdated-browser-rework

Detects outdated browsers and advises users to upgrade to a new version. Handles mobile devices!
MIT License
224 stars 62 forks source link

Firefox Mobile on iOS gets banner #98

Closed benmccallum closed 3 years ago

benmccallum commented 4 years ago

One of our team members (@maraisr) uses Firefox Mobile on iOS and he gets the banner when I'd imagine he shouldn't. I'll get him to supply more info.

benmccallum commented 4 years ago

@maraisr, can you provide some more details (iOS version, FF Mobile app version, etc) and a screenshot of the banner appearing? Cheers!

benmccallum commented 4 years ago

Perhaps chuck in the useragent if you can find it too, so we could debug the script with that.

iManu commented 4 years ago

Hi, I encounted the same with an user on iOS and Firefox.

Actually our config is Firefox: 50, is "Mobile Firefox" could work the same as "Mobile Safari" param ?

This is a capture on my own iOS and fresh mobile FF, but you need to take care of Android too (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent/Firefox): IMG_8313

benmccallum commented 4 years ago

As a string that's copy-pasteable: Mozilla/5.0 (iPhone; CPU OS 13_3_1 like Mac OS X). AppleWebKit/605.1.15 (KHTML, like Gecko) FxiOS/23.0 Mobile/15E148 Safari/605.1.15

For breakpoint in code:

parsedUserAgent = new UserAgentParser('Mozilla/5.0 (iPhone; CPU OS 13_3_1 like Mac OS X). AppleWebKit/605.1.15 (KHTML, like Gecko) FxiOS/23.0 Mobile/15E148 Safari/605.1.15').getResult()

Which equates to:

{
    "ua": "Mozilla/5.0 (iPhone; CPU OS 13_3_1 like Mac OS X). AppleWebKit/605.1.15 (KHTML, like Gecko) FxiOS/23.0 Mobile/15E148 Safari/605.1.15",
    "browser": {
        "name": "Firefox",
        "version": "23.0",
        "major": "23"
    },
    "engine": {
        "name": "WebKit",
        "version": "605.1.15"
    },
    "os": {
        "name": "iOS",
        "version": "13.3.1"
    },
    "device": {
        "vendor": "Apple",
        "model": "iPhone",
        "type": "mobile"
    },
    "cpu": {}
}
benmccallum commented 4 years ago

So the issue is that it's picking the version out as 23, whatever that is. Perhaps the Safari Mobile version since it uses the safari engine on iOS?

I guess we need some code that detects for parsedUserAgent.browser.name === 'Firefox' && parsedUserAgent.engine/os.name === 'WebKit/iOS' and then use say another config specifically forFirefox Mobile on iOS` or something?

How does that sound @mikemaccana ?

benmccallum commented 4 years ago

Or, given the link @iManu shared, under "Firefox for iOS", it looks like it's just using the Mobile Safari UA string with a tacked on bit to identify it as FF, so that makes me think it behaves exactly the same and we should detect this and use whatever the developer sets their min version for Mobile Safari as. i.e. we may not need to worry about giving it it's own setting for separate configuration, and can just make a note in the readme.

What do you think @maraisr?

iManu commented 4 years ago

I think you're right, set the min version for Mobile Safari do the trick

benmccallum commented 4 years ago

@iManu , I've addressed this in the same branch as I had worked on for Edge Chromium version support. Just waiting on @mikemaccana to merge and publish to npm. Not much else I can do.

mikemaccana commented 4 years ago

Thanks @benmccallum! If you add the tests I'll merge.

benmccallum commented 4 years ago

I don't know if there are tests in this repo?

I validated the branch's changes with:

  1. my own Edge (on Chromium), and
  2. for FF Mobile I just dumped the useragent above in the src (index.js - line 43), built, and in the index.html modified the Mobile Safari value up and down over 13.3 to test that.