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

fix(android): file path correction if Uri authority is FileProvider #585

Merged
merged 2 commits into from
Aug 11, 2021

Conversation

Suzzak
Copy link
Contributor

@Suzzak Suzzak commented Apr 17, 2020

Platforms affected

Android

Motivation and Context

Select some videos or images return path error when Uri authority is FileProvider, then the app restarts.. That's why this bug is not always happening but it can be really annoying.

It's probably fixing the following issues (hard to say some descriptions are rather short): Resolves #573, resolves #531, resolves #548, resolves #554, resolves #558, resolves #563

Description

Checking if Uri authority is FileProvider for MediaStore content then find the URI.

Testing

Tested with most of my user's app and with my Huawei p smart 2019 - Android 10 and with Samsung Galaxy S5.

Checklist

  • I've run the tests to see all new and existing tests pass
  • Commit is prefixed with (platform) if this change only applies to one platform (e.g. (android))
  • If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct keyword to close issues using keywords)

@Suzzak Suzzak changed the title (Android) Fix: File path correction if Uri authority is FileProvider (Android) Fix: File path correction if Uri authority is FileProvider label:bug label:platform:android Apr 17, 2020
@Suzzak Suzzak changed the title (Android) Fix: File path correction if Uri authority is FileProvider label:bug label:platform:android (Android) Fix: File path correction if Uri authority is FileProvider Apr 17, 2020
@breautek
Copy link
Contributor

breautek commented Apr 25, 2020

It looks good, do you know if there is a test app available that shows the reproduction of the original issue?

Edit: The sample code #531 worked for me to reproduce this issue

Copy link
Contributor

@breautek breautek left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution.

Was able to reproduce the issue using some sample codes provided in the linked issues. Confirmed that this PR addresses the problem. 👍

I would like to see a review from at least one other member before I merge.

@breautek breautek requested review from erisu and timbru31 April 25, 2020 02:05
fmichael added a commit to fmichael/cordova-plugin-camera that referenced this pull request Jun 11, 2020
@breautek

This comment has been minimized.

@isamuAsial
Copy link

Please merge it. It seems good enough change and the current Android camera is useless.

Co-authored-by: Tim Brust <ratchet.player@gmx.de>
@breautek breautek requested a review from timbru31 July 14, 2020 01:44
@sirhaplo
Copy link

I used this PR to try if it resolves the issue #554 but the problem is still here.
I ( and other people ) think that is not a problem about uri

@robinparadise
Copy link

Guys, any update about this 🥶?

I've tried @timbru31 fileHelper.java and it doesn't work

Thanks

@robinparadise
Copy link

robinparadise commented Jul 21, 2020

I fixed with this code:

public static String getFilePath(Context context, Uri uri) {

    Cursor cursor = null;
    final String[] projection = {
      MediaStore.MediaColumns.DISPLAY_NAME
    };

    try {
      cursor = context.getContentResolver().query(uri, projection, null, null,
        null);
      if (cursor != null && cursor.moveToFirst()) {
        final int index = cursor.getColumnIndexOrThrow(MediaStore.MediaColumns.DISPLAY_NAME);
        return cursor.getString(index);
      }
    } finally {
      if (cursor != null)
        cursor.close();
    }
    return null;
  }

..................

      // DownloadsProvider
      else if (isDownloadsDocument(uri)) {
        String fileName = getFilePath(context, uri);
        if (fileName != null) {
          return Environment.getExternalStorageDirectory().toString() + "/Download/" + fileName;
        }
........................

@sylber-cz
Copy link

Any update on this?

I fixed with this code:

public static String getFilePath(Context context, Uri uri) {

    Cursor cursor = null;
    final String[] projection = {
      MediaStore.MediaColumns.DISPLAY_NAME
    };

    try {
      cursor = context.getContentResolver().query(uri, projection, null, null,
        null);
      if (cursor != null && cursor.moveToFirst()) {
        final int index = cursor.getColumnIndexOrThrow(MediaStore.MediaColumns.DISPLAY_NAME);
        return cursor.getString(index);
      }
    } finally {
      if (cursor != null)
        cursor.close();
    }
    return null;
  }

..................

      // DownloadsProvider
      else if (isDownloadsDocument(uri)) {
        String fileName = getFilePath(context, uri);
        if (fileName != null) {
          return Environment.getExternalStorageDirectory().toString() + "/Download/" + fileName;
        }
........................

Can you share your FileHelper.java file?

@robinparadise
Copy link

@sylber-cz here is my fileHelper.java
FileHelper[OK].java.zip

@sylber-cz
Copy link

@sylber-cz here is my fileHelper.java
FileHelper[OK].java.zip

Thanks Robin, may I ask what version of cordova-plugin-camera are you running? Because I did a quick comparison of your and mine Filehelper.java file, and it seems like they are not equal with each other..

FileHelper.zip

@erisu erisu changed the title (Android) Fix: File path correction if Uri authority is FileProvider fix(android): file path correction if Uri authority is FileProvider Aug 11, 2021
Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

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

There were some unnecessary cleanup, but code wise it LGTM.

@erisu erisu merged commit c7971d9 into apache:master Aug 11, 2021
DavidWiesner added a commit to DavidWiesner/cordova-plugin-camera that referenced this pull request Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
8 participants