-
Notifications
You must be signed in to change notification settings - Fork 1k
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(core): do not add window.cordova on web apps #4214
Conversation
Glad I'm not crazy. I thought I was losing my mind for a while, trying to track down what I'd done that had caused this to start happening. Thanks for the fix! |
@lincolnthree Thank you for reporting it! |
Hey @dwieeb, did this make it into the beta.3 build? Tried it out and it looks like the compiled sources do not include the fix:
|
Actually hang on. I think there's some weird caching going on here. I see the updated dist/ in node_modules, but the |
Okay, confirmed! This is working :) Was a weird NPM multi-version precedence thing... Thanks again! |
I wrapped the logic in the legacy handlers to only add the empty Cordova object and the native event handlers to only be executed when it is running on a device. This fixes the issue with Ionic Core wrongly identifying web apps as being identified as Cordova and hybrid apps as brought up in issue #4198.