Fix #33 Support Android file upload #60

andreipfeiffer · 2018-09-25T12:30:21Z

Fixes #33

I could really use some help from an Android developer on this one, because I just "made it work", don't know how to "make it work good".

Some things that should be reviewed:

validate Android 5.0 devices (my emulator work, but outputs some weird sounds; a Galaxy 4 I tested on crashes)
validate Android 5.1 devices (emulator works, couldn't find a real device)
how to handle File Extensions? (https://www.w3schools.com/tags/att_input_accept.asp)

I'm sure that there's more refactoring to be done, so any help and advice would be appreciated.

- supports photo and video capture - supports the accept attribute - supports the multiple attribute

Titozzz · 2018-09-25T16:55:25Z

Thank you for the fast work 🚀 ! I've linked your PR to a few people to get more reviews so we can get this right

jamonholmgren · 2018-09-25T16:57:03Z

I'm out of town until this weekend and won't have time until then -- but will take a look when I can!

Titozzz · 2018-09-25T17:04:10Z

So about the FileProvider, it's great I think as it is the recommended way to do that by Google.

I'm not an android expert to judge the rest, but it seems fine to me.

One thing tho: I know that the code you used is only working on Android 5 or more, that is fine to me, as supporting older versions means using unofficial/undocumented APIs, but we should document that.

Titozzz · 2018-09-25T17:06:36Z

@lucasferreira Can you look at this ?

lucasferreira · 2018-09-25T18:44:20Z

Hi @Titozzz,

I my soft experience when I implemented the same support in react-native-android-webview it's that you never could "cover" all the Android devices around there. This could be something to warn in a future documentation about this file uploading feature.

But I will try to look and help with the @andreipfeiffer good work ;)

andreipfeiffer · 2018-09-25T18:49:01Z

@Titozzz It's true that I haven't tested on versions older than 5, but from my best guess, it should work (or more precisely, we could implement support), maybe with fewer features.

Currently, there's a @TargetApi(Build.VERSION_CODES.LOLLIPOP) set on the createViewInstance() method in the RNCWebViewManager that was there previously, which is preventing the onShowFileChooser() also.

andreipfeiffer · 2018-09-25T18:52:03Z

In regard to the FileProvider which was added in sdk 22, I have no idea what happens on sdk 21, which is LOLLIPOP 5.0. I'll have to check that.

lucasferreira · 2018-09-25T18:56:51Z

By the comments of @Titozzz Android 5+ could be a requirement to use that "featured"

But here we have a "sort of" implementation for Android 4.1+ without multifile support.

Titozzz · 2018-09-26T07:47:12Z

android/src/main/java/com/reactnativecommunity/webview/RNCWebViewManager.java

@@ -413,6 +414,10 @@ public boolean onConsoleMessage(ConsoleMessage message) {
      public void onGeolocationPermissionsShowPrompt(String origin, GeolocationPermissions.Callback callback) {
        callback.invoke(origin, true, false);
      }
+
+      public boolean onShowFileChooser(WebView webView, ValueCallback<Uri[]> filePathCallback, FileChooserParams fileChooserParams) {


This does not gets triggered below 5.0 (not only because of the @targetAPI but also cause this was implemented in 5.0)

There are others function signature that allow unofficial support but I'm not sure there is a point to support this below 5.0

4.1-4.4 (SDK 16-19) which means JellyBean & KitKat cover more than 10% of market share.
If it's easy to cover that, I think it's definitely worth it.
I would like to give it a shot, at least.

Titozzz · 2018-09-26T07:49:10Z

If we were to support that feature only on 5.0+ devices that would be fine to me.

We need to verify however that we can still use the webview as before on older devices

andreipfeiffer · 2018-09-26T11:41:29Z

@Titozzz I could take a look on how to support 4.1+. Since RN sets minSdkVersion to 16 by default, I guess users would expect that for any community package as well.

The only question is that if you think I should tackle this on this PR, or not.
Since this is not production ready, I don't see an issue to merge this PR and add support for older SDKs later.

lucasferreira · 2018-09-26T12:07:18Z

The worst thing about 4.1+ old API it's that we can't have support for multifile html5 inputs.

And @andreipfeiffer it's right, RN minSdkVersion it's 16 and I really don't know if it good to support (or not) older Android versions bellow 5.0.

Titozzz · 2018-09-26T13:04:00Z

You could make it work for 4.1-4.3 but never for 4.4, and that would mean using undoc APIs so I'm not that hyped about it.

We are trying to keep that repo as clean as possible even if it is PRE-release as it is already linked by the 0.57 changelog and some people already use it

lucasferreira · 2018-09-26T13:22:09Z

So we can point that this feature will work only in Android 5+ and we could provide some method to check file input upload support?

Something like this.refs.myWebView.supportsFileUpload()?

With that the developers could test if the android version supports or not the file upload feature and warn (or adapt) his webview use to that.

What do you guys think?

Titozzz · 2018-09-26T13:23:01Z

That is interesting @lucasferreira

jamonholmgren · 2018-09-27T16:55:26Z

@lucasferreira I would support that.

andreipfeiffer · 2018-09-27T16:59:01Z

I would implement basic file upload (without multiple) for sdk 16+ and progressively enhance it with supported features: accept attr, multiple attr.

Android 4.1-4.4 (SDK 16-19) which means JellyBean & KitKat, cover more than 10% of market share.
If it's easy to cover that, I think it's definitely worth it.

I'll try to do that these days.

Titozzz · 2018-09-27T21:44:27Z

@andreipfeiffer I really believe there is no way to get 4.4 to work and without this 4.1 to 4.3 has a really low market share.

I'd go with 5+ only and @lucasferreira suggestion.

However if you guys prefer the other way that's fine by me but we need to document the compatibility features for <5 so that once react-native drops support for Android < 5 migration is easy

andreipfeiffer · 2018-09-27T21:56:08Z

I'm fine with either approach.

@Titozzz @lucasferreira My knowledge is kinda limited, so could you please explain why is it not possible to implement a basic file upload for 4.4?

Titozzz · 2018-09-28T07:14:07Z

delight-im/Android-AdvancedWebView#4 (comment)

andreipfeiffer · 2018-09-28T07:32:13Z

Woaw, didn't know about that. Thanks for the insight @Titozzz
In that case, I would support @lucasferreira 's proposal also, which 2 things to clarify:

how should that method be implemented? by verifying the version? or is feature detection possible?
how should we name the method? I would prefix it with something that suggests a boolean value, like isFileUploadSupported()

Titozzz · 2018-09-28T13:48:41Z

I would not bother and just check the version.
I like prefixing bool values with is too.
Do not forget to add it on iOS too then

lucasferreira · 2018-09-28T14:39:37Z

I agree with you guys @Titozzz and @andreipfeiffer.

Good method name isFileUploadSupported and check SDK_VERSION for a simple comparison.

andreipfeiffer · 2018-09-29T08:06:13Z

Will look into it in the following days.

jamonholmgren · 2018-09-29T18:11:21Z

On iOS it would just return true, right?

jamonholmgren · 2018-09-29T18:15:39Z

Really appreciate your work on this, @andreipfeiffer !!! ❤️

andreipfeiffer · 2018-11-19T10:16:32Z

@jamonholmgren @Titozzz
I have updated the first comment of the PR.
My main 2 concerns are in regard to Android 5.0 & Android 5.1.

I agree that we should release this as is.
It's not perfect, but it's more than what's currently available.

vitorreis · 2018-11-19T11:08:00Z

Me waiting for this merge

android/src/main/java/com/reactnativecommunity/webview/RNCWebViewModule.java

# [2.10.0](v2.9.0...v2.10.0) (2018-11-21) ### Features * **Android:** Support Android file upload ([#60](#60)) ([752a5b2](752a5b2)), closes [#33](#33)

Titozzz · 2018-11-21T10:47:59Z

🎉 This PR is included in version 2.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Titozzz · 2019-01-18T17:03:20Z

@allcontributors[bot] please add @andreipfeiffer for code, review, ideas

allcontributors · 2019-01-18T17:03:28Z

@Titozzz

I've put up a pull request to add @andreipfeiffer! 🎉

Not sure what it was like when first implemented in react-native-webview#60, but currently if user chooses multiple files to upload, only the first one will actually be uploaded. The reason behind this is that when mulitple files are selected, both `getData` and `getClipData` will not be null, but `getData` only contains the first of those selections. Reordering file selection logic to consider the multiple file case first.

Not sure what it was like when first implemented in react-native-webview#60, but currently if user chooses multiple files to upload, only the first one will actually be uploaded. The reason behind this is that when mulitple files are selected, both `getData` and `getClipData` will not be null, but `getData` only contains the first of those selections. Reordering file selection logic to consider the multiple file case first. (cherry picked from commit 7920ccb)

Fixes react-native-webview#33 I could really use some help from an Android developer on this one, because I just "made it work", don't know how to "make it work good". Some things that should be reviewed: - [ ] validate Android 5.0 devices (my emulator work, but outputs some weird sounds; a Galaxy 4 I tested on crashes) - [ ] validate Android 5.1 devices (emulator works, couldn't find a real device) - [ ] how to handle File Extensions? (https://www.w3schools.com/tags/att_input_accept.asp) I'm sure that there's more refactoring to be done, so any help and advice would be appreciated.

# [2.10.0](react-native-webview/react-native-webview@v2.9.0...v2.10.0) (2018-11-21) ### Features * **Android:** Support Android file upload ([react-native-webview#60](react-native-webview#60)) ([752a5b2](react-native-webview@752a5b2)), closes [react-native-webview#33](react-native-webview#33)

Not sure what it was like when first implemented in react-native-webview#60, but currently if user chooses multiple files to upload, only the first one will actually be uploaded. The reason behind this is that when mulitple files are selected, both `getData` and `getClipData` will not be null, but `getData` only contains the first of those selections. Reordering file selection logic to consider the multiple file case first. (cherry picked from commit 7920ccb)

# [2.10.0](react-native-webview/react-native-webview@v2.9.0...v2.10.0) (2018-11-21) ### Features * **Android:** Support Android file upload ([#60](react-native-webview/react-native-webview#60)) ([752a5b2](react-native-webview/react-native-webview@752a5b2)), closes [#33](react-native-webview/react-native-webview#33)

andreipfeiffer added 2 commits September 25, 2018 15:07

Add support for File Upload on Android

52d2d38

- supports photo and video capture - supports the accept attribute - supports the multiple attribute

Update README: Remove unneeded permission

c6114f3

andreipfeiffer mentioned this pull request Sep 25, 2018

MIGRATED: Input type="file" doesn't work in WebView Android (#16869) #11

Closed

Titozzz mentioned this pull request Sep 25, 2018

New cross-platform WebView package: react-native-webview lucasferreira/react-native-webview-android#112

Open

Titozzz reviewed Sep 26, 2018

View reviewed changes

Merge branch 'master' into feature/android-file-upload

a8ca0ca

Titozzz reviewed Nov 19, 2018

View reviewed changes

android/src/main/java/com/reactnativecommunity/webview/RNCWebViewModule.java Show resolved Hide resolved

Merge branch 'master' into feature/android-file-upload

e356dd1

Titozzz approved these changes Nov 21, 2018

View reviewed changes

Titozzz merged commit 752a5b2 into react-native-webview:master Nov 21, 2018

Titozzz pushed a commit that referenced this pull request Nov 21, 2018

chore(release): 2.10.0 [skip ci]

f79b713

# [2.10.0](v2.9.0...v2.10.0) (2018-11-21) ### Features * **Android:** Support Android file upload ([#60](#60)) ([752a5b2](752a5b2)), closes [#33](#33)

Titozzz added the released label Nov 21, 2018

This comment has been minimized.

Sign in to view

allcontributors bot mentioned this pull request Jan 18, 2019

docs: add andreipfeiffer as a contributor #256

Merged

TheAlmightyBob mentioned this pull request Nov 24, 2022

[android]Why does it read clipboard every time when I open an app with react-native-webview? #2757

Closed

Fix #33 Support Android file upload #60

Fix #33 Support Android file upload #60

Conversation

andreipfeiffer commented Sep 25, 2018 • edited

Titozzz commented Sep 25, 2018

jamonholmgren commented Sep 25, 2018

Titozzz commented Sep 25, 2018

Titozzz commented Sep 25, 2018

lucasferreira commented Sep 25, 2018

andreipfeiffer commented Sep 25, 2018

andreipfeiffer commented Sep 25, 2018

lucasferreira commented Sep 25, 2018 • edited

Titozzz Sep 26, 2018 • edited

Choose a reason for hiding this comment

andreipfeiffer Sep 27, 2018

Choose a reason for hiding this comment

Titozzz commented Sep 26, 2018

andreipfeiffer commented Sep 26, 2018

lucasferreira commented Sep 26, 2018

Titozzz commented Sep 26, 2018

lucasferreira commented Sep 26, 2018

Titozzz commented Sep 26, 2018

jamonholmgren commented Sep 27, 2018

andreipfeiffer commented Sep 27, 2018 • edited

Titozzz commented Sep 27, 2018

andreipfeiffer commented Sep 27, 2018

Titozzz commented Sep 28, 2018

andreipfeiffer commented Sep 28, 2018

Titozzz commented Sep 28, 2018 • edited

lucasferreira commented Sep 28, 2018

andreipfeiffer commented Sep 29, 2018

jamonholmgren commented Sep 29, 2018

jamonholmgren commented Sep 29, 2018

andreipfeiffer commented Nov 19, 2018

vitorreis commented Nov 19, 2018

Titozzz commented Nov 21, 2018

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Titozzz commented Jan 18, 2019

allcontributors bot commented Jan 18, 2019

andreipfeiffer commented Sep 25, 2018 •

edited

lucasferreira commented Sep 25, 2018 •

edited

Titozzz Sep 26, 2018 •

edited

andreipfeiffer commented Sep 27, 2018 •

edited

Titozzz commented Sep 28, 2018 •

edited