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 will be identified as Safari if webkitGetUserMedia is removed #764

Closed
foolip opened this issue Jan 31, 2018 · 19 comments · Fixed by #1146
Closed

Chrome will be identified as Safari if webkitGetUserMedia is removed #764

foolip opened this issue Jan 31, 2018 · 19 comments · Fixed by #1146

Comments

@foolip
Copy link

foolip commented Jan 31, 2018

A continuation of https://github.com/webrtc/adapter/issues/712, or a subset thereof. After some changes this is the remaining problematic code:
https://github.com/webrtc/adapter/blob/0a293a9803dcd612cb73c147218a81d56405d6bd/src/js/utils.js#L151

This is probably blocking https://bugs.chromium.org/p/chromium/issues/detail?id=692736, as being identified as Safari might have other consequences.

@KaptenJansson
Copy link
Collaborator

What about using something unique instead of a related WebRTC Api like getUserMedia?

Like window.chrome ?

@KaptenJansson
Copy link
Collaborator

@fippo @alvestrand

@rtzm
Copy link

rtzm commented Mar 18, 2018

I've run into this issue as well when trying to use this library for an app that I want to run on mobile. The browser when running on Chrome for iOS is being read as Safari version 604. Can /Chrome/ be read in from the User Agent with something like navigator.userAgent.match(/Chrom(e|ium)\/(\d+)\./))?

Otherwise, this is a really helpful library! thanks a bunch, all!

@fippo
Copy link
Member

fippo commented Mar 18, 2018

@rtzm but in that case you want the shim code to be the safari shim even though it is chrome?

@KaptenJansson hrm... how does that behave in chrome on ios?

@rtzm
Copy link

rtzm commented Mar 22, 2018

I just manually tested window.chrome and window.chrome.webstore on chrome for iOS, and both are undefined, although both should work for Chrome for desktop. Made a PR to add this feature detection here.

It also looks like detecting the correct method for getUserMedia on Chrome for iOS is moot, because iOS 11 doesn't allow Chrome to implement WebRTC: http://www.openradar.me/33571214.

@zaxy78
Copy link

zaxy78 commented Mar 22, 2018

@rtzm all browser on iOS are built on top of (WKWebVIew with) Apple's WebKIt engine.
Sadly, all Chrome has is a nice wrapped package for us. that's it. (see Google's past announcement)

So where getUserMedia() works on Safari, it's still blocked on WKWebView..

@fippo
Copy link
Member

fippo commented Mar 22, 2018

window.chrome.webstore does not work in Brave unfortunately which supports RTCPeerConnection (and is, no surprise, using the chrome version.
And just checking for window.chrome yields true in Edge :-(

@rtzm
Copy link

rtzm commented Mar 23, 2018

haha, ok, closing my PR, since it looks like this won't be moving forward this way ¯_(ツ)_/¯

@fippo
Copy link
Member

fippo commented Apr 11, 2018

wondering if just window.chrome is a good way to identify chrome. This would cover chrome, opera and brave at least.

@foolip do you know if there is any description of window.chrome?

@zaxy78
Copy link

zaxy78 commented Apr 11, 2018 via email

@fippo
Copy link
Member

fippo commented Apr 11, 2018

@zaxy78 good point, it does. However we can tell Edge apart since it has RTCIceGatherer from ORTC

@foolip
Copy link
Author

foolip commented Apr 11, 2018

I know there are Chromium-based browsers which don't have window.chrome, and some that do, so I'd say that's worse that UA sniffing :)

@fippo
Copy link
Member

fippo commented Mar 27, 2019

https://bugs.chromium.org/p/chromium/issues/detail?id=946516
-- this breaks on http because Chrome 74 removed webkitGetUserMedia there.

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.
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
Copy link
Member

fippo commented May 22, 2022

https://web.dev/migrate-to-ua-ch/ might solve the problem. We just need the major version anyway.

@fippo
Copy link
Member

fippo commented Mar 6, 2024

@beaufortfrancois can you find folks with an opinion on the approach in #1103 ?

@beaufortfrancois
Copy link

@yoavweiss and @miketaylr may have opinions on this issue

@yoavweiss
Copy link

I don't have a ton of context (e.g. why do we need to detect Chromium here?), but superficially, the approach seems reasonable.

@fippo
Copy link
Member

fippo commented Mar 7, 2024

Some WebRTC features are only available in certain versions (and there are bugs so you often need version detection anyway) so there is a ton of application code that looks "what browser am I in and what workaround is required here. Proper feature detection is better but a lot of these things do not have any exposed API surface.

Shouldn't be necessary in 2024 but ... it is. Let me see if I can revive #1103...

@miketaylr
Copy link

See also whatwg/compat#266

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