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

Bugfix issue 665 #700

Conversation

PieterVanPoyer
Copy link
Contributor

@PieterVanPoyer PieterVanPoyer commented Dec 31, 2020

Platforms affected

Android

Motivation and Context

Fixes #665
Fixes #696

Description

In the processResultFromCamera method, the imageFilePath is used.
https://github.com/apache/cordova-plugin-camera/blob/master/src/android/CameraLauncher.java#L475
It was not restored when the activity was destroyed.

public void onRestoreStateForActivityResult(Bundle state, CallbackContext callbackContext) {

This resulted in a NullPointerException because the resulting sourcePath was null.
String sourcePath = (this.allowEdit && this.croppedUri != null) ?

So I've added the imageFilePath to the state Bundle in the saveInstance and the restoreInstance methods.

You can install and test the plugin (on Android) with next command.

npx cordova plugin remove cordova-plugin-camera
npx cordova plugin add https://github.com/PieterVanPoyer/cordova-plugin-camera/#bugfix/issue-665-save-instance-restore-bug

You could test it with next reproduction repo:
https://github.com/PieterVanPoyer/cordova-camera-plugin-testing-app

Testing

Tested it on an Android 10 emulator.
I enabled the developers mode on the emulator and made the activity destroy everytime the camera is shown.
So I forced the resume flow on Android

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

@PieterVanPoyer
Copy link
Contributor Author

@breautek @jcesarmobile @erisu Is anybody able to review this PR? Kind regards.

Copy link
Contributor

@breautek breautek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@PieterVanPoyer PieterVanPoyer force-pushed the bugfix/issue-665-save-instance-restore-bug branch from 35ea4c9 to db4be57 Compare January 8, 2021 16:17
@PieterVanPoyer
Copy link
Contributor Author

PieterVanPoyer commented Jan 23, 2021

@breautek @erisu maybe it's time to merge this one. And start a release. Some users report that this PR fixes the linked issues.

Kind regards
Pieter

Co-authored-by: Tim Brust <github@timbrust.de>
@PieterVanPoyer
Copy link
Contributor Author

@timbru31 thanks for the review. I did commit your suggestion.

@PieterVanPoyer
Copy link
Contributor Author

@timbru31 or @breautek can anyone merge this one to the master. Most users report that this PR fixes the linked issues.

@breautek breautek closed this Jan 29, 2021
@breautek breautek reopened this Jan 29, 2021
@breautek
Copy link
Contributor

Generally I like to have 2 green checks, but I'll merge sometime Monday assuming there are no objections. Sooner if I get another green check from a PMC member.

@breautek
Copy link
Contributor

breautek commented Feb 2, 2021

Tests are green, and haven't had any objections so merging!

Thank you for your contribution @PieterVanPoyer

@breautek breautek merged commit f704689 into apache:master Feb 2, 2021
@FlossyWeb
Copy link

Hi there,
Are you planning to update the npm package anytime soon ?
It would be nice...
Thank you guys

@codeconsole
Copy link
Contributor

Any chance for a merge or at least a comment on #712 ?

luckyboykg added a commit to mobiliarmad/cordova-plugin-camera that referenced this pull request Mar 24, 2021
@satheeshkumartitan
Copy link

satheeshkumartitan commented May 27, 2021

I've also updated the latest plugin 5.0.2, but the same problems occur on Android 10 (device - redmi note 7pro). The plugin 5.0.2. Forcefully close the application once we capture a photo. This happens on some specific devices. Please update the new version I'm waiting for the new version
Node -v ----14.16.0
ionic -v ---- 6.13.1
cordova -v----10.0.0
ionic cordova platform add android@9.1.0
npm i @ionic-native/camera@5.33.0
tested and both same crash.

I'm waiting for your response, thanks

@jay34fr
Copy link

jay34fr commented Jun 9, 2021

Hi all,
Same issue here.
I updated to 5.0.2 but the app is still restarting after taking and validating a photo.

Cordova 9.0.0
Cordova Android 9.0.0

DavidWiesner pushed a commit to DavidWiesner/cordova-plugin-camera that referenced this pull request Dec 7, 2021
* apacheGH-665 - store the imageFilePath when the app is paused (onSaveInstance) and restore it back.

* Update src/android/CameraLauncher.java whitespace layout

Co-authored-by: Tim Brust <github@timbrust.de>

Co-authored-by: Tim Brust <github@timbrust.de>
(cherry picked from commit f704689)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants