search

polyfillpolyfill / polyfill-service

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

Intl polyfill not provided for SamsungBrowser/2.1 #509

Closed hannesj closed 7 years ago

hannesj commented 8 years ago

I've noticed that for both user agents below, we don't supply the Intl polyfill, even tough it would be needed, as we see Intl is not defined errors from them.

Mozilla/5.0 (Linux; Android 5.0.2; SAMSUNG SM-G850F Build/LRX22G) AppleWebKit/537.36 (KHTML, like Gecko) SamsungBrowser/2.1 Chrome/34.0.1847.76 Mobile Safari/537.36

Mozilla/5.0 (Linux; Android 5.0; SAMSUNG SM-G900F Build/LRX21T) AppleWebKit/537.36 (KHTML, like Gecko) SamsungBrowser/2.1 Chrome/34.0.1847.76 Mobile Safari/537.36

triblondon commented 8 years ago

We identify both of those UA identifiers as chrome/34, and do not serve the Intl polyfill. Since we use the useragent npm module for UA identification, we'd need to either recognise it as a separate browser, or alias it to a browser that matches its behaviour more closely.

Yuripetusko commented 8 years ago

Seeing same issue:

on Samsung galaxy s6 Android 5.1 using Samsung's "Internet" browser OR Samsung galaxy s3 Android 4.1 using Android's stock "Browser" browser. Intl polyfill doesn't work giving me following error ReferenceError: Intl is not defined

antoinerousseau commented 8 years ago

Same problem here with a LG G3: ReferenceError: Intl is not defined

UA: Mozilla/5.0 (Linux; U; Android 5.0; es-sa; LG-D855 Build/LRX21R.A1445306351) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/34.0.1847.118 Mobile Safari/537.36

drawveloper commented 8 years ago

We had the exact same issue. Samsung Galaxy S6 "Internet" app. UA: Mozilla/5.0 (Linux; Android 5.1.1; SAMSUNG SM-G920F Build/LMY47X) AppleWebKit/537.36 (KHTML, like Gecko) SamsungBrowser/3.2 Chrome/38.0.2125.102 Mobile Safari/537.36

drawveloper commented 8 years ago

Apparently useragent@2.1.8 reports this as "Chrome mobile":

> ua.parse('Mozilla/5.0 (Linux; Android 5.1.1; SAMSUNG SM-G920F Build/LMY47X) AppleWebKit/537.36 (KHTML, like Gecko) SamsungBrowser/3.2 Chrome/38.0.2125.102 Mobile Safari/537.36')
Agent {
  family: 'Chrome Mobile',
  major: '38',
  minor: '0',
  patch: '2125',
  source: 'Mozilla/5.0 (Linux; Android 5.1.1; SAMSUNG SM-G920F Build/LMY47X) AppleWebKit/537.36 (KHTML, like Gecko) SamsungBrowser/3.2 Chrome/38.0.2125.102 Mobile Safari/537.36' }
drawveloper commented 8 years ago

@triblondon however if we look at the device property on the parsed user agent, we can verify that the "family" contains Samsung.

> ua.parse('Mozilla/5.0 (Linux; Android 5.1.1; SAMSUNG SM-G920F Build/LMY47X) AppleWebKit/537.36 (KHTML, like Gecko) SamsungBrowser/3.2 Chrome/38.0.2125.102 Mobile Safari/537.36').device
Device {
  family: 'Samsung $2',
  major: 'S

Maybe we could recognise this case as a separate browser and call it SamsungBrowser?

drawveloper commented 8 years ago

Sorry, this will already be solved by https://github.com/ua-parser/uap-core/pull/123. Then we just need to treat that new family.

JakeChampion commented 8 years ago

The useragent module can now detect Samsung, we need to update the module in our project and add Samsung into the config.json files for the polyfill/s in question.

JakeChampion commented 8 years ago

671 Should resolve this issue, please review :-)

bonimba commented 8 years ago

I am still having the same issue on a LG G3 and several other Android smartphones using the stock browser.

this is one of user agents: Mozilla/5.0 (Linux; U; Android 5.0; en-gb; LG-D855/V21a Build/LRX21R.A1446038723) AppleWebKit/537.36 (KHTML, like Gecko) Version/4.0 Chrome/34.0.1847.118 Mobile Safari/537.36. So it is recognized as Chrome 34 but it is actually the stock browser. Anyone still having this issue?

JakeChampion commented 8 years ago

@bonimba Could you make a new issue for that please? I don't think it is to do with Samsung browsers, looking at the useragent it does look like a Chrome useragent.

MasDevProject commented 8 years ago

Same problem here (galaxy s6 edge default Browser). I'm using this cdn:<script src="https://cdn.polyfill.io/v2/polyfill.min.js?features=Intl.~locale.en"></script>

JakeChampion commented 8 years ago

There is a PR for this functionality but it is on hold until we get support from Samsung to own that part of the effort -- https://github.com/Financial-Times/polyfill-service/pull/671

triblondon commented 8 years ago

The UA referenced in this issue is now being recognised as samsung internet/2.1.0. If someone can figure out the closest alias for this in the families that we support, then we could do something there. An imperfect but reasonable starting point would be to alias samsung internet to early versions of Chrome, which would mean potentially serving unnecessary polyfills, but false positives here are much less important than false negatives. And aliasing is cheap. Maintaining a whole new family is expensive.

Take a look at https://github.com/Financial-Times/polyfill-service/blob/master/lib/UA.js#L45.

mdmoreau commented 8 years ago

Would it make sense to alias it to older versions of the stock Android browser instead of Chrome? It seems like that could be a closer match based on my experience with the two.

AdaRoseCannon commented 8 years ago

The current version of Samsung browser is a fork of Chromium 44. So Chrome would be a good match.

mdmoreau commented 8 years ago

Ah that's good to know - thanks. For some reason I had assumed it was a fork of the stock Android browser.

triblondon commented 8 years ago

@AdaRoseEdwards Problem is Chrome has had Intl since 22 (http://caniuse.com/#search=intl), so maybe the SS fork removed support for Intl, somehow? Anyway basically the solution is simply to choose a sufficiently old version of Chrome.

@torgo do you think we could get some official determination on this from Samsung as a stopgap until we have proper support data? It's very simple for us to say 'treat version X of SB as an alias of version Y of Chrome', and it adds no maintenance overhead, so if there's a version combination of Chrome and SB where SB's featureset is a superset of Chrome's, we can use that for now and the only issue is that we will over-polyfill Samsung.

I really want to fix this as we have a fair number of people reporting it. I'll put this out as a suggestion:

samsung internet/2.1.0 == chrome/21

WDYT?

ruiaraujo commented 8 years ago

@triblondon Samsung Browser is a fork of Chromium but the they add features on their own.

The latest version supports Service Workers although it is based on Chromium 44. On the other hand it doesn't have Object.assign which Chromium 44 has.

samsung internet/2.1.0 == chrome/21 seems like a good idea, although each version should be matched with a different chrome version.

samsung internet/4.0 == chrome/43 maybe

JakeChampion commented 8 years ago

I believe this is solved by https://github.com/Financial-Times/polyfill-service/pull/783

hannesj commented 8 years ago

@JakeChampion, only for version 4 and up. We are still seeing quite a lot of v2.1 and v3 in the wild.

AdaRoseCannon commented 8 years ago

Samsung versions less than 4 get identified as versions of Chrome instead of Samsung Internet. But unlike chrome don't include the Intl support. Would it be Possible to add Intl to chrome <=34?

JakeChampion commented 8 years ago

I think we can instead opt to do some custom aliasing work like we have for Facebook's In-App browser #775

triblondon commented 8 years ago

if we identify the browser as chrome already an alias won't help, as we'd be changing the targeting for chrome as well. The solution is either to submit a PR to the ua-core project (if those early versions have any distinguishing characteristics in the UA at all), to make them distinguishable, or serve Intl to chrome < 34. I'm reluctant to do that because it's a large polyfill and Chrome doesn't need it after 24.

That said, serving it to an extra ten versions of Chrome that virtually no-one actually uses (due to auto-update) is not really a big deal...

Why are there so many Samsung 2.1s in the wild? Does Samsung have an auto-update mechanism?

ruiaraujo commented 8 years ago

@triblondon No they do not, this is the native Samsung browser so it is only updated when the phone is updated. At least for all the old versions.

Since Chrome < 34 has no real market share except for these forks, serving Intl to this seems like a pragmatic solution to this problem.

Turbo87 commented 8 years ago

Mozilla/5.0 (Linux; Android 5.1.1; SAMSUNG SM-G800F Build/LMY47X) AppleWebKit/537.36 (KHTML, like Gecko) SamsungBrowser/3.3 Chrome/38.0.2125.102 Mobile Safari/537.36 seems to be correctly identified as samsung_mob/3.3.0 according to https://cdn.polyfill.io/v2/polyfill.js?features=Intl&ua=Mozilla/5.0%20(Linux;%20Android%205.1.1;%20SAMSUNG%20SM-G800F%20Build/LMY47X)%20AppleWebKit/537.36%20(KHTML,%20like%20Gecko)%20SamsungBrowser/3.3%20Chrome/38.0.2125.102%20Mobile%20Safari/537.36, but unfortunately it is still not being served the Intl polyfill. Any ideas how to resolve that?

ruiaraujo commented 8 years ago

Add the option unknown=polyfill.

JakeChampion commented 7 years ago

PR for this --> https://github.com/Financial-Times/polyfill-service/pull/1028