-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Gracefully handles when calling getGamepads() is not allowed... #5581
Conversation
…alling getGamepads() from the context (such as an IFRAME).
warn message is not super useful. Can we just detect we're in an iframe environment and show a more descriptive message? |
If the IFRAME is something like What error message would you consider useful? |
Not sure. Something that helps the dev understand what's going on and how to fix it. e.g:
Message in PR not super useful |
left a comment in PR |
@@ -64,7 +64,11 @@ module.exports.System = registerSystem('tracked-controls-webvr', { | |||
this.el.emit('controllersupdated', undefined, false); | |||
} | |||
} catch (e) { | |||
console.warn('A-Frame requires additional permissions to list the gamepads. If this is running in an IFRAME, you need to add `gamepads` to the `allow` attribute. If this is running as the top-level page, the HTTP `Permissions-Policy` header must not exclude this origin in the `gamepad` directive.', e); | |||
if (e.name === 'SecurityError') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this specific enough? We shouldn't check if we are in an iframe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No other code inside the try-catch can throw a SecurityError.
if (window.self === window.top) { | ||
console.warn('A-Frame requires additional permissions to list the gamepads. The HTTP `Permissions-Policy` header must not exclude this origin in the `gamepad` directive.', e); | ||
} else { | ||
console.warn('A-Frame requires additional permissions to list the gamepads. The iframe `allow` attribute must not block this origin.', e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Message needs rewording. A-Frame doesn't require additional permissions but the iframe permissions policy. Is not the solution to add gamepad to the allow attribute e.g allow="gamepad"
? allow doesn't bock origins afaik?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allow="gamepad" is equivalent to allow="gamepad 'self'" which is fine if the A-Frame app is at the same origin as the top-level page. If the A-Frame app is on a different origin (for example, if the aframe app is listed on itch.io but the app is served from elsewhere), you need allow="gamepad aframe-app-origin.com" or allow="gamepad *"
} catch (e) { | ||
if (e.name === 'SecurityError') { | ||
if (window.self === window.top) { | ||
console.warn('A-Frame requires additional permissions to list the gamepads. The HTTP `Permissions-Policy` header must not exclude this origin in the `gamepad` directive.', e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for this message. How common is this issue? I think I've never seen a server excluding the gamepad API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't seen this in the wild, but it would happen if an aframe app was deployed from an server with a very restrictive policy (as set in the Apache or Nginx configuration, which the a-frame app can't control).
console.warn('A-Frame requires additional permissions to list the gamepads. The iframe `allow` attribute must not block this origin.', e); | ||
} | ||
} else { | ||
console.error('Can\'t update controller list:', e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this error and above just warnings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the A-frame app doesn't need the non-webxr gamepads on this device, failing to get the gamepads will not keep the app from working. If the a-frame app does need gamepads on this device, not getting the gamepads is a problem that has to be fixed. Since this code can't determine which, a warning is reasonable. (However, if the error isn't caught at all, A-Frame doesn't fully initialize.)
"Can't update controller list" would only occur if there was some unforeseen error and would likely be a programming error in A-Frame.
if (window.self === window.top) { | ||
console.warn('The HTTP `Permissions-Policy` header must not block this origin in the `gamepad` directive, to allow A-Frame to list the gamepads.', e); | ||
} else { | ||
console.warn('The iframe `allow` attribute must not block the origin of this A-Frame app, to allow A-Frame to list the gamepads.', e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This message is still not clear. I wouldn't know what to do and I'm well versed in web dev. What about?
It looks like your A-Frame experience is running inside an iframe. In order to access the gamepads API you need to add allow="gamepad" to your iframe attributes
} catch (e) { | ||
if (e.name === 'SecurityError') { | ||
if (window.self === window.top) { | ||
console.warn('The HTTP `Permissions-Policy` header must not block this origin in the `gamepad` directive, to allow A-Frame to list the gamepads.', e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can remove this. Seems pretty rare. Can add later if necessary
What is the general context of this error? Are you still using the webvr code path? |
See this example of the current code failing to load properly
|
Two thoughts here
It should get rid of the error message
|
Adding that check does allow A-Frame to start up, but as soon as the user enters vr-mode, the Uncaught SecurityError prevents tick() from running normally and rendering doesn't happen, unless we catch the SecurityError. |
I merged a PR to remove WebVR support. This should no longer be an issue. Can you give master builds a try and let me know? Thanks! |
With that version of A-Frame, I do not observe the issue, either on startup or in the tick() handler. |
Thanks for the PR, the back and forth and verifying that master is now working. Now that we have deprecated WebVR we no longer need this PR |
… from the context (such as an IFRAME), by the Document's Permission Policy, by catching the DOMException.
When AFrame is run in an IFRAME without the needed permission, the call to Navigator.getGamepads() throws a DOMException which prevents AFrame from loading properly.