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.

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 for issue #670 #673

Merged
merged 1 commit into from
Dec 22, 2019
Merged

Conversation

ongeo-dev
Copy link

It seems not to be necessary to wait for the stream, so it could be shown directly.
Also added some missing semicolons.

What kind of change does this PR introduce?
Fix for 3rd party integration (https://github.com/cordova-rtc/cordova-plugin-iosrtc)

Can it be referenced to an Issue? If so what is the issue # ?
#670

How can we test it?
Start AR.js in Browser or as a cordoja app in iOS aith activated cordova-plugin-iosrtc

Summary
The cordova-plugin-iosrtc is a native replacement of the webrtc function. In iOS its only allowed to use in native safari browser but not in 3rd party apps using WkWebView.
The Plugin does not provide the videoElement.videoWith, that is used to check if the video stream is ready, but in my tests it has not been necessary to wait for it.

Does this PR introduce a breaking change?
I hope not.

Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser
Tested on Cordova in iOS13.2.2 (iPhone 7) and Android 9 (Galaxy S9 - Patch 11/19)

It seems not to be necessary to wait for the stream, so it could be shown directly.
Also added some missing semicolons.
@nicolocarpignoli
Copy link
Collaborator

@ongeo-dev can you provide a simple AR.js app that uses this version, in order to test it easily?

Just run make minify, then use the .min.js created file on the .html example and host it somewhere.

@nicolocarpignoli nicolocarpignoli merged commit 32bf818 into jeromeetienne:dev Dec 22, 2019
nicolocarpignoli added a commit that referenced this pull request Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants