search

olofd / react-native-photos-framework

A modern and comprehensive CameraRoll/iCloud-library-API for React Native 📸 📹
MIT License
220 stars 99 forks source link

Fix crash when calling metadata on new Portrait mode #40

Closed dieppe closed 7 years ago

dieppe commented 7 years ago

A new image subtype corresponding to the new Portrait mode available on the iPhone 7 Plus has been added to the framework.

The following line would crash when accessing metadata on assets which such a subtype:

[dictToExtend setObject:[RNPFHelpers nsOptionsToArray:[asset mediaSubtypes] andBitSize:32 andReversedEnumDict:[RCTConvert PHAssetMediaSubtypeValuesReversed]]

(See PHAssetsService.m#L104)

Adding the new corresponding image subtype photoDepthEffect to the list of known media sub types did the trick.

Update: Of course I just now saw the issues #28 and #32 ...

olofd commented 7 years ago

Is it possible for you to add some type of guard so that an unknown values does not crash but rather generate a warning in the console or something? Just so we don't have weird crashes when Apple adds new values?

Would be great with a guard.

But I'm merging this right away. Post another PR if you have the time for adding and testing the guard.

olofd commented 7 years ago

FYI. This has been discussed before here: #28

dieppe commented 7 years ago

Yep, missed that and saw it just after PRing.

In fact I just noticed the guard is in place since commit 1bb42f2b459bbdec4c509b8b8fc3f1b6a1d031a2...

Anyway, the title of the PR is wrong then, it only adds a new subtype and does not prevent a crash, since no crash were happening after the aforementioned commit.

Next time I'll update first, debug then ;)

olofd commented 7 years ago

Ah! Haha, I did not remember that we actually added the guard. Great. Thanks!