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

[CB-10093][CB-9960][android] fix error resolving content uri to file url #143

Closed
wants to merge 3 commits into from

Conversation

sencenan
Copy link

@sencenan sencenan commented Dec 3, 2015

fixing: https://issues.apache.org/jira/browse/CB-10093

Different app returns different uri. This is causing getRealPath() to return empty path.

Fixed:

  • Gallery: content://media/external/images/media/117209
  • Google Photo: embeds a external image link inside another uri
  • Dropbox: old school file url

Not fixed:

  • Google Drive: document uri, but has no document id, or the id cannot be used to query content resolver

Notes:

I have tested with: Gallery, Dropbox, Google Photo, and various file manager apps. However, this fix as is still does not address the problem with Google Drive.

It seems to me that trying to get real file path from content uri is a fundamentally flawed solution. It breaks the sandbox between apps and this is going to get worse when more apps start to return the URL in "non-standard" formats.

Use getContentResolver().openInputStream() and read the file directly could be a way forward, given that we are copying/rewriting the image file already.

For reference, I am using this following hack on my own branch to address the problem with Google Drive:

private String ouputModifiedBitmap(Bitmap bitmap, Uri uri) throws IOException {
        // Some content: URIs do not map to file paths (e.g. picasa).
        String realPath = FileHelper.getRealPath(uri, this.cordova);

        //START OF HACK
        if (realPath.length() == 0) {
            String copiedFilePath = getTempDirectoryPath() + "/"
                    + System.currentTimeMillis() + ".copied."
                    + (this.encodingType == JPEG ? "jpg" : "png");

            InputStream fin = this.cordova.getActivity().getContentResolver().openInputStream(uri);
            OutputStream fout = new FileOutputStream(copiedFilePath);

            byte[] buf = new byte[1024 * 4];
            int len;

            while ((len = fin.read(buf)) > 0) {
                fout.write(buf, 0, len);
            }

            fin.close();
            fout.close();

            realPath = copiedFilePath;
        }
        //END OF HACK

        // rest of ouputModifiedBitmap method
}

@purplecabbage
Copy link
Contributor

Copy the image to an app accessible temp location and return that url.

@riknoll
Copy link
Contributor

riknoll commented Dec 3, 2015

@purplecabbage @sencenan I agree that getting the file path out of a content URI is becoming too convoluted to be sustainable. Getting the InputStream from the ContentResolver comes with it's own awkwardness as well, however, due to Android's ExifInterface class. We use that class to extract the EXIF data and preserve it as we modify the image (for quality, orientation, etc.). Unfortunately, it does not take in an InputSteam and so we would have to find/code a new EXIF reader or do something weird with writing the image twice (copy image to temp location -> read EXIF from temp image -> overwrite temp image with modified image -> return temp image URI). Thoughts?

@sencenan
Copy link
Author

sencenan commented Dec 3, 2015

@riknoll that's my finding too with regard to ExifInterface class. I do think copying the image could be a viable solution like @purplecabbage said. Not entirely sure about the performance implications though.

@bgaillard
Copy link

Hi, I think this Pull Request is very important, for now we cannot propose to the user of our applications to pick a picture.

Is the Cordova team planning to merge this PR soon ?

Thanks

@infil00p
Copy link
Member

infil00p commented Feb 2, 2017

This isn't going to be merged since we changed how we resolve file paths. That's also why this doesn't merge cleanly.

@asfgit asfgit closed this in f2ca5ed Feb 2, 2017
thehuijb added a commit to thehuijb/cordova-plugin-camera that referenced this pull request Feb 27, 2017
* master: (24 commits)
  CB-12501 (Android) Appium tests don't use XPath selectors anymore
  CB-12469 (ios) Appium tests can now run on iOS 10
  Close apache#170
  Close apache#169
  Close apache#143
  Close apache#158
  Close apache#192
  Close apache#220
  Close apache#239
  Close apache#248
  CB-12005: Changing the getOrientation method to return the defined enumerated EXIF instead of orientation in degrees for Consistency
  Close apache#243
  Closing merged pull request: close apache#251
  Closing invalid pull request: close apache#247
  CB-12368: Fix permission check on Android
  CB-12353 Corrected merges usage in plugin.xml
  CB-12369: Add plugin typings from DefinitelyTyped
  CB-12363 Added build badges for iOS 9.3 and 10.0
  CB-12312: [Appium] [Android] A few changes to the tests:  - updated comments on how to run the tests. extra comments around functionality at certain points in the automation.  - stub of a resolution checker on test startup - still need to figure out acceptable values.  - moved session shutdown to an afterAll clause.  - changed resolution determiner from using webview-based values to using the native windows dimensions - this helps as the webview values may be scaled down intentionally by manufacturers (via changing devicePixelRatio). furthermore, since the screen dimension automation is used purely for native UI automation, better to use the dimensions reported by the native context rather than the web context.  - when finding elements by XPath, use multiple calls to avoid a Windows emulator + Android bug. Made this pattern consistent in the entire test.
  CB-12236 - Fixed RELEASENOTES for cordova-plugin-camera
  ...
thehuijb added a commit to thehuijb/cordova-plugin-camera that referenced this pull request Mar 6, 2017
* master: (197 commits)
  CB-12501 (Android) Appium tests don't use XPath selectors anymore
  CB-12469 (ios) Appium tests can now run on iOS 10
  Close apache#170
  Close apache#169
  Close apache#143
  Close apache#158
  Close apache#192
  Close apache#220
  Close apache#239
  Close apache#248
  CB-12005: Changing the getOrientation method to return the defined enumerated EXIF instead of orientation in degrees for Consistency
  Close apache#243
  Closing merged pull request: close apache#251
  Closing invalid pull request: close apache#247
  CB-12368: Fix permission check on Android
  CB-12353 Corrected merges usage in plugin.xml
  CB-12369: Add plugin typings from DefinitelyTyped
  CB-12363 Added build badges for iOS 9.3 and 10.0
  CB-12312: [Appium] [Android] A few changes to the tests:  - updated comments on how to run the tests. extra comments around functionality at certain points in the automation.  - stub of a resolution checker on test startup - still need to figure out acceptable values.  - moved session shutdown to an afterAll clause.  - changed resolution determiner from using webview-based values to using the native windows dimensions - this helps as the webview values may be scaled down intentionally by manufacturers (via changing devicePixelRatio). furthermore, since the screen dimension automation is used purely for native UI automation, better to use the dimensions reported by the native context rather than the web context.  - when finding elements by XPath, use multiple calls to avoid a Windows emulator + Android bug. Made this pattern consistent in the entire test.
  CB-12236 - Fixed RELEASENOTES for cordova-plugin-camera
  ...

# Conflicts:
#	README.md
#	src/android/CameraLauncher.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants