search

jaydeepw / poly-picker

Android library project for providing multiple image selection from the device.
406 stars 114 forks source link

Crash when taking hi-res photos #58

Closed wjwarren closed 9 years ago

wjwarren commented 9 years ago

Steps to reproduce

  1. Use a device with a hi-res camera, like the Samsung Galaxy S5 which has a 16MP camera.
  2. Start the demo.
  3. Take a photo.

Actual result

  1. The demo crashes.

Expected result

  1. The photo is saved.

Notes Lib version: 1.0.11.

For some reason the demo app does not always capture images in high resolution.

Stacktrace of crash

java.lang.NullPointerException: uriString
    at android.net.Uri$StringUri.<init>(Uri.java:467)
    at android.net.Uri$StringUri.<init>(Uri.java:457)
    at android.net.Uri.parse(Uri.java:429)
    at nl.changer.polypicker.CwacCameraFragment$DemoCameraHost.saveImage(CwacCameraFragment.java:138)
    at com.commonsware.cwac.camera.ImageCleanupTask.run(ImageCleanupTask.java:133)

Looking at the CwacCameraFragment class' saveImage() method, I do see that there is no null checking for the result of calling MediaStore.Images.Media.insertImage(...). According to the [Android docs](https://developer.android.com/reference/android/provider/MediaStore.Images.Media.html#insertImage%28android.content.ContentResolver, android.graphics.Bitmap, java.lang.String, java.lang.String%29) this method could return null.

The URL to the newly created image, or null if the image failed to be stored for any reason.

wjwarren commented 9 years ago

@jaydeepw I pushed a fix for this issue on my fork of this lib. Please let me know if the changeset looks good and I'll create a pull request.

jaydeepw commented 9 years ago

Wow. Thats great. Thanks for the fix. Please pull from master too, if not already. I cannot find your changelog. But as this is a simple null check, just submit the PR and I shall merge to master.

wjwarren commented 9 years ago

Hey, this is the changeset: https://github.com/wjwarren/poly-picker/commit/674e51f047e474d118180cdb6d39e86f829a216c

I created a new branch from the v1.0.11 tag because I didn't want to add snapshot code to my production app.

If you want I can apply the patch to the master branch and create a pull request from there.

jaydeepw commented 9 years ago

Looks good. Apply the patch.

wjwarren commented 9 years ago

Actually, what about user feedback? Right now it will just fail silently.

Perhaps show a Toast when the image can't be saved?

jaydeepw commented 9 years ago

Yes, this thought popped in my mind too when I read the changelog but thought that would be more to ask to you. Showing a Toast is okay in this case. Show it when the system returns the path == null

wjwarren commented 9 years ago

Cool, will send a pull request your way soon :smile:

wjwarren commented 9 years ago

Here is the PR: https://github.com/jaydeepw/poly-picker/pull/59