You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by stalniy <gi...@git.apache.org> on 2017/06/05 11:02:52 UTC
[GitHub] cordova-plugin-media-capture pull request #79: CB-12882: (ios): adds support...
GitHub user stalniy opened a pull request:
https://github.com/apache/cordova-plugin-media-capture/pull/79
CB-12882: (ios): adds support for permissions checks for camera capturing
### Platforms affected
iOS
### What does this PR do?
adds support for checking camera permission and if camera was prohibited shows alert message with information and 2 buttons: OK and Settings. The same `cordova-plugin-camera` does
### What testing has been done on this change?
I ran all automated tests, they passed. Also did manual testing and added 2 additional cases in manual tests for checking presence of alert when camera access was prohibited
### Checklist
- [x] [Reported an issue](http://cordova.apache.org/contribute/issues.html) in the JIRA database
- [x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
- [x] Added automated test coverage as appropriate for this change.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/stalniy/cordova-plugin-media-capture fix/permissions-request
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-plugin-media-capture/pull/79.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #79
----
commit 13a5135e1defee4afd0fd38cd3c34d1dcec87580
Author: Sergii Stotskyi <se...@concreteplatform.com>
Date: 2017-06-05T10:02:29Z
CB-12882: (ios): adds support for permissions checks for captureVideo and captureImage methods
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture pull request #79: CB-12882: (ios): adds support...
Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-media-capture/pull/79#discussion_r120496658
--- Diff: src/ios/CDVCapture.m ---
@@ -292,6 +295,65 @@ - (CDVPluginResult*)processVideo:(NSString*)moviePath forCallbackId:(NSString*)c
return [CDVPluginResult resultWithStatus:CDVCommandStatus_OK messageAsArray:fileArray];
}
+- (void)showAlertIfAccessProhibeted
+{
--- End diff --
For styling consistency, can we keep the opening brace of a function on the same line as the function?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture issue #79: CB-12882: (ios): adds support for pe...
Posted by stalniy <gi...@git.apache.org>.
Github user stalniy commented on the issue:
https://github.com/apache/cordova-plugin-media-capture/pull/79
@filmaj could you please merge this if everything looks good?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture issue #79: CB-12882: (ios): adds support for pe...
Posted by cordova-qa <gi...@git.apache.org>.
Github user cordova-qa commented on the issue:
https://github.com/apache/cordova-plugin-media-capture/pull/79
Cordova CI Build has completed successfully.
**Commit** - [Link](https://github.com/apache/cordova-plugin-media-capture/pull/79/commits/d19be5be1fa5dfbc30fbd997f5e780a056616138)
**Dashboard** - [Link](http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-capture-pr/43/)
72 tests run, 0 skipped, 0 failed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture pull request #79: CB-12882: (ios): adds support...
Posted by stalniy <gi...@git.apache.org>.
Github user stalniy commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-media-capture/pull/79#discussion_r120534419
--- Diff: src/ios/CDVCapture.m ---
@@ -292,6 +295,65 @@ - (CDVPluginResult*)processVideo:(NSString*)moviePath forCallbackId:(NSString*)c
return [CDVPluginResult resultWithStatus:CDVCommandStatus_OK messageAsArray:fileArray];
}
+- (void)showAlertIfAccessProhibeted
+{
+ if (![self hasCameraAccess]) {
+ [self showPermissionsAlert];
+ }
+}
+
+- (BOOL)hasCameraAccess
+{
+ if (![AVCaptureDevice respondsToSelector:@selector(authorizationStatusForMediaType:)]) {
+ return YES;
+ }
--- End diff --
I'm ok with that. Actually all iOS <= 10.0 currently are not available for installation on phones. So, it makes sense.
Initially I tried to keep support line the same as `cordova-plugin-camera` does.
So, just to double check:
1. I remove all `#pragma` statements
2. I remove all `&UIApplicationOpenSettingsURLString != NULL` checks
3. I add `ios` version restriction to be iOS 8+
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture pull request #79: CB-12882: (ios): adds support...
Posted by shazron <gi...@git.apache.org>.
Github user shazron commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-media-capture/pull/79#discussion_r120506321
--- Diff: src/ios/CDVCapture.m ---
@@ -292,6 +295,65 @@ - (CDVPluginResult*)processVideo:(NSString*)moviePath forCallbackId:(NSString*)c
return [CDVPluginResult resultWithStatus:CDVCommandStatus_OK messageAsArray:fileArray];
}
+- (void)showAlertIfAccessProhibeted
+{
+ if (![self hasCameraAccess]) {
+ [self showPermissionsAlert];
+ }
+}
+
+- (BOOL)hasCameraAccess
+{
+ if (![AVCaptureDevice respondsToSelector:@selector(authorizationStatusForMediaType:)]) {
+ return YES;
+ }
--- End diff --
Just a note here regarding iOS support for this plugin. The code is sound, but this selector already exists since iOS 7. I'm wondering if we should add iOS engine restrictions to this plugin's plugin.xml instead of all this runtime checking.
FYI The latest cordova-ios only supports iOS 9 or greater.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture pull request #79: CB-12882: (ios): adds support...
Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-media-capture/pull/79#discussion_r120496517
--- Diff: src/ios/CDVCapture.m ---
@@ -292,6 +295,65 @@ - (CDVPluginResult*)processVideo:(NSString*)moviePath forCallbackId:(NSString*)c
return [CDVPluginResult resultWithStatus:CDVCommandStatus_OK messageAsArray:fileArray];
}
+- (void)showAlertIfAccessProhibeted
--- End diff --
Can we change the function name to `showAlertIfAccessProhibited`, just for spelling, please? :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture issue #79: CB-12882: (ios): adds support for pe...
Posted by cordova-qa <gi...@git.apache.org>.
Github user cordova-qa commented on the issue:
https://github.com/apache/cordova-plugin-media-capture/pull/79
Cordova CI Build has completed successfully.
**Commit** - [Link](https://github.com/apache/cordova-plugin-media-capture/pull/79/commits/db0842798ac21eaf3b19931390fbe04008007b95)
**Dashboard** - [Link](http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-capture-pr/41/)
72 tests run, 0 skipped, 0 failed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture issue #79: CB-12882: (ios): adds support for pe...
Posted by cordova-qa <gi...@git.apache.org>.
Github user cordova-qa commented on the issue:
https://github.com/apache/cordova-plugin-media-capture/pull/79
Cordova CI Build has completed successfully.
**Commit** - [Link](https://github.com/apache/cordova-plugin-media-capture/pull/79/commits/13a5135e1defee4afd0fd38cd3c34d1dcec87580)
**Dashboard** - [Link](http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-capture-pr/40/)
72 tests run, 0 skipped, 0 failed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture issue #79: CB-12882: (ios): adds support for pe...
Posted by cordova-qa <gi...@git.apache.org>.
Github user cordova-qa commented on the issue:
https://github.com/apache/cordova-plugin-media-capture/pull/79
Cordova CI Build has completed successfully.
**Commit** - [Link](https://github.com/apache/cordova-plugin-media-capture/pull/79/commits/ba868e27f3ab2bae523a61700943ae512e461107)
**Dashboard** - [Link](http://cordova-ci.cloudapp.net:8080/job/cordova-plugin-media-capture-pr/42/)
72 tests run, 0 skipped, 0 failed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture pull request #79: CB-12882: (ios): adds support...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-plugin-media-capture/pull/79
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture pull request #79: CB-12882: (ios): adds support...
Posted by stalniy <gi...@git.apache.org>.
Github user stalniy commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-media-capture/pull/79#discussion_r120533655
--- Diff: src/ios/CDVCapture.m ---
@@ -292,6 +295,65 @@ - (CDVPluginResult*)processVideo:(NSString*)moviePath forCallbackId:(NSString*)c
return [CDVPluginResult resultWithStatus:CDVCommandStatus_OK messageAsArray:fileArray];
}
+- (void)showAlertIfAccessProhibeted
+{
--- End diff --
I put `{` in the same place as it's in other functions. So, this is from existing code:
```
- (void)getFormatData:(CDVInvokedUrlCommand*)command
{
```
And it's written like this for every function in that file.
Maybe I missed something?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture pull request #79: CB-12882: (ios): adds support...
Posted by stalniy <gi...@git.apache.org>.
Github user stalniy commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-media-capture/pull/79#discussion_r120533184
--- Diff: src/ios/CDVCapture.m ---
@@ -292,6 +295,65 @@ - (CDVPluginResult*)processVideo:(NSString*)moviePath forCallbackId:(NSString*)c
return [CDVPluginResult resultWithStatus:CDVCommandStatus_OK messageAsArray:fileArray];
}
+- (void)showAlertIfAccessProhibeted
--- End diff --
sure :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture issue #79: CB-12882: (ios): adds support for pe...
Posted by stalniy <gi...@git.apache.org>.
Github user stalniy commented on the issue:
https://github.com/apache/cordova-plugin-media-capture/pull/79
@shazron could you please provide a feedback?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture issue #79: CB-12882: (ios): adds support for pe...
Posted by filmaj <gi...@git.apache.org>.
Github user filmaj commented on the issue:
https://github.com/apache/cordova-plugin-media-capture/pull/79
Awesome, thanks for the PR @stalniy and thanks @shazron for the review!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-plugin-media-capture pull request #79: CB-12882: (ios): adds support...
Posted by stalniy <gi...@git.apache.org>.
Github user stalniy commented on a diff in the pull request:
https://github.com/apache/cordova-plugin-media-capture/pull/79#discussion_r120596224
--- Diff: src/ios/CDVCapture.m ---
@@ -292,6 +295,65 @@ - (CDVPluginResult*)processVideo:(NSString*)moviePath forCallbackId:(NSString*)c
return [CDVPluginResult resultWithStatus:CDVCommandStatus_OK messageAsArray:fileArray];
}
+- (void)showAlertIfAccessProhibeted
+{
+ if (![self hasCameraAccess]) {
+ [self showPermissionsAlert];
+ }
+}
+
+- (BOOL)hasCameraAccess
+{
+ if (![AVCaptureDevice respondsToSelector:@selector(authorizationStatusForMediaType:)]) {
+ return YES;
+ }
--- End diff --
Fixed, please check whether I put cordova-ios dependency correctly
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org