Fix regex checking for cardboard support #358

InfiniteLee · 2018-05-02T22:49:43Z

fix the regex so that ios cardboard is properly detected. Fixes #352

Despite the name of the branch, iOS is explicitly disabled until we fix rendering on carboard iOS. Same for Android/Firefox.

gfodor · 2018-05-10T18:43:39Z

What's the case here this misses? As it is right now I think \W* on both ends basically serves no purpose since it will match anything and could be removed. I'm guessing there is an example where it's a substring?

InfiniteLee · 2018-05-10T19:13:04Z

@gfodor I believe it has to do with the fact that "Cardboard" is the first word that shows up in the navigator.getVRDisplays() name ("Cardboard VRDisplay") on iOS. So \W alone wont match anything.

"Cardboard VRDisplay".match(/\Wcardboard\W/i)
> null
"Cardboard VRDisplay".match(/\W*cardboard\W*/i)
> Array [ "Cardboard " ]

vs on Android:

"Google, Inc. Cardboard v1".match(/\Wcardboard\W/i)
> Array [ "Cardboard " ]

gfodor · 2018-05-10T22:07:30Z

Ah I understand -- I believe I intended these to be \b's (word boundaries) not \W's -- should match begin/end of line as well as whitespaces

InfiniteLee · 2018-05-10T22:49:14Z

I'll try that.

@brianpeiris and I also just discovered this same bug is preventing us from launching cardboard on firefox android.

InfiniteLee · 2018-05-10T22:52:53Z

\b wont match if cardboard appears in the middle of another string e.g. "GoogleCardboard". I don't know if that will be important or not.

gfodor · 2018-05-10T22:57:41Z

i guess its not the end of the world if we just match the substring. sgtm.

gfodor · 2018-05-10T22:58:07Z

i'd just drop the \w*'s now

cvan · 2018-05-10T23:14:04Z

feel free to do d.displayName.toLowerCase().includes("cardboard")

btw, displayName is deprecated in WebVR v1.1 - though it is still implemented in the WebVR v1.1 browsers and the WebVR Polyfill. Apple will not be implementing it in Safari.

so likely you will want to check d.capabilities.hasExternalDisplay === false instead to future-proof this.

cvan · 2018-05-03T02:26:06Z

src/utils/vr-caps-detect.js

@@ -70,12 +70,12 @@ export async function getAvailableVREntryTypes() {
      ? VR_DEVICE_AVAILABILITY.yes
      : VR_DEVICE_AVAILABILITY.no;

-    cardboard = displays.find(d => d.capabilities.canPresent && d.displayName.match(/\Wcardboard\W/i))
+    cardboard = displays.find(d => d.capabilities.canPresent && d.displayName.match(/\W*cardboard\W*/i))


when the WebVR Polyfill is used, you can check d.isPolyfilled

I'm guessing we need to re-write this all imminently once WebXR lands in origin trial in chrome, so probably not worth worrying about for now

brianpeiris · 2018-05-11T03:01:41Z

src/utils/vr-caps-detect.js

@@ -43,6 +43,8 @@ export async function getAvailableVREntryTypes() {
  const isSamsungBrowser = browser.name === "chrome" && navigator.userAgent.match(/SamsungBrowser/);
  const isOculusBrowser = navigator.userAgent.match(/Oculus/);
  const isDaydreamCapableBrowser = !!(isWebVRCapableBrowser && browser.name === "chrome" && !isSamsungBrowser);
+  const isIPhone = navigator.userAgent.match(/iPhone/);


Would we want to guard against other iDevices? See this regex @robertlong constructed previously: https://github.com/mozilla/hubs/blob/5a8c9635aa1414e5dcabf65d9709b880ce1a6cba/src/utils/ios-audio-context-fix.js#L5

👍 I don't own an iPad, but I'd assume the bug exists there as well?

we already have https://www.npmjs.com/package/mobile-detect in our packages, perhaps that's a good way to check this? or https://www.npmjs.com/package/device-detect

cvan

good work! the WebVR and headset landscape makes this quite tricky to chase down

cvan · 2018-05-11T01:49:48Z

src/utils/vr-caps-detect.js


    // For daydream detection, in a WebVR browser we can increase confidence in daydream compatibility.
-    const hasDaydreamWebVRDevice = displays.find(d => d.displayName.match(/\Wdaydream\W/i));
+    const hasDaydreamWebVRDevice = displays.find(d => d.displayName.match(/daydream/i));


since you're checking d.capabilities.canPresent above, do you want to do it here as well?

Is that necessary? I suspect that a daydream display wouldn't show up at all if that were the case.

cvan · 2018-05-11T01:52:05Z

src/utils/vr-caps-detect.js

@@ -70,12 +71,13 @@ export async function getAvailableVREntryTypes() {
      ? VR_DEVICE_AVAILABILITY.yes


btw, it can likely be discussed in a separate issue, but should GENERIC_ENTRY_TYPE_DEVICE_BLACKLIST instead be checking d.capabilities.hasExternalDisplay === false?

cvan · 2018-05-11T05:58:06Z

src/utils/vr-caps-detect.js

@@ -43,6 +43,8 @@ export async function getAvailableVREntryTypes() {
  const isSamsungBrowser = browser.name === "chrome" && navigator.userAgent.match(/SamsungBrowser/);
  const isOculusBrowser = navigator.userAgent.match(/Oculus/);
  const isDaydreamCapableBrowser = !!(isWebVRCapableBrowser && browser.name === "chrome" && !isSamsungBrowser);
+  const isIPhone = navigator.userAgent.match(/iPhone/);


👍 I don't own an iPad, but I'd assume the bug exists there as well?

# Conflicts: # src/utils/vr-caps-detect.js

… readable

fix the regex so that ios cardboard is properly detected.

8b4da5f

InfiniteLee mentioned this pull request May 2, 2018

Cardboard VR detection incorrect #333

Closed

Merge branch 'master' into fix/ios_cardboard_support

74b9a92

gfodor self-requested a review May 10, 2018 18:44

Merge branch 'master' into fix/ios_cardboard_support

c0da8fa

cvan reviewed May 10, 2018

View reviewed changes

InfiniteLee added 2 commits May 10, 2018 18:12

Merge branch 'master' into fix/ios_cardboard_support

b306aa8

explicitly prevent iPhones from using cardboard for now

46b3c20

InfiniteLee changed the title ~~WIP - enable cardboard on iOS~~ Fix regex checking for cardboard support May 11, 2018

InfiniteLee added 3 commits May 10, 2018 18:38

just use /cardboard/ and /daydream/ instead of making it complicated

84865cc

block firefox on android as well

9c0fc30

oops

9b779be

brianpeiris reviewed May 11, 2018

View reviewed changes

cvan suggested changes May 11, 2018

View reviewed changes

Adding device-detect to check against all "iDevices".

3d43b0a

brianpeiris mentioned this pull request Jun 1, 2018

Gaze cursor does not work in Cardboard mode #334

Closed

InfiniteLee added 2 commits June 1, 2018 14:42

Merge branch 'master' into fix/ios_cardboard_support

94b3831

# Conflicts: # src/utils/vr-caps-detect.js

ci seems to think this is not passing linter, so making it a bit more…

1ded034

… readable

InfiniteLee merged commit 4a6a722 into master Jun 1, 2018

brianpeiris deleted the fix/ios_cardboard_support branch September 24, 2018 17:30

This was referenced Nov 4, 2022

[Snyk] Fix for 1 vulnerabilities devcode1981/hubs#21

Open

[Snyk] Fix for 1 vulnerabilities MarcelRaschke/hubs#26

Open

This was referenced Nov 5, 2022

[Snyk] Fix for 1 vulnerabilities MarcelRaschke/hubs#27

Open

[Snyk] Fix for 1 vulnerabilities meonBot/hubs#8

Open

[Snyk] Fix for 1 vulnerabilities devcode1981/hubs#22

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix regex checking for cardboard support #358

Fix regex checking for cardboard support #358

InfiniteLee commented May 2, 2018 •

edited

Loading

gfodor commented May 10, 2018

InfiniteLee commented May 10, 2018

gfodor commented May 10, 2018

InfiniteLee commented May 10, 2018

InfiniteLee commented May 10, 2018

gfodor commented May 10, 2018

gfodor commented May 10, 2018

cvan commented May 10, 2018 •

edited

Loading

cvan May 3, 2018 •

edited

Loading

gfodor May 11, 2018

brianpeiris May 11, 2018

cvan May 11, 2018

gfodor May 11, 2018

cvan left a comment

cvan May 11, 2018

InfiniteLee May 11, 2018

cvan May 11, 2018

cvan May 11, 2018

		@@ -70,12 +71,13 @@ export async function getAvailableVREntryTypes() {
		? VR_DEVICE_AVAILABILITY.yes

Fix regex checking for cardboard support #358

Fix regex checking for cardboard support #358

Conversation

InfiniteLee commented May 2, 2018 • edited Loading

gfodor commented May 10, 2018

InfiniteLee commented May 10, 2018

gfodor commented May 10, 2018

InfiniteLee commented May 10, 2018

InfiniteLee commented May 10, 2018

gfodor commented May 10, 2018

gfodor commented May 10, 2018

cvan commented May 10, 2018 • edited Loading

cvan May 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cvan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

InfiniteLee commented May 2, 2018 •

edited

Loading

cvan commented May 10, 2018 •

edited

Loading

cvan May 3, 2018 •

edited

Loading