Skip to content

Commit

Permalink
feat(android)!: Android 13 support (#844)
Browse files Browse the repository at this point in the history 
* feat(android)!: Android 13 support
* refactor(android): simplify getPermissions logic
* feat(android)!: bump cordova-android requirement to >=12.0.0
* feat(android): update saveAlbumPermission to include Android 9 and below use case

---------

Co-authored-by: ochakov <[email protected]>
  • Loading branch information
@erisu @ochakov
erisu and ochakov authored Aug 26, 2023
1 parent 61a6e9b commit 505ccef
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 13 deletions.
2 changes: 1 addition & 1 deletion package-lock.json
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"cordova": ">=9.0.0"
},
"7.0.0": {
"cordova-android": ">=10.0.0",
"cordova-android": ">=12.0.0",
"cordova-ios": ">=5.1.0",
"cordova": ">=9.0.0"
},
Expand Down
6 changes: 4 additions & 2 deletions plugin.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

<engines>
<engine name="cordova" version=">=9.0.0"/>
<engine name="cordova-android" version=">=10.0.0" />
<engine name="cordova-android" version=">=12.0.0" />
<engine name="cordova-ios" version=">=5.1.0" />
</engines>

Expand All @@ -55,7 +55,9 @@
</feature>
</config-file>
<config-file target="AndroidManifest.xml" parent="/*">
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" />
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" android:maxSdkVersion="32" />
<uses-permission android:name="android.permission.READ_MEDIA_IMAGES" />
<uses-permission android:name="android.permission.READ_MEDIA_VIDEO" />
</config-file>
<config-file target="AndroidManifest.xml" parent="application">
<provider
Expand Down
64 changes: 55 additions & 9 deletions src/android/CameraLauncher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Licensed to the Apache Software Foundation (ASF) under one
package org.apache.cordova.camera;

import android.Manifest;
import android.annotation.SuppressLint;
import android.app.Activity;
import android.content.ActivityNotFoundException;
import android.content.ContentResolver;
Expand Down Expand Up @@ -59,6 +60,7 @@ Licensed to the Apache Software Foundation (ASF) under one
import java.io.InputStream;
import java.io.OutputStream;
import java.text.SimpleDateFormat;
import java.util.ArrayList;
import java.util.Date;

/**
Expand Down Expand Up @@ -122,8 +124,6 @@ public class CameraLauncher extends CordovaPlugin implements MediaScannerConnect
private boolean orientationCorrected; // Has the picture's orientation been corrected
private boolean allowEdit; // Should we allow the user to crop the image.

protected final static String[] permissions = { Manifest.permission.CAMERA, Manifest.permission.READ_EXTERNAL_STORAGE, Manifest.permission.WRITE_EXTERNAL_STORAGE };

public CallbackContext callbackContext;
private int numPics;

Expand Down Expand Up @@ -193,8 +193,9 @@ public boolean execute(String action, JSONArray args, CallbackContext callbackCo
}
else if ((this.srcType == PHOTOLIBRARY) || (this.srcType == SAVEDPHOTOALBUM)) {
// FIXME: Stop always requesting the permission
if(!PermissionHelper.hasPermission(this, Manifest.permission.READ_EXTERNAL_STORAGE)) {
PermissionHelper.requestPermission(this, SAVE_TO_ALBUM_SEC, Manifest.permission.READ_EXTERNAL_STORAGE);
String[] permissions = getPermissions(true, mediaType);
if(!hasPermissions(permissions)) {
PermissionHelper.requestPermissions(this, SAVE_TO_ALBUM_SEC, permissions);
} else {
this.getImage(this.srcType, destType);
}
Expand All @@ -221,6 +222,37 @@ else if ((this.srcType == PHOTOLIBRARY) || (this.srcType == SAVEDPHOTOALBUM)) {
// LOCAL METHODS
//--------------------------------------------------------------------------

private String[] getPermissions(boolean storageOnly, int mediaType) {
ArrayList<String> permissions = new ArrayList<>();

if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) {
// Android API 33 and higher
switch (mediaType) {
case PICTURE:
permissions.add(Manifest.permission.READ_MEDIA_IMAGES);
break;
case VIDEO:
permissions.add(Manifest.permission.READ_MEDIA_VIDEO);
break;
default:
permissions.add(Manifest.permission.READ_MEDIA_IMAGES);
permissions.add(Manifest.permission.READ_MEDIA_VIDEO);
break;
}
} else {
// Android API 32 or lower
permissions.add(Manifest.permission.READ_EXTERNAL_STORAGE);
permissions.add(Manifest.permission.WRITE_EXTERNAL_STORAGE);
}

if (!storageOnly) {
// Add camera permission when not storage.
permissions.add(Manifest.permission.CAMERA);
}

return permissions.toArray(new String[0]);
}

private String getTempDirectoryPath() {
File cache = cordova.getActivity().getCacheDir();
// Create the cache directory if it doesn't exist
Expand All @@ -243,8 +275,13 @@ private String getTempDirectoryPath() {
* @param encodingType Compression quality hint (0-100: 0=low quality & high compression, 100=compress of max quality)
*/
public void callTakePicture(int returnType, int encodingType) {
boolean saveAlbumPermission = PermissionHelper.hasPermission(this, Manifest.permission.READ_EXTERNAL_STORAGE)
&& PermissionHelper.hasPermission(this, Manifest.permission.WRITE_EXTERNAL_STORAGE);
String[] storagePermissions = getPermissions(true, mediaType);
boolean saveAlbumPermission;
if (android.os.Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
saveAlbumPermission = this.saveToPhotoAlbum ? hasPermissions(storagePermissions) : true;
} else {
saveAlbumPermission = hasPermissions(storagePermissions);
}
boolean takePicturePermission = PermissionHelper.hasPermission(this, Manifest.permission.CAMERA);

// CB-10120: The CAMERA permission does not need to be requested unless it is declared
Expand Down Expand Up @@ -275,10 +312,9 @@ public void callTakePicture(int returnType, int encodingType) {
} else if (saveAlbumPermission) {
PermissionHelper.requestPermission(this, TAKE_PIC_SEC, Manifest.permission.CAMERA);
} else if (takePicturePermission) {
PermissionHelper.requestPermissions(this, TAKE_PIC_SEC,
new String[]{Manifest.permission.READ_EXTERNAL_STORAGE, Manifest.permission.WRITE_EXTERNAL_STORAGE});
PermissionHelper.requestPermissions(this, TAKE_PIC_SEC, storagePermissions);
} else {
PermissionHelper.requestPermissions(this, TAKE_PIC_SEC, permissions);
PermissionHelper.requestPermissions(this, TAKE_PIC_SEC, getPermissions(false, mediaType));
}
}

Expand Down Expand Up @@ -1232,6 +1268,7 @@ private void checkForDuplicateImage(int type) {
// delete the duplicate file if the difference is 2 for file URI or 1 for Data URL
if ((currentNumOfImages - numPics) == diff) {
cursor.moveToLast();
@SuppressLint("Range")
int id = Integer.valueOf(cursor.getString(cursor.getColumnIndex(MediaStore.Images.Media._ID)));
if (diff == 2) {
id--;
Expand Down Expand Up @@ -1391,4 +1428,13 @@ public void onRestoreStateForActivityResult(Bundle state, CallbackContext callba

this.callbackContext = callbackContext;
}

private boolean hasPermissions(String[] permissions) {
for (String permission: permissions) {
if (!PermissionHelper.hasPermission(this, permission)) {
return false;
}
}
return true;
}
}

2 comments on commit 505ccef

@bonjourjoel
Copy link

Choose a reason for hiding this comment

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

Hi,

I'm testing the version "7.0.0-dev" in order to make android API 33 work.

  1. I would like to suggest (ask?) to remove the condition android:maxSdkVersion="32" from the xml line <uses-permission android:maxSdkVersion="32" android:name="android.permission.WRITE_EXTERNAL_STORAGE" />

Why?

  • Because it's a useless condition. Android will ignore the permission if sdk_int>=33.
  • Because it's in conflict with other plugins, which add this permission without this condition. And then it doesn't compile.
  • Because i have tested without it by deleting the line in a hook after prepare, and it seems to work fine.
  1. I have tested it with and without asking the permissions myself using cordova.plugins.diagnostic.
  • The "take picture" function works already, because it seems that you have removed the permission external storage if sdk>=33. And no other permission is required in android 13 for taking a picture.
  • The "browse gallery" function almost works because you have removed the permission external storage if sdk>=33. But you forgot to check and request the permission READ_MEDIA_IMAGES if sdk >= 33. It would also need to be added to the manifest, and no xml condition would be required because it would be ignored if the permission is not available on a device.

Regards.

@breautek
Copy link
Contributor

@breautek breautek commented on 505ccef Aug 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I would like to suggest (ask?) to remove the condition android:maxSdkVersion="32" from the xml line

On this, I understand what you're saying but there was already an extensive on this topic in the original PR. While it's not strictly necessary, it's "more" correct and the conflict could happen regardless of what we actually choose (e.g. we don't specify the maxSdkVersion but another plugin does).

Personally how I want to address this problem is to make the plugin not use external storage by default which eliminates the need to use WRITE_EXTERNAL_STORAGE even for < API 28 devices at all. It will simplify a lot of things and the app developer can then choose where they want to store the media asset from there, whether that is to move it to a permanent internal location, or a permanent external location, or off-device, etc. We have a PoC in the media-capture plugin that does just this and it appears to work well (for audio anyway).

In the meantime however, if you're using a plugin that conflicts with the permission, you can use an after_prepare hook to "correct" it. Ideally Cordova would just figure it out but handling configuration updates is something that never worked well.

In hooks/stripExtraWritePerm.js

const FS = require('fs');
const Path = require('path');

let path = Path.resolve('platforms/android/app/src/main/AndroidManifest.xml');

let manifest = FS.readFileSync(path, {
    encoding: 'utf-8'
});

manifest = manifest.replace(/^(\s)+<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" \/>$/gm, '');

FS.writeFileSync(path, manifest);

In your config.xml:

<widget ...>
    ...
    <platform name="android">
        <hook type="after_prepare" src="hooks/stripExtraWritePerm.js" />
    </platform>
</widget>

The regex will look for <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" /> include any prefixed whitespace. If you want to remove something else or if you have multiple plugins all adding this permission in a slightly different way you may need to modify or add additional regex tests.

The "browse gallery" function almost works because you have removed the permission external storage if sdk>=33. But you forgot to check and request the permission READ_MEDIA_IMAGES if sdk >= 33. It would also need to be added to the manifest, and no xml condition would be required because it would be ignored if the permission is not available on a device.

On this, I'd suggest creating an issue that can be better tracked.

Please sign in to comment.