Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DOMException when trying to use XR inside an iframe #23174

Merged
merged 1 commit into from
Feb 7, 2022

Conversation

hybridherbst
Copy link
Contributor

  • Fix DOMException when trying to use XR inside an iFrame
  • show "VR NOT ALLOWED" / "AR NOT ALLOWED" instead
  • align VRButton/ARButton code more closely

Description

Fixes a DOMException that can happen when someone tried to use the VR/AR buttons inside an iFrame, which needs a special iFrame permission xr-spatial-tracking. Instead of throwing, the exception is now logged as warning to the console as well.

ARButton already had that, so this PR adds it to VRButton and amends the messaging for both somewhat (now saying "NOT ALLOWED" instead of "NOT SUPPORTED").

To see the current state with DOMException go to this page and look at the console before Glitch fixes iframe permissions on their end: https://glitch.com/~three-xr

This contribution is funded by 🌵 Needle.

… ALLOWED" / "AR NOT ALLOWED" instead and align VR/AR code
@hybridherbst
Copy link
Contributor Author

I'd like to make a follow-up PR to clean up ARButton and VRButton more / fix issues with some devices but it would be great if this here is merged first - or some feedback :)

Copy link
Collaborator

@Mugen87 Mugen87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the spec calling isSessionSupported() can potentially reject the promise and then execute the rejection handler with a DOMException instead.

It seems the only cause for this exception is the mentioned security error:

If the requesting document’s origin is not allowed to use the "xr-spatial-tracking" permissions policy, reject promise with a "SecurityError" DOMException and return it.

Regarding this definition, the PR is definitely an improvement.

@Mugen87 Mugen87 added this to the r138 milestone Feb 4, 2022
@mrdoob mrdoob merged commit e162ded into mrdoob:dev Feb 7, 2022
@mrdoob
Copy link
Owner

mrdoob commented Feb 7, 2022

Thanks!

donmccurdy pushed a commit to donmccurdy/three.js that referenced this pull request Mar 10, 2022
… ALLOWED" / "AR NOT ALLOWED" instead and align VR/AR code (mrdoob#23174)
@hybridherbst hybridherbst deleted the fix-xr-dom-exception branch April 30, 2022 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants