search

jitsi / js-utils

Utilities for Jitsi JS projects
Apache License 2.0
32 stars 60 forks source link

Detection of chromium based browsers and runtimes not exposing chrome extensions API fails (i.e. webOS) #20

Closed jdapena closed 4 years ago

jdapena commented 4 years ago

https://github.com/jitsi/js-utils/blob/abc73a98a61124aa12c87c72dd0d86985a1a4d41/browser-detection/BrowserDetection.js#L44

The current algorithm for detecting if a browser is based on Chromium fails in webOS, as the web runtime does not expose window.chrome API. This recent change breaks detection of webOS as Chromium based even if it is based in Chromium m72 or m53.

saghul commented 4 years ago

@jallamsetty1 I think we add the window.chrome check before you came because Edge (the old one) also exposed it so we needed to do more checks. I think we no longer need it so we could probably dump it.

@jdapena Can you share a full UA string from such device? Will it match the next checks?

jdapena commented 4 years ago

This is an example of one based on chromium53:

Mozilla/5.0 (Web0S; Linux/SmartTV) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.34 Safari/537.36 HbbTV/1.4.1 (+DRM+CPAS 2.0.1; LGE; 49UK6470PLC; WEBOS4.0 04.10.45; W4_LM18A; W4_LM18A;)

More recent ones could match patterns like this:

Mozilla/5.0 (Web0S; Linux/SmartTV) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/VERSION Safari/537.36 WebAppManager

(Being VERSION the version of chromium being used). So, for webOS, just matching the Web0S (with zero) string and Chrome should be enough, but that would only fix the problem for webOS and not for other OS that is using Chromium and not exposing window.chrome API.

Also, WebAppManager may show, or not. I.e. browser shows something different, and depending on product I think strings may change even more.

saghul commented 4 years ago

I think that just removing the window.chrome check should do then. Note that we currently don't support M53 because it's too old. The olders Chrome we support is 62 IIRC but we are in the proess of bumping it to 69 or 72.

jdapena commented 4 years ago

That should be enough for new products at least, thanks! I don't think we can ask webpages to hold compatibility for so old chromium forever so at least from my side it is OK.

jdapena commented 4 years ago

Anything else pending to have it deployed in meet.jit.si (sorry, no idea about the actual release process!).

saghul commented 4 years ago

First we need to update this dependency on lib-jitsi-meet and jitsi-meet (@jallamsetty1 can you pease take care of that?). Then it will be part of meet.jit.si on the next update. "When it the next update Saúl?!" I hear you :-) I think we might be pushing out a release this week, otherwise it may take 1 or 2 more weeks, we don't do scheduled releases as such, but release when there is stuff ready :-)

saghul commented 4 years ago

Oh, I forgot something: here is a secret, once all dependencies have been updated alpha.jitsi.net will be automagically updated (it's deployed on every merge) so you should be able to test there very soon.

jallamsetty1 commented 4 years ago

@jdapena, alpha.jitsi.net has been updated with this change. Please feel free it to give it a try, thanks !

jdapena commented 4 years ago

Confirmed our webOS builds are now working with Alpha Jitsi. Once it lands in production meet.jit.si we'll be set with this. Thanks!

jdapena commented 4 years ago

Verified this is now working in webOS after the fix has landed meet.jit.si.

saghul commented 4 years ago

Cheers!