1412336 - Device orientation in A-Frame demo doesn't work

Status: ASSIGNED

(Core :: DOM: Device Interfaces, defect, P2)

Description

[Andreas Bovens [:abovens]]

Steps to reproduce:
1. Open Nightly on Android and go to https://ada-a-frame.glitch.me
2. Change the device orientation by moving the device.

Actual result:
Orientation events are not registered, unlike in other browsers (e.g. Chrome)

Comment 1

[Hsin-Yi Tsai (she/her) (REO Fx 129) [:hsinyi]]

Hi Dylan, are you the right person to check this issue? Or do you know who I can ask. I am asking you because you fixed a similar bug 1356921. Thank you.

Comment 2

[Dylan Roeh (:droeh) (he/him)]

Yeah, I can look into it. A quick investigation shows that a-frame's device orientation isn't working in general in Nightly, but in Release it works fine provided you use a recent enough version of a-frame (update the version in the link to 0.7.0 and it works, or try the demos on https://aframe.io/examples). 

I'll figure out what's causing the breakage in Nightly.

Comment 3

[Dylan Roeh (:droeh) (he/him)]

Attachment: Don't set dom.vr.enabled to true in Fennec nightly

This is happening because dom.vr.enabled is set to true in Fennec nightly, this patch removes that. Randall, I'm flagging you for review because I'm not sure if there's some good reason why we were doing that in the first place.

Comment 4

[Randall Barker [:rbarker]]

Comment on attachment 8930480 [details] [diff] [review]
Don't set dom.vr.enabled to true in Fennec nightly

Review of attachment 8930480 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/app/mobile.js
@@ -848,4 @@
>  pref("consoleservice.logcat", true);
>  #endif
>  
> -#ifndef RELEASE_OR_BETA

Probably should be:

#if defined(MOZ_ANDROID_GOOGLE_VR)

::: modules/libpref/init/all.js
@@ +5225,4 @@
>  
>  // WebVR is enabled by default in beta and release for Windows and for all
>  // platforms in nightly and aurora.
> +#if defined(XP_WIN) || defined(XP_MACOSX) || !defined(RELEASE_OR_BETA) && !defined(MOZ_WIDGET_ANDROID)

I believe mobile.js will overwrite this so I think this fine. If you enable it in mobile.js when defined(MOZ_ANDROID_GOOGLE_VR).

Comment 5

[Dylan Roeh (:droeh) (he/him)]

Attachment: Don't set dom.vr.enabled to true in Fennec nightly

Thanks, I wasn't aware of MOZ_ANDROID_GOOGLE_VR.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5970c2922ed
Do not set dom.vr.enabled to true in Fennec nightly. r=rbarker

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for failing mochitests dom/tests/mochitest/general/test_interfaces.html and dom/vr/test/mochitest/test_vrDisplay_canvas2d.html (VRDisplay not available):

https://hg.mozilla.org/integration/mozilla-inbound/rev/37e8b667e99722c1c1d2ffc68576e2151eac31b4

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=c5970c2922ed8e6bfb1b173b350977dd34a80680&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=retry

Failure log dom/tests/mochitest/general/test_interfaces.html: https://treeherder.mozilla.org/logviewer.html#?job_id=146801715&repo=mozilla-inbound
dom/tests/mochitest/general/test_interfaces.html | VRDisplay should be defined on the global scope

Failure log dom/vr/test/mochitest/test_vrDisplay_canvas2d.html: https://treeherder.mozilla.org/logviewer.html#?job_id=146801758&repo=mozilla-inbound
dom/vr/test/mochitest/test_vrDisplay_canvas2d.html | Finish running WebVR Canvas2D test. - Finish running WebVR Canvas2D test.: navigator.getVRDisplays is not a function

Comment 8

[Dylan Roeh (:droeh) (he/him)]

OK, apparently we don't want to just disable that pref on nightly.

Randall, I was talking to snorp about this and he mentioned you had a hack that provides orientation events through webvr, which is basically what we'd need to do to have aframe use the device orientation with dom.vr.enabled = true. Do you think that's a reasonable approach and can you put the hack up somewhere if so?

Comment 9

[Randall Barker [:rbarker]]

(In reply to Dylan Roeh (:droeh) from comment #8)
> OK, apparently we don't want to just disable that pref on nightly.
> 
> Randall, I was talking to snorp about this and he mentioned you had a hack
> that provides orientation events through webvr, which is basically what we'd
> need to do to have aframe use the device orientation with dom.vr.enabled =
> true. Do you think that's a reasonable approach and can you put the hack up
> somewhere if so?

Actually, I think the bug is with the WebVR polyfill that AFrame is using. the polyfill seems to be assuming that if the dom has WebVR components then it will be able to get a display which isn't a great assumption. I would recommend either disabling the test and leaving the vr dom off or wait for the WebVR polyfil to be fixed (not likely since it will take months for it to make it to the AFrame fork if the past is any indication).

Comment 10

[Dietrich Ayala (:dietrich)]

In June when the polyfill broke orientation on Firefox Android, it got pushed through polyfill->aframe->release pretty quickly once all the right folks in those projects were all looking at it.

ni? Diego for how we turned it around last time.

Comment 11

[Dylan Roeh (:droeh) (he/him)]

(In reply to Randall Barker [:rbarker] from comment #9)
> (In reply to Dylan Roeh (:droeh) from comment #8)
> > OK, apparently we don't want to just disable that pref on nightly.
> > 
> > Randall, I was talking to snorp about this and he mentioned you had a hack
> > that provides orientation events through webvr, which is basically what we'd
> > need to do to have aframe use the device orientation with dom.vr.enabled =
> > true. Do you think that's a reasonable approach and can you put the hack up
> > somewhere if so?
> 
> Actually, I think the bug is with the WebVR polyfill that AFrame is using.
> the polyfill seems to be assuming that if the dom has WebVR components then
> it will be able to get a display which isn't a great assumption. I would
> recommend either disabling the test and leaving the vr dom off or wait for
> the WebVR polyfil to be fixed (not likely since it will take months for it
> to make it to the AFrame fork if the past is any indication).

Right, this is pretty much what I suggested to snorp, and he didn't seem happy with either approach. I guess we can talk about it on Wednesday.

Comment 12

[James Willcox (:snorp) ([e-mail]) (he/him)]

I don't think I realized that the pose comes from the display. I agree that fixing the polyfill seems like the best way forward.

Comment 13

[Jordan Santell [:jsantell] [@jsantell]]

The polyfill has a flag `ALWAYS_APPEND_POLYFILL_DISPLAY` which always fires up the polyfill even when native WebVR support exists, in the event there are no native VR displays. It looks like A-Frame[0] is not currently using this flag. I believe this will fix the issue, but keep in mind it's not on by default due to always clobbering the native implementation when on mobile, so this is a decision each consumer of the polyfill must make. Please file an issue on the polyfill[1] if this is not the case -- thank you!

[0] https://github.com/aframevr/aframe/blob/ba02747ad0f23f98a58f4590b6ff75267d7cda75/src/index.js#L4-L10
[1] https://github.com/googlevr/webvr-polyfill

Comment 14

[Dylan Roeh (:droeh) (he/him)]

Thanks, Jordan. I'm guessing A-Frame is not going to want to set that flag given that it will clobber a native implementation -- snorp, what are your thoughts here?

Comment 15

[James Willcox (:snorp) ([e-mail]) (he/him)]

I think I discussed with Dylan just disabling WebVR in Firefox since we don't support any actual displays there currently.

Comment 16

[Andreas Bovens [:abovens]]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #15)
> I think I discussed with Dylan just disabling WebVR in Firefox since we
> don't support any actual displays there currently.

But it can be used with a viewer like cardboard, right?

Comment 17

[Jordan Santell [:jsantell] [@jsantell]]

@droeh they could, it clobbers in the sense of globals (`navigator.getVRDisplays`) but still will natively use the frame data and viewer, etc., where VR is enabled and provides a VRDisplay (Daydream in Chrome, GearVR in Samsung Internet).

It sounds like disabling it would work as well -- regardless, the polyfill should handle the no viewer situation.

Hope this helps!

Comment 18

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:cmartin, could you have a look please?
For more information, please visit auto_nag documentation.