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.

Sign up for GitHub

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

Closed
wants to merge 7 commits into from
Closed

Gracefully handles when calling getGamepads() is not allowed... #5581

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
f7338f3
Gracefully handles when Document's Permission Policy does not allow c…
DougReeder Sep 28, 2024
07650ee
Improves warning message
DougReeder Sep 29, 2024
7a6a13d
tracked-controllers-webvr: separate error messages for permission and…
DougReeder Sep 30, 2024
9eaeb0f
tracked-controls: specific warning messages when gamepad access is bl…
DougReeder Sep 30, 2024
ab0065f
tracked-controls: updates wording of gamepad error messages
DougReeder Oct 1, 2024
6a6bb02
tracked-controls: only shows permissions warning once
DougReeder Oct 13, 2024
a3f8df8
tracked-controllers-webvr: when not in vr-mode, doesn't call getGamep…
DougReeder Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 28 additions & 16 deletions src/systems/tracked-controls-webvr.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,38 @@ module.exports.System = registerSystem('tracked-controls-webvr', {
* Update controller list.
*/
updateControllerList: function () {
var controllers = this.controllers;
var gamepad;
var gamepads;
var i;
var prevCount;
try {
var controllers = this.controllers;
var gamepad;
var gamepads;
var i;
var prevCount;

gamepads = navigator.getGamepads && navigator.getGamepads();
if (!gamepads) { return; }
gamepads = navigator.getGamepads && navigator.getGamepads();
if (!gamepads) { return; }

prevCount = controllers.length;
controllers.length = 0;
for (i = 0; i < gamepads.length; ++i) {
gamepad = gamepads[i];
if (gamepad && gamepad.pose) {
controllers.push(gamepad);
prevCount = controllers.length;
controllers.length = 0;
for (i = 0; i < gamepads.length; ++i) {
gamepad = gamepads[i];
if (gamepad && gamepad.pose) {
controllers.push(gamepad);
}
}
}

if (controllers.length !== prevCount) {
this.el.emit('controllersupdated', undefined, false);
if (controllers.length !== prevCount) {
this.el.emit('controllersupdated', undefined, false);
}
} catch (e) {
if (e.name === 'SecurityError') {
Copy link
Member

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?

https://stackoverflow.com/questions/326069/how-to-identify-if-a-webpage-is-being-loaded-inside-an-iframe-or-directly-into-t

Copy link
Contributor Author

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);
Copy link
Member

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

Copy link
Contributor Author

@DougReeder DougReeder Oct 1, 2024 •
edited
Loading

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).

} else {
console.warn('A-Frame requires additional permissions to list the gamepads. The iframe `allow` attribute must not block this origin.', e);
Copy link
Member

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?

Copy link
Contributor Author

@DougReeder DougReeder Oct 1, 2024 •
edited
Loading

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 *"

}
} else {
console.error('Can\'t update controller list:', e);
Copy link
Member

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?

Copy link
Contributor Author

@DougReeder DougReeder Oct 1, 2024 •
edited
Loading

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.

}
}
}
});
Loading