1580567 - Implement XR device access permission UI

Status: RESOLVED FIXED

(Core :: WebVR, task)

Comment 1

[:kip (Kearwood Gilbert)]

Attachment: Bug 1580567 - Implement XR device access permission UI

Comment 2

[:kip (Kearwood Gilbert)]

Patch is WIP. Notably, need to avoid creating the VR presentation when the promise is not resolved.

Comment 3

[:kip (Kearwood Gilbert)]

Try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3cd4d5896b4f6e9a698c84fb1a943d5dd4eccd6

Comment 4

[:kip (Kearwood Gilbert)]

Updated try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=72f60dee4deeaf155a7a2a8bbed26c5ed5d29be2

Comment 5

[:kip (Kearwood Gilbert)]

Updated try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=30ebaecdddd9f206a6715ae970c1347f13f529bf

Comment 6

[:kip (Kearwood Gilbert)]

Fixed WPT, changed WebVR interfaces to [SecureContext]

Updated try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d6e2ee1a8ffc716e991e2782fda9df9bd47f74f9

Comment 7

[:kip (Kearwood Gilbert)]

Fixed dom/tests/mochitest/general/test_interfaces.html, to reflect that the WebVR interfaces are now [SecureContext]. (This will require DOM peer review).

Updated try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=375bfa4ea8d24150b0f8905c0a8f1fb94dd2caef

Comment 8

[:kip (Kearwood Gilbert)]

Prevented redundant popups and spamming of permission prompt.

Updated try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f68e0363bd1de53419c7c3351cf20d7a3ece2435

Comment 9

[:kip (Kearwood Gilbert)]

Added screenshots from this patch for reference to Bug 1579281.

Comment 10

[:kip (Kearwood Gilbert)]

Fixes for review feedback and merged in status icons from Bug 1579276.

Updated try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9bc8b42959042d4e76db102ea72b81027ba15f6c

Comment 11

[:kip (Kearwood Gilbert)]

Fixed issue with removing Virtual Reality permissions from an iframe that had previously been set to remember.

Updated try push (T-shaped), for sanity check:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c4fd46abcd9f1893eae042f16fa0b2a2bca5b293

IIUC, this fix should address the last of the review feedback.

Comment 12

[:kip (Kearwood Gilbert)]

With Daosheng's fix to follow in Bug 1599927, do you have any additional review feedback on the patch?

Comment 13

[Imanol Fernandez]

I tried both patches together and it works well, r+

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15f8fa34d2f4
Implement XR device access permission UI r=fluent-reviewers,bzbarsky,pbz,daoshengmu,imanol

Comment 15

[Razvan Maries]

Backed out for perma fails on browser_temporary_permissions_expiry.js and browser_privatebrowsing_rememberprompt.js.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=279659624&resultStatus=testfailed%2Cbusted%2Cexception&revision=15f8fa34d2f49562743d050164d939a66a0a34ab

Logs:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=279659626&repo=autoland&lineNumber=6229
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=279659624&repo=autoland&lineNumber=6654

Backout: https://hg.mozilla.org/integration/autoland/rev/b4195b31ca936f7e2541c8f4d04aa9744d98443c

Comment 16

[Razvan Maries]

Also caused:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=279664085&repo=autoland&lineNumber=3993
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=279672193&repo=autoland&lineNumber=3805

Comment 17

[:kip (Kearwood Gilbert)]

I'll update the patch with fixes for the test failures and attempt to re-land.

The test-interfaces.html test possibly failed due to the r+ from a dom peer (bz) not being re-applied after the patch was rebased...

Comment 18

[:kip (Kearwood Gilbert)]

Tests were failing due to changes related to permission requests from iframes since the patch was authored.

I have un-bitrotted the patch and tested locally.

Updated try push:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=71fa78845f15cb4f863acb57489b5a7af1e9a733

Comment 19

[:kip (Kearwood Gilbert)]

I notice from the last try run, the patch is also causing:

TEST-UNEXPECTED-ERROR | /feature-policy/reporting/vr-reporting.https.html | unable to find test window

I'll investigate and update the patch.

Comment 20

[:kip (Kearwood Gilbert)]

(In reply to :kip (Kearwood Gilbert) from comment #19)

I notice from the last try run, the patch is also causing:

TEST-UNEXPECTED-ERROR | /feature-policy/reporting/vr-reporting.https.html | unable to find test window

I'll investigate and update the patch.

Note that this is on the android platform only. We don't expect VR to be enabled on android, except in the geckoview-based Firefox Reality browser.

Comment 21

[:kip (Kearwood Gilbert)]

(In reply to :kip (Kearwood Gilbert) from comment #20)

(In reply to :kip (Kearwood Gilbert) from comment #19)

I notice from the last try run, the patch is also causing:

TEST-UNEXPECTED-ERROR | /feature-policy/reporting/vr-reporting.https.html | unable to find test window

I'll investigate and update the patch.

Note that this is on the android platform only. We don't expect VR to be enabled on android, except in the geckoview-based Firefox Reality browser.

I've repro'ed locally and confirm that the follow-up, bug 1599927, corrects this WPT failure.

Comment 22

[:kip (Kearwood Gilbert)]

If I can get the test-interfaces.html re-reviewed, and otherwise get a r+ from @bz, I can attempt to re-land this in a lando stack together with Bug 1599927.

Comment 23

[Daosheng Mu[:daoshengmu]]

(In reply to :kip (Kearwood Gilbert) from comment #20)

(In reply to :kip (Kearwood Gilbert) from comment #19)

I notice from the last try run, the patch is also causing:

TEST-UNEXPECTED-ERROR | /feature-policy/reporting/vr-reporting.https.html | unable to find test window

I'll investigate and update the patch.

Note that this is on the android platform only. We don't expect VR to be enabled on android, except in the geckoview-based Firefox Reality browser.

Is it resolved after applying the patch from Bug 1599927?

Comment 24

[:kip (Kearwood Gilbert)]

(In reply to Daosheng Mu[:daoshengmu] from comment #23)

(In reply to :kip (Kearwood Gilbert) from comment #20)

(In reply to :kip (Kearwood Gilbert) from comment #19)

I notice from the last try run, the patch is also causing:

TEST-UNEXPECTED-ERROR | /feature-policy/reporting/vr-reporting.https.html | unable to find test window

I'll investigate and update the patch.

Note that this is on the android platform only. We don't expect VR to be enabled on android, except in the geckoview-based Firefox Reality browser.

Is it resolved after applying the patch from Bug 1599927?

Indeed, it is! I'll land these two patches together.

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29809135f18e
Implement XR device access permission UI r=fluent-reviewers,bzbarsky,pbz,daoshengmu,imanol

Comment 26

[Stefan Hindli [:stefan_hindli]]

Backed out 2 changesets (Bug 1580567, Bug 1599927) for causing permafailures in dom/tests/mochitest/general/test_interfaces.html CLOSED TREE
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&fromchange=4f54a53cd1dae622d1693861e1ae6de26043a012&selectedJob=280599662
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280599662&repo=autoland&lineNumber=3988

Comment 27

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe86796921dd
Backed out 2 changesets (bug 1580567, bug 1599927) for causing permafailures in dom/tests/mochitest/general/test_interfaces.html CLOSED TREE

Comment 28

[:kip (Kearwood Gilbert)]

Looks like this needed to be re-reviewed by BZ (dom peer) to pass the test_interfaces.html after the rebasing / updating. I see there is a new r+ now, will re-attempt now in hopes it doesn’t get stomped again by another patch in auto land.

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/522c8d183bd5
Implement XR device access permission UI r=fluent-reviewers,bzbarsky,pbz,daoshengmu,imanol

Comment 30

[:kip (Kearwood Gilbert)]

Ugh.. I see now the last r+ from @bz was prior to the most recent rebase... sorry for causing another back out. Would you mind re-reviewing the patch, @bzbarsky ?

Comment 31

[Cristian Brindusan [:cbrindusan]]

Backed out 2 changesets (Bug 1580567, Bug 1599927) for mochitest failures at test_interfaces.html.
https://hg.mozilla.org/integration/autoland/rev/5a9195515b054a11b8844f918c1a18011d948245

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=0e0d88942d33f80fa42ab54ca11fdb96877aebc5&selectedJob=280610653

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=280610653&repo=autoland&lineNumber=3988

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cce8edb56b19
Implement XR device access permission UI r=fluent-reviewers,bzbarsky,pbz,daoshengmu,imanol

Comment 33

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/cce8edb56b19

Comment 34

[:kip (Kearwood Gilbert)]

Clearing NI related to earlier backout. This has now landed successfully.

Comment 35

[:gerard-majax]

Hello,

On recent nightlies on linux (but I guess everywhere), each and ever web page that integrates a video from Vimeo provider does trigger the display of this notification. For a simple video, that feels weird. I understand that currently, anyway, the implementation is not ready on Linux, but I'm wondering how to check if it's legitimate that for a trivial video the vimeo integration triggers such VR permission.

Comment 36

[:gerard-majax]

FTR, even on their website. One example: https://vimeo.com/304198961

Comment 37

[:kip (Kearwood Gilbert)]

(In reply to Alexandre LISSY :gerard-majax from comment #35)

Hello,

On recent nightlies on linux (but I guess everywhere), each and ever web page that integrates a video from Vimeo provider does trigger the display of this notification. For a simple video, that feels weird. I understand that currently, anyway, the implementation is not ready on Linux, but I'm wondering how to check if it's legitimate that for a trivial video the vimeo integration triggers such VR permission.

Hello,

Thanks for the feedback!

Many sites are using libraries that access the WebVR API during their initialization, even when the VR functions are not in use. The prompt is displayed before performing enumeration of VR hardware due to the enumeration itself having some side effects such as causing software to start up in the background or devices to power on.

This results in the prompt being displayed in cases where it might not be relevant.

I've filed a couple of follow-up bugs, which I'm working on now to help the situation:

Bug 1603825 - Suppress the VR permission UI when no VR runtimes are detected
Bug 1603631 - Consider hiding "Virtual Reality" permission options for Linux until WebXR is fully enabled on Linux

I hope this helps!

Comment 38

[:kip (Kearwood Gilbert)]

(In reply to :kip (Kearwood Gilbert) from comment #37)

(In reply to Alexandre LISSY :gerard-majax from comment #35)

Hello,

On recent nightlies on linux (but I guess everywhere), each and ever web page that integrates a video from Vimeo provider does trigger the display of this notification. For a simple video, that feels weird. I understand that currently, anyway, the implementation is not ready on Linux, but I'm wondering how to check if it's legitimate that for a trivial video the vimeo integration triggers such VR permission.

Hello,

Thanks for the feedback!

Many sites are using libraries that access the WebVR API during their initialization, even when the VR functions are not in use. The prompt is displayed before performing enumeration of VR hardware due to the enumeration itself having some side effects such as causing software to start up in the background or devices to power on.

This results in the prompt being displayed in cases where it might not be relevant.

I've filed a couple of follow-up bugs, which I'm working on now to help the situation:

Bug 1603825 - Suppress the VR permission UI when no VR runtimes are detected
Bug 1603631 - Consider hiding "Virtual Reality" permission options for Linux until WebXR is fully enabled on Linux

I hope this helps!

Until these updates land, note that you can disable the permission prompts yourself. In the "Privacy and Security" section in the browser options screen, there is a new "Virtual Reality" permissions section. In there, you can activate the "Block new requests asking to access your virtual reality devices"

Comment 39

[:gerard-majax]

(In reply to :kip (Kearwood Gilbert) from comment #38)

[...]

Bug 1603825 - Suppress the VR permission UI when no VR runtimes are detected

So basically once this once is fixed, this would not bother people with no VR hardware. And those with VR hardware would, by just iterating, expose some informations, so the prompt makes sense there, even if it's just probing ?