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-25259] Android: Use ContentProvider to obtain image preview #9419

Merged
merged 4 commits into from Jan 24, 2018

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented Sep 11, 2017

  • Use TiFileProvider to obtain the Uri for the image preview for compatibility with Android 7.0+
TEST CASE
<uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE"/>
<uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/>
var win = Ti.UI.createWindow({backgroundColor: 'grey'}),
    btn = Ti.UI.createButton({title : 'SCREENSHOT'});

btn.addEventListener('click', function() {
    Ti.Media.takeScreenshot(function(image) {
        if (image && image.media) {
            var tmp = Ti.Filesystem.getFile(Ti.Filesystem.externalStorageDirectory, 'temp.png');

            tmp.write(image.media);
            Ti.API.info('temp file: ' + tmp.resolve());

            Ti.Media.previewImage({
                success : function() {
                    Ti.API.info('preview success');
                },
                error : function(e) {
                    alert('preview failed: ' + e.message);
                    Ti.API.error(e.message);
                },
                image : tmp.read()
            });
        } else {
            alert('failed to obtain screenshot');
        }
    });
});

win.add(btn);
win.open();
NOTE: Also test this on KitKat (4.4) and below devices, there could be a separate issue writing to external storage. But I do not have a device with 4.4 to verify this.

JIRA Ticket

Copy link
Contributor

@jquick-axway jquick-axway left a comment

Choose a reason for hiding this comment

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

CR: Pass

@build
Copy link
Contributor

build commented Nov 14, 2017

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

@garymathews garymathews modified the milestones: 7.0.0, 7.1.0 Nov 29, 2017
TiIntentWrapper previewIntent = new TiIntentWrapper(intent);
String mimeType = image.getMimeType();
Uri imageUri = TiFileProvider.createUriFrom(f.getNativeFile());
Copy link
Contributor

Choose a reason for hiding this comment

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

The getNativeFile() method can return null if:

  • The photo came from the cloud.
  • The image was loaded from a blob. (Can test with View.toImage().)

Calling getNativeFile() will crash with a NullPointerException for the above case too unless my code changes from PR #9223 gets merged in. (Required fixes to TitaniumBlob and TiBlob.)

Ideally, we should store the image to a temp file if it's in pure blob form before previewing it.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can make this a separate bug-fix/PR if you want, because it looks like the issue I mentioned was always there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this should be a separate issue

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay. I just wrote up a ticket with a reproducible case here...
https://jira.appcelerator.org/browse/TIMOB-25619

Copy link
Contributor

@jquick-axway jquick-axway left a comment

Choose a reason for hiding this comment

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

CR: Pass

@ssjsamir ssjsamir requested review from ssjsamir and removed request for ssjsamir December 20, 2017 17:12
@lokeshchdhry
Copy link
Contributor

FR Passed.

No exposed beyond app through Intent.getData() seen on API 24 & above.
Tested on 7.1.1, 6.0.1 & 4.4.4.

Studio Ver: 5.0.0.201712081732
SDK Ver: 7.1.0 local build
OS Ver: 10.13.2
Xcode Ver: Xcode 8.3.3
Appc NPM: 4.2.11
Appc CLI: 7.0.1
Daemon Ver: 1.0.1
Ti CLI Ver: 5.0.14
Alloy Ver: 1.10.10
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 1.8.0_101
Devices: ⇨ google Pixel --- Android 7.1.1
⇨ google Nexus 5 --- Android 6.0.1

@lokeshchdhry
Copy link
Contributor

@garymathews , Can you please add tests.

@lokeshchdhry lokeshchdhry merged commit d0d0097 into tidev:master Jan 24, 2018
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