device-orientation-permisson-ui: Don't emit "granted" event until permissions are actually granted.

[diarmidmackenzie]

Description:

See https://github.com/aframevr/aframe/issues/5078

Changes proposed:

Code is currently structured as:

requestPermission.catch(<create dialog>).then(<mark permissions enabled>)

This is flawed, because when the catch branch is executed, the then branch then gets executed immediately as well.

The fix is to restructure the code as follows:

requestPermission.then(<mark permissions enabled>).catch(<create dialog>)

I've also made a small UT fix to check for the non-emission of events at this stage in the process. This change made the script fail without the fix, and it succeeds with the fix in place.

UTs for this component could benefit from some significant extensions, but I'm not sure how soon I'd find time for that, and I didn't want to hold up making this fix available. So I've only implemented this minimal update to the UT alongside this change.

[dmarcos]

It seems I don't fully understand promises 😄 If the catch function doesn't explicitly return a value the then function shouldn't run, right? It looks you're seeing something different and I don't understand promises.

[diarmidmackenzie]

Here's the documentation I can find on catch()

The Promise returned by catch() is rejected if onRejected throws an error or returns a Promise which is itself rejected; otherwise, it is fulfilled.

I infer from this that if the function provided in catch() returns nothing, catch() itself returns a Promise that is fulfilled, hence the .then() is invoked. That matches what I see in my testing.

Alternatives to the fix I proposed could be:

• throwing an error in the catch() function
• explicitly returning a rejected Promise (e.g. the original Promise)

I've tested these, and both work - specifically:

either adding this line at the end of the catch() function

throw 'need to show permissions dialog';

or (more convoluted, and I think potentially rather confusing, but it does seem to work)....

const promiseRef = DeviceOrientationEvent.requestPermission().catch(function () {

and then at the end of the catch() function:

return promiseRef

[dmarcos]

Thanks for the explanation!