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-9148 - (android) Add support for input[type=file] to InAppBrowser! #205

Closed
wants to merge 8 commits into from

Conversation

Projects
None yet
8 participants
@darron1217
Copy link
Contributor

darron1217 commented Jan 3, 2017

Platforms affected

Android

What does this PR do?

This PR enables default action for input[type=file] at inappbrowser.
Which means, input[type=file] opens native file chooser in android.

What testing has been done on this change?

I tested input[type=file] element to open file chooser.
And it worked beautifully :D

Checklist

  • [ v ] Reported an issue in the JIRA database
  • [ v ] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
  • [ X ] Added automated test coverage as appropriate for this change.

@darron1217 darron1217 changed the title Cb 9148 (android) Add support for input[type=file] to InAppBrowser! CB-9148 - (android) Add support for input[type=file] to InAppBrowser! Jan 3, 2017

@cordova-qa

This comment has been minimized.

Copy link

cordova-qa commented Jan 3, 2017

Cordova CI Build has completed successfully.

Commit - Link
Dashboard - Link

Builder Name Console Output Test Report Device Logs
Windows 10 Store Link Link Link
iOS Link Link Link
Android 4.4 Link Link Link
Android 5.1 Link Link Link
@infil00p
Copy link
Member

infil00p left a comment

There's some minor changes that need to be done before we can accept this, such as using the cordova.startActivityForResult method instead of grabbing the Activity, but once that cleanup is done, I'm fine with this being added.

Reminder: Cordova does not support Android versions lower than Jellybean, which is Android 4.1. We don't need code for anything lower unless it's absolutely required.


// [important] Register ActvityResultCallback on Cordova
cordova.setActivityResultCallback(InAppBrowser.this);
cordova.getActivity().startActivityForResult(content, FILECHOOSER_REQUESTCODE_LOLLIPOP);

This comment has been minimized.

Copy link
@infil00p

infil00p Jan 3, 2017

Member

This should be using the Plugin API's startActivityForResult where you pass in the plugin that you're sending the Activity to. Doing this is actually very unstable, and can be unpredictable since other plugins also use Intents.

i.addCategory(Intent.CATEGORY_OPENABLE);

cordova.setActivityResultCallback(InAppBrowser.this);
cordova.getActivity().startActivityForResult( Intent.createChooser(i, "File Chooser"), FILECHOOSER_REQUESTCODE);

This comment has been minimized.

Copy link
@infil00p

infil00p Jan 3, 2017

Member

This should also be using the plugin API to start the activity for result.

// Call file chooser for Android 3.0+
openFileChooser(uploadMsg, "");
}

This comment has been minimized.

Copy link
@infil00p

infil00p Jan 3, 2017

Member

Why does this need to be here? We do not support any versions of Android below Android 4.1, so this would never be called.

This comment has been minimized.

Copy link
@jcesarmobile

jcesarmobile Jan 3, 2017

Member

Well, latest cordova-android doesn't support Android versions below 4.1, but the plugin can be used with older versions of cordova-android as we have the engine set to cordova >=3.1.0. We should bump the engine version to the one that started setting the SDK version to 16 if we are not going to support older Android versions in plugins.

This comment has been minimized.

Copy link
@infil00p

infil00p Jan 3, 2017

Member

@jcesarmobile In this case, this method is supporting versions of Android below 3.0, and we don't have a version of Cordova that we support or maintain that uses this. (We dropped Gingerbread years ago to much fanfare!!)

For some reason the version that supports 3.0 and higher is the primary method, so we have to keep it, but I have no idea why this method is needed.

// For Android 5.0+
public boolean onShowFileChooser (WebView webView, ValueCallback<Uri[]> filePathCallback, WebChromeClient.FileChooserParams fileChooserParams)
{
Log.d(LOG_TAG, "File Chooser 5.0+");

This comment has been minimized.

Copy link
@jcesarmobile

jcesarmobile Jan 3, 2017

Member

We use LOG instead of Log to use Cordova LOG instead of default Log, so it can be disabled with a preference.

This comment has been minimized.

Copy link
@infil00p

infil00p Jan 3, 2017

Member

I'm going to open an issue to create an Android Style Guide or something. I kinda feel bad we're being this nit-picky on someone's first PR. (I'm still going to do it, because we're supposed to, but still)

This comment has been minimized.

Copy link
@darron1217

darron1217 Jan 4, 2017

Author Contributor

@infil00p It's okay because I'm really enjoying this process :)
I am the person who loves stable and beautiful codes.
Thanks for all the comments and interests about my suggestion!
I'm newbie here, but I'll try hard to be a good contributor here :D

@darron1217

This comment has been minimized.

Copy link
Contributor Author

darron1217 commented Jan 4, 2017

@infil00p
I fixed my code according to comments :)

@cordova-qa

This comment has been minimized.

Copy link

cordova-qa commented Jan 4, 2017

Cordova CI Build has completed successfully.

Commit - Link
Dashboard - Link

Builder Name Console Output Test Report Device Logs
Windows 10 Store Link Link Link
iOS Link Link Link
Android 4.4 Link Link Link
Android 5.1 Link Link Link
@infil00p

This comment has been minimized.

Copy link
Member

infil00p commented Jan 6, 2017

@darron1217 Looks good! Did you get the iCLA in? Once you do that, I'll merge this.

@asfgit asfgit closed this in fe686b3 Jan 9, 2017

@infil00p

This comment has been minimized.

Copy link
Member

infil00p commented Jan 9, 2017

Congrats on your first major contribution to Cordova! Well done!

@darron1217

This comment has been minimized.

Copy link
Contributor Author

darron1217 commented Jan 9, 2017

@infil00p Thank you for your guidance :)
I was glad that my work can be help for improving project 👍

@markusjwetzel

This comment has been minimized.

Copy link

markusjwetzel commented Jan 14, 2017

Does not seem to work for me on Android 4.3. When I try to choose a file for an input[type=file] field, I get an "application for this action cannot be found" error.

@darron1217

This comment has been minimized.

Copy link
Contributor Author

darron1217 commented Jan 16, 2017

@markusjwetzel I've just tested it on Android 4.4 and as you said, it isn't working...
I'll fix it ASAP :)

--> I realized that Android removed the openfilechooser function from Kitkat 4.4.2 which is version that I've tested...
Currently, there is no solution for Kitkat 4.4.2 :(

@darron1217

This comment has been minimized.

Copy link
Contributor Author

darron1217 commented Jan 16, 2017

@markusjwetzel Did you tested it on real device?

@infil00p

This comment has been minimized.

Copy link
Member

infil00p commented Jan 16, 2017

@markusjwetzel @darron1217 Technically, this won't work on any version of Android below API Level 21:
https://developer.android.com/reference/android/webkit/WebChromeClient.html

And, Google didn't remove the functionality, in fact they added it with the 5.0.x release. Before 5.0, this functionality didn't even exist. This issue should really exist in another JIRA issue, although due to it only affecting 4.4 and below, it would be listed as a low priority issue.

@jcesarmobile

This comment has been minimized.

Copy link
Member

jcesarmobile commented Jan 16, 2017

@infil00p openFileChooser wasn't a public API, but it was working before Android 4.4.0. In Android 4.4.0 they decided to remove it and said that it wasn't a public API https://code.google.com/p/android/issues/detail?id=62220
Then they added it back in 4.4.3, so on 4.4.0, 4.4.1 and 4.4.2 it won't work.
And finally in Android 5 they added the public API onShowFileChooser

I agree, this should be a new issue.

@darron1217, BTW, you can take a look into the implementation in the Cordova webview, I think it works fine with Android 4.3
https://github.com/apache/cordova-android/blob/master/framework/src/org/apache/cordova/engine/SystemWebChromeClient.java

@markusjwetzel

This comment has been minimized.

Copy link

markusjwetzel commented Jan 16, 2017

@darron1217 Yes, I tested it on a real device.

@darron1217

This comment has been minimized.

Copy link
Contributor Author

darron1217 commented Jan 16, 2017

@infil00p @jcesarmobile Thanks for all the comments :)
Now I can clearly see what's going on with this strange input=file bug....

@darron1217

This comment has been minimized.

Copy link
Contributor Author

darron1217 commented Jan 31, 2017

@markusjwetzel Can you share your detailed error log?
I think the error "application for this action cannot be found" is not a general issue.

@markusjwetzel

This comment has been minimized.

Copy link

markusjwetzel commented Feb 1, 2017

Sorry, really new to app development, so I don't know how to get an error log from a real device and don't really have time to figure that out. But I guess that this error can be reproduced easily.

The error was translated from German. So the official English version might differ a bit, but at least the meaning should be the same. Hope that helps a little bit. 😃

dpa99c added a commit to dpa99c/cordova-plugin-themeablebrowser that referenced this pull request Mar 22, 2017

@phstc

This comment has been minimized.

Copy link

phstc commented Apr 14, 2017

Unfortunately, this is also not working for me.

I updated `inappbrowser to the latest:

cordova-plugin-inappbrowser 1.7.0 "InAppBrowser"

Which should include this change, but the file input is still not opening the native file browser.

The same code works fine in iOS (#139).

I'm wondering if I'm missing something, maybe a conflicting plugin? I'm testing it through the PhoneGap Android version, can it be a thing?

@phstc

This comment has been minimized.

Copy link

phstc commented Apr 14, 2017

I'm testing it through the PhoneGap Android version, can it be a thing?

That was it.

It's working fine after building the app. The issue was with the PhoneGap app, which is probably not running the latest.

@darron1217 darron1217 deleted the darron1217:CB-9148 branch Apr 18, 2017

DenisAtFourth added a commit to Fourth-Engage/cordova-plugin-inappbrowser that referenced this pull request Jun 7, 2017

@sbhandary

This comment has been minimized.

Copy link

sbhandary commented Jun 29, 2017

Hello,

I am still having the issue with the android. Is this issue fixed/ included in the cordova?

@andersborgabiro

This comment has been minimized.

Copy link

andersborgabiro commented Jun 29, 2017

It's included in cordova-plugin-inappbrowser 1.7.0. Note though that it will not work on certain Android versions, notably 4.4.2, as shown below. I don't know about before that. On a Samsung S3 with 4.3 I got something like "There's no associated app...", which is something else. I use PhoneGap Build and it invokes 1.7.1 of the plugin.

Hopefully someone knows what versions the fix works on.


_@markusjwetzel I've just tested it on Android 4.4 and as you said, it isn't working...
I'll fix it ASAP :)

--> I realized that Android removed the openfilechooser function from Kitkat 4.4.2 which is version that I've tested...
Currently, there is no solution for Kitkat 4.4.2 :(_

de-dan pushed a commit to de-dan/cordova-plugin-inappbrowser that referenced this pull request Sep 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.