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

chrome 74 is identified as safari on http #935

Closed
fippo opened this issue Mar 27, 2019 · 12 comments
Closed

chrome 74 is identified as safari on http #935

fippo opened this issue Mar 27, 2019 · 12 comments

Comments

@fippo
Copy link
Member

fippo commented Mar 27, 2019

a variant of #764:
Chrome 74 makes getUserMedia securecontext, i.e. not available when using http and not https. This leads to chrome being detected as Safari (due to UA sniffing, bad!)
I expect Mozilla to follow here which means Firefox detection would be broken too

One potential solution might be to not run any shims on http and detect as unknown browser there. This isn't going to work great but there are just two legitimate use-cases on http:

  1. recveive-only video
  2. datachannel-only
    so the impact might be limited.
@dagingaa
Copy link
Collaborator

I think it's reasonable to not shim on http for adapter by default, and if it's needed either stay on old version, fork, or just use https.

Exception should be made for localhost perhaps (fairly common to consider localhost secure, but then again I guess feature detection isn't a problem in that case).

fippo added a commit that referenced this issue Mar 28, 2019
getUserMedia was recently made [SecureContext] and does not exist when running on http. This lead, similar to the scenario in #764 to Chrome being detected as Safari, activating the wrong shims.

RTCPeerConnection will still be available but can only be used for datachannels and receive-only contexts where adapter doesn't offer much benefit.

Fixes #935, requires a major version bump.
@guidou
Copy link
Collaborator

guidou commented Mar 28, 2019

+1

fippo added a commit that referenced this issue Mar 28, 2019
getUserMedia was recently made [SecureContext] and does not exist when running on http. This lead, similar to the scenario in #764 to Chrome being detected as Safari, activating the wrong shims.

RTCPeerConnection will still be available but can only be used for datachannels and receive-only contexts where adapter doesn't offer much benefit.

Fixes #935, requires a major version bump.
fippo added a commit that referenced this issue Mar 28, 2019
getUserMedia was recently made [SecureContext] and does not exist when running on http. This lead, similar to the scenario in #764 to Chrome being detected as Safari, activating the wrong shims.

RTCPeerConnection will still be available but can only be used for datachannels and receive-only contexts where adapter doesn't offer much benefit.

Fixes #935, requires a major version bump.
@mazzomazzo
Copy link

Guys,

This proposal is absolutely unacceptable.

Your case 1: recveive-only video is one of the most common uses of WebRTC.

All the applications that just play live streams sent from servers, will be affected for no reason. We have thousands of users, doing video surveillance from IP cameras, and they don't run their webpages over https, they don't need to. They just use http.

Look at this, you are going to break it for no reason:
http://livecam.nmu.edu/1/
http://livecam.nmu.edu/2/
http://livecam.nmu.edu/3/

@fippo
Copy link
Member Author

fippo commented Mar 28, 2019

@mazzomazzo how is that going to break? This would be the equivalent of adapter not being there, not RTCPeerConnection disappearing.

(i can see how it would break if you try the legacy offerToReceive* stuff in safari though)

@mazzomazzo
Copy link

Chrome does not implement 100% of standard, so shims from adapter are used.
Talk about Edge. These pages will not work in Edge any more.
Same about other browsers.

Please correct me if I am wrong and explain how
http://livecam.nmu.edu/1/
can work without adapter.

@fippo
Copy link
Member Author

fippo commented Mar 28, 2019

FWIW: I wouldn't recommend pulling adapter from github pages as this might lead to regressions when this gets updated without you knowing about it.

if you have an alternative solution to #764 that does not require looking for prefixed names or UA sniffing i'd be happy to hear about it.

@mazzomazzo
Copy link

  1. Why not look for prefixed names to identify browser? Everyone does that.
    Isn't there a reliable way to identify Chrome?
    Like navigator.userAgent.match(/Chrom(e|ium)/(\d+)./))?

  2. I hope you are not proceeding with this approach of dropping support for insecure contexts.

  3. If you are proceeding, then will there be a branch, or version on github that will still support insecure contexts, that we could rely on?

@fippo
Copy link
Member Author

fippo commented Mar 28, 2019

  1. this is called UA sniffing. It doesn't work well and as you have seen misidentified Chrome 74 on http as safari. See Chrome will be identified as Safari if webkitGetUserMedia is removed #764 for more discussion.
  2. I would prefer not to but wrongly identifying as Safari is as bad.
  3. https://webrtc.github.io/adapter/adapter-7.2.1.js is the current version. Versioned releases don't get updated.

@mazzomazzo
Copy link

  1. Is there really no reliable way to detect Chrome/Chromium? Why can't we all ask Chrome people to introduce some unique thing in Chrome for that purpose?

  2. You decide just to amputate a leg because of ugly tattoo. Yes, detecting Chrome as Safari is bad, but solving it like this is cruel and leads to much worse consequences. And again, isn't there a reliable way to identify Safari? Amazing if this is so.

  3. Just educate me - the adapter_factory.js and utils.js needed by adapter-7.2.1.js, what about these files?
    These don't change? Or are there specific versions for these too?

@dagingaa
Copy link
Collaborator

dagingaa commented Mar 28, 2019 via email

@fippo
Copy link
Member Author

fippo commented Apr 1, 2019

the other alternative would be to check for webkitRTCPeerConnection iff webkitGetUserMedia is not there . We assumed I think that doesn't apply to safari (which does not have webkitRTCPeerConnection) and for Edge (which oddly exposes that) we can detect its edge-y ness by checking for RTCIceGatherer.

fippo added a commit that referenced this issue Apr 2, 2019
Fixes #935 which is the chrome detection on insecure contexts where webkitGetUserMedia was removed along with getUserMedia in M74+.
Fallback is to check for webkitRTCPeerConnection which avoiding Edge by checking for RTCIceGatherer.
@fippo fippo closed this as completed in #942 Apr 9, 2019
@fippo
Copy link
Member Author

fippo commented Apr 9, 2019

This should be fixed (without dropping http support) in v7.2.3 which just landed on npm as well as adapter-latest.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants