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

[TIMOB-25363] Android: Use ContentProvider for Intent data #9500

Merged
merged 3 commits into from Oct 17, 2017

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented Oct 2, 2017

  • Use ContentProvider for Intent DATA
TEST CASE
var extFile = Ti.Filesystem.getFile(Ti.Filesystem.externalStorageDirectory, 'test.pdf'),
    openPDF = function(pdf) {
        Ti.Android.currentActivity.startActivity(
            Ti.Android.createIntent({
                action : Ti.Android.ACTION_VIEW,
                type : 'application/pdf',
                data : pdf.nativePath
            })
        );
    };

if (extFile.exists()) {
    openPDF(extFile);
} else {
    var httpClient = Titanium.Network.createHTTPClient({
        onload: function() {
            extFile.createFile();
            extFile.write(this.responseData);
            openPDF(extFile);
        }
    });
    httpClient.open('GET','https://github.com/mozilla/pdf.js/raw/master/examples/helloworld/helloworld.pdf');
    httpClient.send();
}

JIRA Ticket

@garymathews garymathews added this to the 7.0.0 milestone Oct 2, 2017
@garymathews garymathews changed the title [TIMOB-25363] Android: Use ContentProvider for Intent DATA [TIMOB-25363] Android: Use ContentProvider for Intent data Oct 2, 2017
} else {
intent.setType(type);
}
} else if (data != null) {
intent.setData(Uri.parse(data));
intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
intent.setData(TiFileProvider.createUriFrom(data));
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing import:
import org.appcelerator.titanium.io.TiFileProvider;

@build
Copy link
Contributor

build commented Oct 3, 2017

Fails
🚫

Tests have failed, see below for more information.

Warnings
⚠️

🔬 There are library changes, but no changes to the unit tests. That's OK as long as you're refactoring existing code

Tests:

Classname Name Time Error
android.Titanium.UI.View TIMOB-20598 2.148 AssertionError: expected 180 to equal 100
ios.Titanium.UI.View TIMOB-20598 5.003 file:///Users/build/Libra
ios.Titanium.UI.WebView loading 10.003 file:///Users/build/Librar

Generated by 🚫 dangerJS

try {
String path = getUriPrefix() + file.getAbsolutePath();
String path = getUriPrefix() + filePath.replaceAll("^file://(/android_asset)?", "");
Copy link
Contributor

Choose a reason for hiding this comment

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

When we wrote the TiContentProvider, we purposely limited the createUriFrom() method to only support File objects to limit it to only supporting external files on the filesystem. If we were to add support for reading asset files and resource files in the future (and we should), we would have to override the ContentProvider.openAssetFile() method in order to access embedded files within the APK as well change the URI prefix to better represent where the file came from (currently hardcoded to "/filesystem").

The test case for this code change is limited to external files. Can we keep the old way of doing it for now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is still only limited to external files (even though there's regex that cleans our /android_asset identifier). I've just added the ability to pass a file path String in addition to the existing File object. Which is more appropriate for passing the data String when creating the Intent instead of needing to create a File object only to grab it's path again.

@lokeshchdhry
Copy link
Contributor

FR Passed

No FileUriExposedException is thrown & pdf opens successfully.

Studio Ver: 4.10.0.201709271713
SDK Ver: 7.0.0 local build
OS Ver: 10.12.3
Xcode Ver: Xcode 8.3.3
Appc NPM: 4.2.9
Appc CLI: 6.2.4
Ti CLI Ver: 5.0.14
Alloy Ver: 1.9.14
Node Ver: 7.10.1
Java Ver: 1.8.0_101
Devices: ⇨ google Nexus 5 --- Android 6.0.1
⇨ google Pixel --- Android 7.1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants