immersive-web / webxr-polyfill

Use the WebXR Device API today, providing fallbacks to native WebVR 1.1 and Cardboard
Apache License 2.0
378 stars 84 forks source link

Event subtypes (XRSessionEvent) not working properly on Safari #130

Open blairmacintyre opened 4 years ago

blairmacintyre commented 4 years ago

On safari (mobile -- desktop doesn't "see" webxr so can't test), the inline-session example (at least) fails to "Exit VR" properly.

The issue seems to be with the way the XRSessionEvent is defined, created, and/or dispatched.

The event handler is received here:

      function onSessionEnded(event) {
        // Only reset the button when the immersive session ends.
        if (event.session.isImmersive) {
          xrButton.setSession(null);
        }
      }

On desktop Firefox (for example), things work properly: event is an XRSessionEvent and event.session.isImmersive exists.

On Safari, the event is showing up as an Event type, and the prototype does not have the session getter defined, so event.session fails.

I do not understand how to fix this, because the code appears correct ... could this be a bug in Safari's javascript? (I am trying to use this code as part of an update to the WebXR Viewer on iOS, but cannot see how to get this to work).

blairmacintyre commented 4 years ago

Found this on SO, seems to confirm it's an issue with extending native classes on Safari https://stackoverflow.com/questions/58471434/problem-extending-native-es6-classes-in-safari

jsantell commented 4 years ago

Found this on SO, seems to confirm it's an issue with extending native classes on Safari https://stackoverflow.com/questions/58471434/problem-extending-native-es6-classes-in-safari

Most likely it's this. I've made an EventTarget polyfill in the past specifically for not being able to extend EventTarget in Safari

blairmacintyre commented 4 years ago

I implemented the suggested solution in the three places needed here, and it works fine. Should we just include that here? I'm happy to submit a PR.

jsantell commented 4 years ago

I think there'll be some differences between the polyfilled EventTarget and the real one (some properties aren't modifiable on a synthesized event); I wonder if it should only be polyfilled on iOS (the linked polyfill doesn't handle injection). Either way, SGTM!

blairmacintyre commented 4 years ago

So you’re already using your EventTarget polyfill in this; the issue I was running into is with Event.

I don’t think the suggested change has any downside since it’s just setting the prototype to the class itself, right? The bug is that this wasn’t done for some reason.


Blair MacIntyre Principal Research Scientist

snt frm iOS kybrd, pls pardon typos and weird auto-corrections

blairmacintyre.me && pronoun.is/he/him


From: Jordan Santell notifications@github.com Sent: Wednesday, January 1, 2020 5:47 PM To: immersive-web/webxr-polyfill Cc: Blair MacIntyre; Author Subject: Re: [immersive-web/webxr-polyfill] Event subtypes (XRSessionEvent) not working properly on Safari (#130)

I think there'll be some differences between the polyfilled EventTarget and the real one (some properties aren't modifiable on a synthesized event); I wonder if it should only be polyfilled on iOS (the linked polyfill doesn't handle injection). Either way, SGTM!

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/immersive-web/webxr-polyfill/issues/130?email_source=notifications&email_token=AAKJJ7MF2HVDDV45YUALOOTQ3UMRFA5CNFSM4KACNKUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH5ODZY#issuecomment-570089959, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAKJJ7PFHOSYHU7PF45KOFDQ3UMRFANCNFSM4KACNKUA.

blairmacintyre commented 4 years ago

Here's the code to implement this fix. https://github.com/immersive-web/webxr-polyfill/pull/133