Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] IntersectionObserver not polyfilled on iOS device #302

Closed
EECOLOR opened this issue Sep 6, 2019 · 24 comments
Closed

[BUG] IntersectionObserver not polyfilled on iOS device #302

EECOLOR opened this issue Sep 6, 2019 · 24 comments
Assignees

Comments

@EECOLOR
Copy link

EECOLOR commented Sep 6, 2019

What

I am receiving reports of the following error:

Can't find variable: IntersectionObserver

My polyfill request looks like this: https://polyfill.io/v3/polyfill.js?unknown=polyfill&features=default%2CSymbol%2Cfetch%2CArray.prototype.find%2CIntersectionObserver%2CWeakSet%2Ces2015%2Ces2016%2Ces2017&flags=gated

The user agent string: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.1.2 Safari/605.1.15

When I call the service with my user agent string spoofed I am receiving the following header in the javascript file:

/* Polyfill service v3.38.0
 * For detailed credits and licence information see https://github.com/financial-times/polyfill-service.
 * 
 * Features requested: Array.prototype.find,IntersectionObserver,Symbol,WeakSet,default,es2015,es2016,es2017,fetch
 * 
 * - _ESAbstract.Call, License: CC0 (required by "_ESAbstract.ToPrimitive", "_ESAbstract.ToString", "String.prototype.repeat", "es2015", "_ESAbstract.OrdinaryToPrimitive")
 * - _ESAbstract.CreateMethodProperty, License: CC0 (required by "Math.clz32", "es2015", "Math.sign", "String.prototype.repeat")
 * - _ESAbstract.Get, License: CC0 (required by "_ESAbstract.OrdinaryToPrimitive", "_ESAbstract.ToPrimitive", "_ESAbstract.ToString", "String.prototype.repeat", "es2015")
 * - _ESAbstract.IsCallable, License: CC0 (required by "_ESAbstract.GetMethod", "_ESAbstract.ToPrimitive", "_ESAbstract.ToString", "String.prototype.repeat", "es2015", "_ESAbstract.OrdinaryToPrimitive")
 * - _ESAbstract.RequireObjectCoercible, License: CC0 (required by "String.prototype.repeat", "es2015")
 * - _ESAbstract.ToInteger, License: CC0 (required by "String.prototype.repeat", "es2015")
 * - _ESAbstract.ToObject, License: CC0 (required by "_ESAbstract.GetV", "_ESAbstract.GetMethod", "_ESAbstract.ToPrimitive", "_ESAbstract.ToString", "String.prototype.repeat", "es2015")
 * - _ESAbstract.GetV, License: CC0 (required by "_ESAbstract.GetMethod", "_ESAbstract.ToPrimitive", "_ESAbstract.ToString", "String.prototype.repeat", "es2015")
 * - _ESAbstract.GetMethod, License: CC0 (required by "_ESAbstract.ToPrimitive", "_ESAbstract.ToString", "String.prototype.repeat", "es2015")
 * - _ESAbstract.ToUint32, License: CC0 (required by "Math.clz32", "es2015")
 * - _ESAbstract.Type, License: CC0 (required by "_ESAbstract.ToString", "String.prototype.repeat", "es2015", "_ESAbstract.ToPrimitive", "_ESAbstract.OrdinaryToPrimitive")
 * - _ESAbstract.OrdinaryToPrimitive, License: CC0 (required by "_ESAbstract.ToPrimitive", "_ESAbstract.ToString", "String.prototype.repeat", "es2015")
 * - _ESAbstract.ToPrimitive, License: CC0 (required by "_ESAbstract.ToString", "String.prototype.repeat", "es2015")
 * - _ESAbstract.ToString, License: CC0 (required by "String.prototype.repeat", "es2015")
 * - Math.clz32, License: CC0 (required by "es2015")
 * - Math.sign, License: CC0 (required by "es2015")
 * - String.prototype.repeat, License: CC0 (required by "es2015") */

A search in the rest of the file however shows no sign of IntersectionObserver.

It might be related to: #285

@JakeChampion
Copy link
Owner

This is now fixed -- polyfillpolyfill/polyfill-service#1936 (comment)

@EECOLOR
Copy link
Author

EECOLOR commented Sep 7, 2019

@JakeChampion unfortunately it is not fixed. Calling the service (https://polyfill.io/v3/polyfill.js?features=IntersectionObserver) using the Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.1.2 Safari/605.1.15 user agent string does not give the polyfill.

@EECOLOR
Copy link
Author

EECOLOR commented Sep 7, 2019

It's probably caused by the user agent being recognised as safari. And for safari it is configured like this:

safari = "< 12.1"

https://github.com/Financial-Times/polyfill-library/blob/master/polyfills/IntersectionObserver/config.toml

@EECOLOR
Copy link
Author

EECOLOR commented Sep 7, 2019

I checked the error log and these are the user agent strings that still produce errors (do not get polyfilled):

Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.1.2 Safari/605.1.15
Mozilla/5.0 (iPhone; CPU iPhone OS 12_4 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148
Mozilla/5.0 (iPhone; CPU iPhone OS 12_4 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148 ManagedBrowser/20181024.1

Note that the fix you linked did solve a few of the reported errors. This one for example does get polyfilled: Mozilla/5.0 (iPhone; CPU iPhone OS 12_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/16B92 [LinkedInApp]

So I think the detection script parses the OS 12_4 string as safari/12.4 which is incorrect. According to this list of Safari user agent strings the OS version does not determine the Safari version. It seems that it would need a Version/x.y.z string. In the case that Version is missing I think it should report unknown.

@EECOLOR
Copy link
Author

EECOLOR commented Sep 11, 2019

@JakeChampion Can you please reopen this ticket? Or should this ticket be moved to another repository?

@JakeChampion JakeChampion reopened this Sep 11, 2019
@EECOLOR
Copy link
Author

EECOLOR commented Sep 12, 2019

Ahh, I think the problem is most likely in one of these libraries:

https://github.com/Financial-Times/polyfill-useragent-normaliser
https://github.com/Financial-Times/useragent_parser

When I have some extra time I will try to get them running locally and hopefully eventually be able to get you a pull request.

@EECOLOR
Copy link
Author

EECOLOR commented Sep 12, 2019

Hmm, the problem seems to be even deeper: https://github.com/ua-parser/uap-core

This is from their tests:

  - user_agent_string: 'Mozilla/5.0 (iPod touch; CPU iPhone OS 9_3_2 like Mac OS X) AppleWebKit/601.1.46 (KHTML, like Gecko) Mobile/13F69'
    family: 'Mobile Safari UI/WKWebView'
    major: '9'
    minor: '3'
    patch: '2'

The OS version is used as the browser version. If you check any of the online user agent string analysers you get some mixed results about the version:

  • 9.3.2
  • 8+
  • -
  • null
  • undefined

https://en.wikipedia.org/wiki/Safari_version_history does not contain enough information either.

Here they state that the mobile safari version would probably be 601.1

@EECOLOR
Copy link
Author

EECOLOR commented Sep 12, 2019

I have files an issue at uap-core: ua-parser/uap-core#429

@EECOLOR
Copy link
Author

EECOLOR commented Oct 1, 2019

@JakeChampion there seems to be no activity in https://github.com/ua-parser/uap-core

Since you did most work on https://github.com/Financial-Times/polyfill-useragent-normaliser, do you have a suggestion on how to proceed?

With iOS reaching version 12 the problem of the Safari version being detected as the iOS version becomes greater. Over time more modern feature polyfills will not be served.

@EECOLOR
Copy link
Author

EECOLOR commented Oct 23, 2019

Sorry to bother you, still need some advise.

@EECOLOR
Copy link
Author

EECOLOR commented Jan 31, 2020

I got a response, created a pull request: ua-parser/uap-core#458

@JakeChampion
Copy link
Owner

Sorry for lack of communication on this @EECOLOR, if you give me a list of user-agent string and what you want them parsed as, I will add them to both:
https://github.com/Financial-Times/polyfill-useragent-normaliser
https://github.com/Financial-Times/useragent_parser
and we won't need to wait on ua-parser/uap-core#458

@EECOLOR
Copy link
Author

EECOLOR commented Feb 2, 2020

Sorry for lack of communication on this @EECOLOR

@JakeChampion no worries, it happens.

if you give me a list of user-agent string and what you want them parsed as, I will add them

If you check https://github.com/ua-parser/uap-core/pull/458/files you can see what I changed, which is essentially: "don't assume the OS version is the browser version for Apple devices". The pull request (the changes) also contains user agent strings and expected versions.

@JakeChampion JakeChampion added this to incoming in Origami ✨ Feb 3, 2020
@JakeChampion JakeChampion self-assigned this Feb 3, 2020
@EECOLOR
Copy link
Author

EECOLOR commented Feb 10, 2020

@JakeChampion The pull request to uap-core has been merged.

@EECOLOR
Copy link
Author

EECOLOR commented Feb 21, 2020

@JakeChampion any indication on when you can integrate the changes?

@EECOLOR
Copy link
Author

EECOLOR commented Mar 13, 2020

@JakeChampion now that uap-core has been merged, should I open a ticket at the useragent_parser or will you handle it from here?

@JakeChampion
Copy link
Owner

I've made a PR for it, Financial-Times/useragent_parser#42

@EECOLOR
Copy link
Author

EECOLOR commented Mar 17, 2020

@JakeChampion for some reason that pull request does not include the correct changes.

You can see it if you search for Mozilla/5.0 (iPhone; CPU iPhone OS 7_1_2 like Mac OS X) AppleWebKit/537.51.2 (KHTML, like Gecko) Mobile/11D257, that is still listed as 7.1.2 (the OS version).

If you look it up here you see it is undertermined: https://github.com/ua-parser/uap-core/blob/master/tests/test_ua.yaml

@JakeChampion
Copy link
Owner

@EECOLOR This is the one :-) Financial-Times/useragent_parser#43

@JakeChampion
Copy link
Owner

@EECOLOR I actually figured out how to detect the correct Safari version based on the AppleWebkit token in the useragent string so I went with that instead

@EECOLOR
Copy link
Author

EECOLOR commented Mar 19, 2020

@JakeChampion Great! Where did you find that information?

@JakeChampion
Copy link
Owner

From https://github.com/mdn/browser-compat-data/blob/master/browsers/safari_ios.json and wikipedia

@JakeChampion
Copy link
Owner

I checked the error log and these are the user agent strings that still produce errors (do not get polyfilled):

Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.1.2 Safari/605.1.15
Mozilla/5.0 (iPhone; CPU iPhone OS 12_4 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148
Mozilla/5.0 (iPhone; CPU iPhone OS 12_4 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148 ManagedBrowser/20181024.1

These all now work 🎉

I'll be closing this issue now as I've confirmed all useragents which were not working for IntersectionObserver now are.

If you find others, please feel free to comment back on this issue and I can reopen it, or we can start a new issue :-)

Origami ✨ automation moved this from incoming to complete Mar 19, 2020
@EECOLOR
Copy link
Author

EECOLOR commented Mar 20, 2020

@JakeChampion Thanks for the effort Jake, this was a tricky one!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 17, 2020
@robertboulton robertboulton removed this from Done in Origami ✨ Jul 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants