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): Allow loading local files #883

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NoelDeMartin
Copy link

Platforms affected

Android

Motivation and Context

This PR allows opening local files in the inappbrowser for Android applications targeting SDK 30 or later. The use-case for this is the same as #693, but applicable to Android instead of iOS.

Description

Starting with SDK 30, the default value for the allowFileAccess setting changed in Android (as specified in the documentation). It was true before and it's false now, meaning that this can potentially break existing applications.

This problem is something that has already been tackled in cordova core (Initially in apache/cordova-android#1111, later updated in apache/cordova-android#1222). So I just followed the same strategy and this setting is only updated if the AndroidInsecureFileModeEnabled preference is set to true.

Testing

I've tested the fix in my application. Given that the code is quite simple and it's already in cordova core, I don't think it needs any further testing.

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • 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)
  • I've updated the documentation if necessary

jeansparipassu added a commit to paripassubr/cordova-plugin-inappbrowser that referenced this pull request Nov 8, 2021
@quenis
Copy link

quenis commented Nov 19, 2021

Hello, please accept the merge 'cause we need that correction. Thanks bro!

@NoelDeMartin
Copy link
Author

@quenis Until this gets merged, you can use our fork which is the 5.0 release + this one fix. https://github.com/moodlemobile/cordova-plugin-inappbrowser

@quenis
Copy link

quenis commented Nov 19, 2021

@quenis Until this gets merged, you can use our fork which is the 5.0 release + this one fix. https://github.com/moodlemobile/cordova-plugin-inappbrowser

@NoelDeMartin Thanks man for yr reply!

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.

3 participants