[webvr] Enable WebVR By Default on all platforms

VERIFIED FIXED in mozilla46

Status

()

defect
VERIFIED FIXED
4 years ago
2 years ago

People

(Reporter: kip, Assigned: kip)

Tracking

(Depends on 1 bug, Blocks 1 bug, {dev-doc-complete, devrel-needed})

Trunk
mozilla46
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox46 disabled, relnote-firefox 55+)

Details

(Whiteboard: [webvr])

Attachments

(1 attachment, 2 obsolete attachments)

Assignee

Description

4 years ago
WebVR has been developed behind the dom.vr.enabled preference, but has not yet been enabled by default due to incompatibility with e10s.

Bug 1182048 enables compatibility with e10s.

Once Bug 1182048 lands, we wish to enable WebVR by default.
Assignee

Updated

4 years ago
See Also: → 1215147
Assignee

Comment 1

4 years ago
The patch changes two preferences:

dom.vr.enabled is now "true" by default.

dom.vr.add-test-devices is now "0" by default, so that content may detect VR devices accurately.
Assignee

Comment 2

4 years ago
Bug 1218482 - Enable WebVR By Default
- dom.vr.enabled is now "true" by default.
- dom.vr.add-test-devices is now "0" by default, so that content may detect
  VR devices accurately.
Assignee

Comment 3

4 years ago
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Bug 1218482 - Enable WebVR By Default
- dom.vr.enabled is now "true" by default.
- dom.vr.add-test-devices is now "0" by default, so that content may detect
  VR devices accurately.
Attachment #8679023 - Flags: review?(vladimir)
Release Note Request (optional, but appreciated)
[Why is this notable]: WebVR enabled by default
[Suggested wording]: Use virtual reality devices with the web with the introduction of WebVR
[Links (documentation, blog post, etc)]:
http://mozvr.com/
http://webvr.info/
relnote-firefox: --- → ?
Assignee

Comment 7

4 years ago
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Bug 1218482 - Enable WebVR By Default
- dom.vr.enabled is now "true" by default.
- dom.vr.add-test-devices is now "0" by default, so that content may detect
  VR devices accurately.
Assignee

Comment 8

4 years ago
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Bug 1218482 - Enable WebVR By Default
- dom.vr.enabled is now "true" by default.
- dom.vr.add-test-devices is now "0" by default, so that content may detect
  VR devices accurately.
Attachment #8679023 - Flags: review?(bzbarsky)
Assignee

Comment 9

4 years ago
As this functionality is not intended to land in Firefox 44, we will delay committing until after the Firefox 44 branch date and let it ride the trains to Firefox 45.
Assignee

Comment 10

4 years ago
I have added bzbarsky to the reviewers for the patch, as it now includes an update to test_interfaces.html to confirm that there has been DOM peer review.
Whiteboard: [webvr]
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

https://reviewboard.mozilla.org/r/23343/#review21111

I would really like to see answers to Ehsan's questions in the intent to ship thread before we ship this.
Attachment #8679023 - Flags: review?(bzbarsky)

Comment 13

4 years ago
Hmm, does "all platforms" include Android and b2g?  It seems like those platforms don't need this enabled, so you should probably make the pref change in firefox.js.
Assignee

Comment 14

4 years ago
(In reply to Ehsan Akhgari (don't ask for review please) [Away Nov 3-19] from comment #13)
> Hmm, does "all platforms" include Android and b2g?  It seems like those
> platforms don't need this enabled, so you should probably make the pref
> change in firefox.js.
This does indeed include Android and B2G.  Android can utilize VR through the Google "Cardboard VR" holders.  I have not yet tested this with B2G; however, it should theoretically work as well.  (I'll give it a try and report back)
Assignee

Comment 30

4 years ago
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23343/diff/2-3/
Attachment #8679023 - Flags: review?(bugs)
Assignee

Comment 31

4 years ago
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23343/diff/2-3/
Attachment #8679023 - Flags: review?(bugs) → review?(bzbarsky)
Assignee

Comment 33

4 years ago
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23343/diff/2-3/
Assignee

Comment 34

4 years ago
I have updated the (In reply to :Ehsan Akhgari from comment #13)
> Hmm, does "all platforms" include Android and b2g?  It seems like those
> platforms don't need this enabled, so you should probably make the pref
> change in firefox.js.

I have updated the patches to enable the dom.vr.enable preference and the DOM interfaces in test_interfaces.html to enable WebVR by default for non-release builds only.

Does this look good to you?  Are there any other changes required?
Flags: needinfo?(ehsan)

Comment 35

4 years ago
Yes, this looks good!  But your test_interfaces.html changes look wrong, you shouldn't need to set |android: true|.
Flags: needinfo?(ehsan)
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

https://reviewboard.mozilla.org/r/23343/#review23461

r=me
Attachment #8679023 - Flags: review?(bzbarsky) → review+
Assignee

Comment 37

4 years ago
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23343/diff/3-4/
Attachment #8679023 - Flags: review?(vladimir)
Assignee

Updated

4 years ago
Blocks: 1229464
Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/46da19b55672 for "Assertion failure: mCompositorThreadHolder, at /builds/slave/b2g_m-in_emu-d_dep-00000000000/build/gecko/gfx/vr/ipc/VRManagerParent.cpp:109" in a wide variety of b2g emulator mochitests.
Assignee

Comment 43

3 years ago
Comment on attachment 8679023 [details]
MozReview Request: Bug 1218482 - Remove unnecessary assertion

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23343/diff/3-4/
Attachment #8679023 - Attachment description: MozReview Request: Bug 1218482 - Enable WebVR By Default → MozReview Request: Bug 1218482 - Remove unnecessary assertion
Assignee

Updated

3 years ago
Attachment #8679023 - Flags: review+
Assignee

Comment 45

3 years ago
Running B2G Emulator Mochitests on Try to ensure that this is the extent of the issues.
Assignee

Comment 46

3 years ago
Attachment #8679023 - Attachment is obsolete: true
Assignee

Comment 47

3 years ago
Removed an unnecessary assertion that was erroneously triggering
on shutdown in B2G Mochitests.
Assignee

Updated

3 years ago
Depends on: 1235803
Assignee

Comment 48

3 years ago
Comment on attachment 8703008 [details] [diff] [review]
Bug 1218482 - Remove unnecessary assertion

This patch has been moved to Bug 1235803
Attachment #8703008 - Attachment is obsolete: true
Assignee

Comment 50

3 years ago
As Bug 1235803 and Bug 1235740 have landed, now re-landing this one.

Comment 51

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/591f1064b73c
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
Confirming that WebVR is now enabled by default across platforms (Win 10 x64, Ubuntu 14.04 x86 and Mac OS X 10.9.5), using latest 46 Nightly, build ID: 20160105030211.
Status: RESOLVED → VERIFIED
Kip, is there a blog post or something I can link to for the aurora release notes?
Flags: needinfo?(kgilbert)
Browser compat data now updated on all the WebVR API ref pages:

https://developer.mozilla.org/en-US/docs/Web/API/WebVR_API

Also, I want to confirm that this is noted on the developer release notes:

https://developer.mozilla.org/en-US/Firefox/Releases/46#New_APIs

On a separate note, I've noticed that the MDN ref docs are now out-of-date with the spec; this will be updated once we have spare cycles to do so, probably in Q2.
I have a question here: is the implementation out-of-date with the MDN ref docs? We have not seen requests for changes in our weekly doc request triage meetings.
(In reply to Jean-Yves Perrier [:teoli] from comment #56)
> I have a question here: is the implementation out-of-date with the MDN ref
> docs? We have not seen requests for changes in our weekly doc request triage
> meetings.

This is a good point. I am assuming we'd be up to date with the latest spec, which is not necessarily going to be the case ;-)
Assignee

Comment 58

3 years ago
(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #57)
> (In reply to Jean-Yves Perrier [:teoli] from comment #56)
> > I have a question here: is the implementation out-of-date with the MDN ref
> > docs? We have not seen requests for changes in our weekly doc request triage
> > meetings.
> 
> This is a good point. I am assuming we'd be up to date with the latest spec,
> which is not necessarily going to be the case ;-)

A brief scan through the current MDN docs appears to match the implementation.

It should be noted; however, that the current API that is implemented will be deprecated soon in favor of the compatibility-breaking "WebVR 1.0" api:

https://mozvr.github.io/webvr-spec/

The dust is still settling on "WebVR 1.0" as it has just been released publicly for feedback.  Our implementation of this API is tracked in Bug 1250244.  This is expected to land within one quarter.  Please NI me if you would like more details.
Flags: needinfo?(kgilbert)
(In reply to :kip (Kearwood Gilbert) from comment #58)
> (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #57)
> > (In reply to Jean-Yves Perrier [:teoli] from comment #56)
> > > I have a question here: is the implementation out-of-date with the MDN ref
> > > docs? We have not seen requests for changes in our weekly doc request triage
> > > meetings.
> > 
> > This is a good point. I am assuming we'd be up to date with the latest spec,
> > which is not necessarily going to be the case ;-)
> 
> A brief scan through the current MDN docs appears to match the
> implementation.
> 
> It should be noted; however, that the current API that is implemented will
> be deprecated soon in favor of the compatibility-breaking "WebVR 1.0" api:
> 
> https://mozvr.github.io/webvr-spec/
> 
> The dust is still settling on "WebVR 1.0" as it has just been released
> publicly for feedback.  Our implementation of this API is tracked in Bug
> 1250244.  This is expected to land within one quarter.  Please NI me if you
> would like more details.

Thanks for the info kip — I think we should be ok at this point; I've put WebVR docs update into our publishing pipeline for Q2, and we've got the dev-doc-needed keyword set on the implementation bug, so it will come up in our triage list when the work is complete. We might want to still touch base nearer the time to discuss a schedule for getting the doc updates published.
We should update the release notes as we disabled it because of bug 1239055
dev-doc-needed so that we undo what has been done in comment 55 :-(
Compat information reverted.
Kip: which bug should the writers follow to be notified when WebVR will be re-enabled by default?
Flags: needinfo?(kgilbert)
Assignee

Comment 64

3 years ago
(In reply to Jean-Yves Perrier [:teoli] from comment #63)
> Kip: which bug should the writers follow to be notified when WebVR will be
> re-enabled by default?

FYI - WebVR is still enabled by default in non-release builds.  Once stability is improved, we still still want to wait for changes in the WebVR API to stabilize with WebVR 1.0.

I have added Bug 1256444 to track the dependencies and enabling WebVR in release.
Flags: needinfo?(kgilbert)
Thanks a lot, :kip
Marking this as disabled for 46 since it will not be on by default on release next week.

Updated

3 years ago
Depends on: 1271157
Depends on: 1314012
adding to the 55beta release notes
You need to log in before you can comment on or make changes to this bug.