polyfillpolyfill / polyfill-service

Automatic polyfill service.
https://polyfill.io/
MIT License
7.53k stars 741 forks source link

Facebook iOS app browser not supported #561

Closed stensrud closed 8 years ago

stensrud commented 8 years ago

The Facebook native iOS app uses a UIWebView with a custom user-agent. Ideally Polyfill should return a polyfill for the current Safari/iOS version, but returns an error message.

Here is a polyfill request with a Facebook user-agent:

curl "https://cdn.polyfill.io/v2/polyfill.min.js?features=Object.assign,Promise,Array.prototype.find,Array.prototype.findIndex,Intl,Intl.~locale.nb&flags=always,gated" --header "User-agent: Mozilla/5.0 (iPhone; CPU iPhone OS 9_2 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Mobile/13C75 [FBAN/FBIOS;FBAV/46.0.0.54.156;FBBV/18972819;FBDV/iPhone8,1;FBMD/iPhone;FBSN/iPhone OS;FBSV/9.2;FBSS/2; FBCR/Telenor;FBID/phone;FBLC/nb_NO;FBOP/5]"

Returns:

/* Unsupported UA detected: facebook/46.0.0 */
stensrud commented 8 years ago

My temporary solution is to add a modified ´ua´ query string parameter on the backend if Facebook is detected:

var m = userAgent.match(/^(.+)(\[.+FBIOS.+\][ ]*)/);
if (m) {
    return '&ua='+encodeURIComponent(m[1]+' Safari');
}
triblondon commented 8 years ago

ok, this is easy enough - seems like the UA parser recognises the FB browser but we don't support it in the service. What does the 46 refer to in the UA string and is it bound to the iOS version or the FB app version?

esseb commented 8 years ago

FBAV/46 could be Facebook App Version, but I can only find assumptions online, no official documentation from Facebook:

stensrud commented 8 years ago

Even if it's an assumption, we can be pretty sure 46 is the app version. It corresponds to the version number from the App Store.

Running the useragent-manually returns: { family: 'Facebook', major: '46', minor: '0', patch: '0', device: { family: 'iPhone8,1', major: '0', minor: '0', patch: '0' }, os: { family: 'iOS', major: '9', minor: '2', patch: '0' } }

The big question, imho, is whether it is reliable enough to detect rendering engine and browser features using this massive user-agent library. Facebook is just one out of hundreds or thousands of webview-based browsers. How about Twitter? Pinterest? Slack? LinkedIn? 1Password? RSS readers?

For iOS devices (that's what i know), it would be safer to detect OS version. iOS disallows 3rd party web rendering engines, so you can be pretty sure what engine is in use just knowing the OS version.

triblondon commented 8 years ago

Agreed. This is probably one to raise in ua-parser.

stensrud commented 8 years ago

Well, the ua-parser project did actually do a lot of extra work to create this bug (keeping track of 1000s of user agents), and it's not hard to imagine use cases where this is the wanted behavior, for instance analytics. Ideally, the ua-parser project would detect rendering engine in addition to browser and os, but that would be a lot to ask from them.

I would suggest switching to os-detection for iOS, or maybe a WebKit fallback for non-recognized browsers. It depends of your goals for this project. If polyfill-service should work on the open web, and not just for internal tools, a catch-all solution is probably required.

triblondon commented 8 years ago

OK, fair point. I am totally fine with aliasing the Facebook browser version sequence to the versions we already recognise for ios_saf, which are iOS version numbers. We have an existing mechanism for aliasing, but it would need to be tweaked to use os data for the target version.

Ultimately this would enable the Facebook browser to be recognised by us as ios_saf/8 or whatever iOS version. You could start by taking a look at service/UA.js.

rixth commented 8 years ago

I am seeing breakage as the Intl polyfill is not being returned for the FB in app browser either.

UA: Mozilla/5.0 (iPhone; CPU iPhone OS 9_1 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Mobile/13B143 [FBAN/FBIOS;FBAV/44.0.0.54.111;FBBV/17684769;FBDV/iPhone7,2;FBMD/iPhone;FBSN/iPhone OS;FBSV/9.1;FBSS/2; FBCR/Verizon;FBID/phone;FBLC/en_US;FBOP/5]

API response:

/* Polyfill service v3.4.2
 * For detailed credits and licence information see http://github.com/financial-times/polyfill-service.
 *
 * UA detected: facebook/44.0.0 (unknown/unsupported; using policy `unknown=ignore`)
 * Features requested: Intl.~locale.en
 *  */
triblondon commented 8 years ago

I think the solution here is likely to be as proposed in https://github.com/Financial-Times/polyfill-service/issues/561#issuecomment-169111053

yamadapc commented 8 years ago

I've this and a similar issue with

curl "https://cdn.polyfill.io/v2/polyfill.js?features=Object.assign,Promise,Array.prototype.find,Array.prototype.findIndex,Intl,Intl.~locale.nb&flags=always,gated" --header "User-Agent: Mozilla/5.0 (Linux; Android 4.4.4; SAMSUNG SM-J100M Build/KTU84P) AppleWebKit/537.36 (KHTML, like Gecko) SamsungBrowser/2.0 Chrome/34.0.1847.76 Mobile Safari/537.36"

I'd rather have the polyfill injected when the UA isn't supported. Does that not sound like a sensible solution?

EDIT: Sorry, I didn't know about the unknown option.

andyburke commented 7 years ago

I'm still seeing this. Is this supposed to be fixed? We're explicitly including setImmediate support, but it's not working in Facebook's in-app browser.

cc @jacklee0223

JakeChampion commented 7 years ago

This is fixed, please open a new issue with a reduced test case if you are having issues.

Here is the PR which resolved the issue -- https://github.com/Financial-Times/polyfill-service/pull/775

andyburke commented 7 years ago

@JakeChampion Thanks, here's a FB story link that links to the following HTML:

https://m.facebook.com/story.php?story_fbid=10154490930306508&id=739196507

<!DOCTYPE html>
<html lang="en">

<head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1">

    <title>FB Browser Window setImmediate repro</title>

    <script src="https://cdn.polyfill.io/v2/polyfill.min.js?features=default,setImmediate"></script>
</head>

<body>
    <script type="text/javascript">
        setImmediate( function() {
            alert( 'setImmediate worked' );
        } );
    </script>
<body>
andyburke commented 7 years ago

@JakeChampion any update on this? get a chance to try the repro above?

JakeChampion commented 7 years ago

Sorry, I haven't reproduced this yet, I need to locate an iOS device and download Facebook. Could you please open this as a new issue, that way it will be easier for other contributors to see the issue and attempt to reproduce it also.