erisu commented on code in PR #1609: URL: https://github.com/apache/cordova-android/pull/1609#discussion_r1180612369
########## templates/project/res/xml/opener_paths.xml: ########## @@ -0,0 +1,14 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- https://developer.android.com/reference/android/support/v4/content/FileProvider.html#SpecifyFiles --> +<paths xmlns:android="http://schemas.android.com/apk/res/android";> + <!-- cordova.file.dataDirectory --> + <files-path name="files" path="." /> + <!-- cordova.file.cacheDirectory --> + <cache-path name="cache" path="." /> + <!-- cordova.file.externalDataDirectory --> + <external-files-path name="external-files" path="." /> + <!-- cordova.file.externalCacheDirectory --> + <external-cache-path name="external-cache" path="." /> + <!-- cordova.file.externalRootDirectory --> + <external-path name="external" path="." /> +</paths> Review Comment: ```suggestion </paths> ``` Add new line at the end of the file ########## templates/project/res/xml/opener_paths.xml: ########## Review Comment: Please rename this file to: `cdv_core_file_provider_paths.xml` It appears that the file name you using exists in a cordova plugin. Using the same name could lead to issues. Using a unique name will avoid conflicts. It also uses out prefixing scheme. ########## templates/project/AndroidManifest.xml: ########## @@ -46,5 +46,8 @@ <category android:name="android.intent.category.LAUNCHER" /> </intent-filter> </activity> + <provider android:name="androidx.core.content.FileProvider" android:authorities="${applicationId}.provider" android:exported="false" android:grantUriPermissions="true"> + <meta-data android:name="android.support.FILE_PROVIDER_PATHS" android:resource="@xml/opener_paths" /> Review Comment: ```suggestion <meta-data android:name="android.support.FILE_PROVIDER_PATHS" android:resource="@xml/cdv_core_file_provider_paths" /> ``` Make sure you update the resource path after you update the file name, ########## templates/project/AndroidManifest.xml: ########## @@ -46,5 +46,8 @@ <category android:name="android.intent.category.LAUNCHER" /> </intent-filter> </activity> + <provider android:name="androidx.core.content.FileProvider" android:authorities="${applicationId}.provider" android:exported="false" android:grantUriPermissions="true"> Review Comment: ```suggestion <provider android:name="androidx.core.content.FileProvider" android:authorities="${applicationId}.cdv.core.file.provider" android:exported="false" android:grantUriPermissions="true"> ``` Make the authorities' name unique to prevent collisions. The camera plugin had been used at one point in time `android:authorities="${applicationId}.provider"` and had a naming collision with another plugin that used the same name. Seeing how this had happened in the past, with the same authorities' name that you defined above, proves that there are potential plugins out there with the same name. I expect a collision. Using my suggestion should be good enough. ########## templates/project/res/xml/opener_paths.xml: ########## @@ -0,0 +1,14 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- https://developer.android.com/reference/android/support/v4/content/FileProvider.html#SpecifyFiles --> Review Comment: ```suggestion <!-- https://developer.android.com/reference/androidx/core/content/FileProvider --> ``` The Android Support Library is deprecated. Point to the AndroidX Library documentation instead. There is no anchor to "Specifying Available Files" but it exists on the above link ########## templates/project/res/xml/opener_paths.xml: ########## @@ -0,0 +1,14 @@ +<?xml version="1.0" encoding="utf-8"?> Review Comment: ```suggestion <?xml version="1.0" encoding="utf-8"?> <!-- Licensed to the Apache Software Foundation (ASF) under one or more contributor license agreements. See the NOTICE file distributed with this work for additional information regarding copyright ownership. The ASF licenses this file to you under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. --> ``` Add Apache License Header ########## framework/src/org/apache/cordova/engine/SystemWebChromeClient.java: ########## @@ -212,53 +217,110 @@ public View getVideoLoadingProgressView() { } @Override - public boolean onShowFileChooser(WebView webView, final ValueCallback<Uri[]> filePathsCallback, final WebChromeClient.FileChooserParams fileChooserParams) { + public boolean onShowFileChooser(WebView webView, final ValueCallback<Uri[]> filePathsCallback, + final WebChromeClient.FileChooserParams fileChooserParams) { + Intent fileIntent = fileChooserParams.createIntent(); + // Check if multiple-select is specified Boolean selectMultiple = false; if (fileChooserParams.getMode() == WebChromeClient.FileChooserParams.MODE_OPEN_MULTIPLE) { selectMultiple = true; } - Intent intent = fileChooserParams.createIntent(); - intent.putExtra(Intent.EXTRA_ALLOW_MULTIPLE, selectMultiple); - + fileIntent.putExtra(Intent.EXTRA_ALLOW_MULTIPLE, selectMultiple); + // Uses Intent.EXTRA_MIME_TYPES to pass multiple mime types. String[] acceptTypes = fileChooserParams.getAcceptTypes(); if (acceptTypes.length > 1) { - intent.setType("*/*"); // Accept all, filter mime types by Intent.EXTRA_MIME_TYPES. - intent.putExtra(Intent.EXTRA_MIME_TYPES, acceptTypes); + fileIntent.setType("*/*"); // Accept all, filter mime types by Intent.EXTRA_MIME_TYPES. + fileIntent.putExtra(Intent.EXTRA_MIME_TYPES, acceptTypes); + } + + // Image from camera intent + Uri tempUri = null; + Intent captureIntent = null; + if (fileChooserParams.isCaptureEnabled()) { + captureIntent = new Intent(MediaStore.ACTION_IMAGE_CAPTURE); Review Comment: Please confirm: I believe you need to add this to the [AndroidManifest queries](https://developer.android.com/guide/topics/manifest/queries-element). ```xml <queries> <intent> <action android:name="android.media.action.IMAGE_CAPTURE" /> </intent> </queries> ``` Ref: https://developer.android.com/training/package-visibility Because the Intent [MediaStore.ACTION_IMAGE_CAPTURE](https://developer.android.com/reference/android/provider/MediaStore#ACTION_IMAGE_CAPTURE) is for accessing the system's camera application for capturing an image and returning it, the app might not have visibility to the package. > When an app targets Android 11 (API level 30) or higher and queries for information about the other apps that are installed on a device, the system filters this information by default. This filtering behavior means that your app can’t detect all the apps installed on a device, which helps minimize the potentially sensitive information that your app can access but doesn't need to fulfill its use cases. Here is also an SO referencing similar feedback: https://stackoverflow.com/questions/63246442/android-11-r-return-empty-list-when-querying-intent-for-action-image-capture -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@cordova.apache.org For additional commands, e-mail: issues-h...@cordova.apache.org
