Skip to content

Android: Remove unknown permission android.permission.RECORD_VIDEO#200

Merged
breautek merged 1 commit into
apache:masterfrom
ath0mas:fix/record-video-permission
Feb 17, 2021
Merged

Android: Remove unknown permission android.permission.RECORD_VIDEO#200
breautek merged 1 commit into
apache:masterfrom
ath0mas:fix/record-video-permission

Conversation

@ath0mas
Copy link
Copy Markdown
Contributor

@ath0mas ath0mas commented Oct 31, 2020

Platforms affected

Android

Motivation and Context

Permission android.permission.RECORD_VIDEO does not exist.
Fix #199

Description

Also described as a made-up permission in this SO post.
Android guides on audio and video recording with MediaRecorder only refer to RECORD_AUDIO and some other permissions but never to this one.

Testing

build

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)
  • I've updated the documentation if necessary

@ath0mas ath0mas force-pushed the fix/record-video-permission branch from 74f2a4f to 3f34981 Compare October 31, 2020 00:05
@ath0mas ath0mas changed the title Remove unknown permission android.permission.RECORD_VIDEO Android: Remove unknown permission android.permission.RECORD_VIDEO Oct 31, 2020
@breautek
Copy link
Copy Markdown
Contributor

breautek commented Oct 31, 2020

Looks good to me.

The plugin uses intents, not the camera apis directly so the camera permission is not required.

@breautek breautek requested review from erisu and timbru31 October 31, 2020 01:20
@vesper8
Copy link
Copy Markdown

vesper8 commented Feb 16, 2021

Is this going to be merged? It's been 3 1/2 months for a tiny change that is causing apps to be flagged by Android..

@ath0mas
Copy link
Copy Markdown
Contributor Author

ath0mas commented Feb 16, 2021

@vesper8 waiting for this PR among few others contributed but not merged either :/

(for now I am successfully using published fork only meant to pre-release those missing fixes)

@vesper8
Copy link
Copy Markdown

vesper8 commented Feb 17, 2021

@ath0mas thanks for mentioning your fork. I would like to use it also but I get an error when trying to add it:

cordova plugin add cordova-plugin-ns0m-media-capture
Invalid Version: null

Any idea why this is happening?

@breautek
Copy link
Copy Markdown
Contributor

breautek commented Feb 17, 2021

I'll give out a last call. If I don't see a rejection in the next 48 hours or so, I'll merge.

@vesper8
Copy link
Copy Markdown

vesper8 commented Feb 17, 2021

@breautek thank you!

Also.. it's not entirely clear if you need to tag a new release in order for us to be able to get the latest changes after you've merged it in? I would assume that a new release/tag is needed otherwise we'll just be getting 3.0.3 which is dated from June 2019

@jcesarmobile
Copy link
Copy Markdown
Member

jcesarmobile commented Feb 17, 2021

According to this, all permissions should be removed, not just RECORD_VIDEO, but that can be done in a separate PR

@breautek
Copy link
Copy Markdown
Contributor

breautek commented Feb 17, 2021

Since I got my green checks I'll go ahead and merge now.

@breautek breautek merged commit 0e2d9f6 into apache:master Feb 17, 2021
@breautek
Copy link
Copy Markdown
Contributor

breautek commented Feb 17, 2021 •
edited
Loading

Also.. it's not entirely clear if you need to tag a new release in order for us to be able to get the latest changes after you've merged it in? I would assume that a new release/tag is needed otherwise we'll just be getting 3.0.3 which is dated from June 2019

I'm not sure when a release to NPM will be made. Personally I might be busy due to other upcoming projects. You can (however I must say it should be for testing purposes only...) install from git. I'd recommend using the commit hash so that you can ensure a consistent "version" of what you're installing. The command below should work, although untested. Don't forget to remove your current plugin install.

cordova plugin remove cordova-plugin-media-capture
cordova plugin add https://github.com/apache/cordova-plugin-media-capture.git#0e2d9f61f9a9b8016d324750cfd577b506c7fcf1

Thank you @ath0mas for your effort in preparing this PR.

@ath0mas ath0mas deleted the fix/record-video-permission branch February 17, 2021 20:55
@ath0mas
Copy link
Copy Markdown
Contributor Author

ath0mas commented Feb 17, 2021

Thanks for the merge 👍

@ath0mas
Copy link
Copy Markdown
Contributor Author

ath0mas commented Feb 17, 2021

@ath0mas thanks for mentioning your fork. I would like to use it also but I get an error when trying to add it:

cordova plugin add cordova-plugin-ns0m-media-capture
Invalid Version: null

Any idea why this is happening?

@vesper8 same here, version with pre-release suffix seams not fine for cordova-lib.
I always use cordova plugin add cordova-plugin-ns0m-media-capture@<version>.

@ath0mas ath0mas mentioned this pull request Feb 19, 2021
Merged
@ath0mas ath0mas mentioned this pull request Apr 19, 2021
Closed
pinionpi added a commit to pinionpi/cordova-plugin-media-capture-mp4video that referenced this pull request Oct 19, 2021
@pinionpi
Merge remote-tracking branch 'origin-apache/master'
fcfe5d8 
* origin-apache/master: (91 commits)
  ci(ios): update workflow w/ iOS 15 (apache#231)
  ci: add action-badge (apache#230)
  ci: remove travis & appveyor (apache#229)
  ci: add gh-actions workflows (apache#228)
  fix(android): remove unknown permission android.permission.RECORD_VIDEO (apache#200)
  ci: add node-14.x to workflow (apache#203)
  chore: clean up package.json (apache#193)
  ci(travis): update osx xcode image (apache#189)
  breaking(ios): remove code warnings (apache#177)
  ci(travis): updates Android API level (apache#188)
  chore: adds package-lock file (apache#180)
  refactor(eslint): use cordova-eslint /w fix (apache#179)
  chore(npm): use short notation in package.json (apache#178)
  chore(asf): update git notification settings
  Update CONTRIBUTING.md
  ci: updates Node.js versions (apache#165)
  chore(npm): improve ignore list (apache#164)
  Small javadoc fix (apache#161)
  ci(travis): upgrade to node 8
  chore(release): 3.0.4-dev
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcesarmobile jcesarmobile jcesarmobile approved these changes

@NiklasMerz NiklasMerz NiklasMerz approved these changes

@breautek breautek breautek approved these changes

@erisu erisu Awaiting requested review from erisu

@timbru31 timbru31 Awaiting requested review from timbru31

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Android RECORD_VIDEO permission?

5 participants

@ath0mas @breautek @vesper8 @jcesarmobile @NiklasMerz