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

photo library access #1459

Closed
7macaw opened this issue Sep 18, 2016 · 10 comments
Closed

photo library access #1459

7macaw opened this issue Sep 18, 2016 · 10 comments

Comments

@7macaw
Copy link

7macaw commented Sep 18, 2016

ChartViewBase.saveToCameraRoll() accesses the photo library (obviously.) While a nice feature, Itunes Connect now* insists that an NSPhotoLibraryUsageDescription key should be added to Info.plist. And adding it and not actually having functionality that accesses photo library may cause problems with review.

Most people probably don't need this functionality, so maybe it should be conditionally-compiled and off by default.

*note sure how recent this is, I just grabbed this branch for the Swift 3.0 update, but it is a real problem.

@liuxuan30
Copy link
Member

hmm.. where it says it will be a review problem if you had functionality but not using it?

@ashrobo
Copy link

ashrobo commented Sep 18, 2016

Agreed, my app was just rejected after updating to the Swift 3 version of the library and submitting with Xcode 8. Took me a while to figure out which library was causing the issue.

This app attempts to access privacy-sensitive data without a usage description. The app's Info.plist must contain an NSPhotoLibraryUsageDescription key with a string value explaining to the user how the app uses this data.

Given I (and potentially many other users of the library) don't use this feature it would be nice to not have to add this key to my info.plist.

@danielgindi
Copy link
Collaborator

Well it's a feature, and Apple probably detects it's existence in the code.
It's not easy to allow you to choose whether to contain this piece of code or not - we could maybe enable/disable it based on macros.
If there are other suggestions - please put it in here. We'll vote!

@ashrobo
Copy link

ashrobo commented Sep 18, 2016

Yes I'm not actually sure the best way around it, I guess it's not too much hassle to add the key to the info.plist. It might just be worth adding a note in the readme as it took me a while to realize it was Charts causing the issue.

@7macaw
Copy link
Author

7macaw commented Sep 18, 2016

I didn't say it will cause problems in review, but that it may. Apple is finicky about these things at times. And it is already annoying - we either have to go into the library code and take out the feature, or set a dummy NSPhotoLibraryUsageDescription string.

I think not enabling it unless a macro is defined should be a could solution.

@ezamagni
Copy link
Contributor

If i can suggest a more drastic but (imho) cleaner approach, i would simply remove the feature.
After all, it's only a simple wrapper around calling UIImageWriteToSavedPhotosAlbum, we already have an UIImage object by calling getChartImage and it's the client app's responsibility to deal with it in whatever way best suits its needs.

@danielgindi
Copy link
Collaborator

@ezamagni I tend to support your view

@pmairoldi
Copy link
Collaborator

I agree too. Saving to camera roll is a very platform dependent thing which dosen't make much sense in a cross platform library. Outputting an NS/UIImage is fine I think.

@danielgindi
Copy link
Collaborator

Turns out Apple (automated) Review fails for NSPhotoLibraryUsageDescription even if it's in InfoPlist.strings, something going on there with this specific key. Other apps, no relation to Charts, fail to upload to Apple.

I'm removing this method anyway.

@1397v
Copy link

1397v commented Oct 10, 2017

Well then, as you decided to remove this method how do we find the path to the camera roll?
Thank you!

(For now, I am using the same method as before, using UIImageWriteToSavedPhotosAlbum after getChartImage, but it doesn't work setting transparency to true at getChartImage)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants