Closed Bug 1456557 Opened 3 years ago Closed 6 months ago

Multiple files won't upload after selecting for HTML input element

Categories

(Firefox for Android Graveyard :: General, defect, P3)

Firefox 61
All
Android
defect

Tracking

(fennec?, firefox61 wontfix, firefox63 wontfix, firefox64 wontfix, firefox65 wontfix, firefox66 wontfix, firefox67 affected)

RESOLVED WORKSFORME
Tracking Status
fennec ? ---
firefox61 --- wontfix
firefox63 --- wontfix
firefox64 --- wontfix
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- affected

People

(Reporter: andrei.a.lazar, Assigned: andrei.a.lazar, NeedInfo)

References

()

Details

(Keywords: parity-chrome, Whiteboard: [priority:high][geckoview:fenix:p3])

Attachments

(1 file)

How to reproduce: 
1. open https://xianqiao.wang/test/firefox-file-upload/ in firefox for android
2. click upload
3. choose multiple file
Bogdan, can you please confirm this?

Expected result: output the files size
Actual result: throws an exception
Flags: needinfo?(bogdan.surd)
tracking-fennec: --- → ?
Flags: needinfo?(bogdan.surd)
Hardware: Unspecified → All
Version: unspecified → Firefox 61
Device:
 - Sony Xperia Z5 (Android 6.0.1)

I can confirm that this is happening when selecting multiple files. The page trows an exception.
In the documentation https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file it's said that:

A file input's value attribute contains a DOMString that represents the path to the selected file(s). If the user selected multiple files, the value represents the first file in the list of files they selected. The other files can be identified using the input's HTMLInputElement.files property.

I followed this trail but with no success on FilePicker.js, so at the moment, even though multiple files are selected, only one is going to be uploaded.
Mike we need some tech consultation on JavaScript on this - Andrei has tried to get the JS correct but he can’t get multiple files to upload. Can you take a look at his patch and see if you can advise on a fix?
Flags: needinfo?(michael.l.comella)
Attachment #8972571 - Flags: review?(sdaswani) → review?(michael.l.comella)
> In the documentation https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/file it's said that

I don't think you need to worry about `<input>`. FilePicker.js appears to pass on the files with `this.file` [1]:


    EventDispatcher.instance.sendRequestForResult(msg).then(file => {
      this._filePath = file || null;
      this._promptActive = false;

      if (!file) {
        return;
      }

      if (this._domWin) {
        return this._domWin.File.createFromNsIFile(this.file, { existenceCheck: false });
      }

      return File.createFromNsIFile(this.file, { existenceCheck: false });

Under the hood, `this.file` comes from `this_filePath` that we set earlier on in this function [2]:

  get file() {
    if (!this._filePath) {
        return null;
    }

    return new FileUtils.File(this._filePath);
  },

To support multiple files, I think parts of FilePicker.js needs to be rewritten to support multiple files. fwiw, there is also a `this.files` (which currently just wraps one `this.file`) but it doesn't seem we use it all the time.

NB: before you do that, I don't know what state FilePicker.js' callers expect it to end up in (e.g. does the function return multiple files? Or does the caller actually access the value via FilePicker.file?) and, if the callers expect just a single file, how hard it will be to change the caller. I'd recommend doing simple tests first (e.g. try to return 2 hard-coded file Uris) to try to get the code working end-to-end to ensure it's reasonable/feasible before you invest in doing more work on this component.

Unfortunately, Gecko code, especially ones shared with desktop, is hard-to-change and, in my experience, hard to follow the code base of. As such, I don't actually know who is calling FilePicker.js to be able to tell you more and figure out how difficult this actually is! x_x

---

fwiw, it's possible to get a debugger working in JS land, if it's helpful (e.g. to verify that the first function is getting all the right files). iirc, you can use remote debugging to start a debugger session on the main process and set breakpoints in the UI or with `debugger;` JS statements. These docs [3] detail setting up remote debugging for a web page but I think the process is the same. tbh, I find Firefox's WebIDE to be unpredictable so I'd actually recommend using Chrome's remote dev tools (they use the same protocol and work interchangeably).

[1]: https://searchfox.org/mozilla-central/rev/53afcfdbabed96883126d0ebbcac499b358e32f2/mobile/android/components/FilePicker.js#230
[2]: https://searchfox.org/mozilla-central/rev/53afcfdbabed96883126d0ebbcac499b358e32f2/mobile/android/components/FilePicker.js#130

[3]: https://developer.mozilla.org/en-US/docs/Tools/Remote_Debugging/Debugging_Firefox_for_Android_with_WebIDE
Flags: needinfo?(michael.l.comella)
Comment on attachment 8972571 [details]
Bug 1456557 - Multiple files won't upload after selecting for HTML input element. (WIP)

https://reviewboard.mozilla.org/r/241142/#review247700

I didn't go through this super thoroughly yet because it's missing the final solution but the general idea lgtm!

fwiw, this could be easier to review if it was broken apart into several commits that each did one thing. That being said, don't waste a lot of time making the commits – only do it if it's actually convenient! For example, a few chunks that I see that can be broken apart are:
- Modify onActivityResult to support multiple files
- Modify `*LoaderCallbacks` to support multiple files
- Modify `FilePicker.js` to support multiple files

If done in order, this shows where the data initially comes from (the Android Intent -> onActivityResult) and evenually ends up (-> LoaderCallbacks -> FilePicker.js), making it easier to understand the content that has been changed.

::: mobile/android/base/java/org/mozilla/gecko/FilePickerResultHandler.java:67
(Diff revision 1)
>      }
>  
>      @Override
>      public void onActivityResult(int resultCode, Intent intent) {
>          if (resultCode != Activity.RESULT_OK) {
> -            sendResult("");
> +            sendResult(new String[]{});

nit: iirc, generally spacing is `new String[] {}` or `new String[] { "some strings" }`. If checkstyle doesn't complain (I sent it to try), I don't think we should waste time changing it in these commits but I just wanted to let you know for future reference.

::: mobile/android/base/java/org/mozilla/gecko/FilePickerResultHandler.java:228
(Diff revision 1)
> -            this.cacheDir = cacheDir;
> -            this.tabId = tabId;
>          }
>  
>          @Override
>          public Loader<Cursor> onCreateLoader(int id, Bundle args) {

This is a much better solution, thanks for cleaning things up! :)

::: mobile/android/base/java/org/mozilla/gecko/FilePickerResultHandler.java:233
(Diff revision 1)
>          public Loader<Cursor> onCreateLoader(int id, Bundle args) {
>              final Context context = GeckoAppShell.getApplicationContext();
> +            Uri[] filesUris;
> +            Parcelable[] parcelables = args != null ? args.getParcelableArray(FILES_PATHS_BUNDLE_KEY) : null;
> +
> +            if (parcelables != null) {

I see a lot of this code is duplicated with the other loader – can we share any code?

::: mobile/android/base/java/org/mozilla/gecko/FilePickerResultHandler.java:263
(Diff revision 1)
>          @Override
>          public void onLoadFinished(Loader<Cursor> loader, Cursor cursor) {
> +            List<String> filesNames = new ArrayList<>();
>              if (cursor.moveToFirst()) {
> -                String fileName = cursor.getString(0);
> -
> +                do {
> +                    filesNames.add(cursor.getString(cursor.getColumnIndex(DATA)));

nit: cache `cursor.getColumnIndex` instead of calling it each time: it's caused perf problems in the past.
Comment on attachment 8972571 [details]
Bug 1456557 - Multiple files won't upload after selecting for HTML input element. (WIP)

https://reviewboard.mozilla.org/r/241142/#review247710

Please re-add r? when you've finished the commit (or NI if you have more questions)!
Attachment #8972571 - Flags: review?(michael.l.comella)
Whiteboard: [priority:high]
Status: NEW → ASSIGNED
There is very similar issue in Chrome. Please check it and see if it's helpful https://bugs.chromium.org/p/chromium/issues/detail?id=854425&can=2&start=0&num=100&q=&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Component%20Status%20Owner%20Summary%20OS%20Modified&groupby=&sort=

Could it be due to a bug in some of the Android apps like Photos?
Android 6.0.1 Firefox 61.0

using accept="image/*"

- can't select multiple files (by pressing long) in gallery
- can't select multiple files via google photos (known bug in google photos)


using accept="*.jpg, *.png" or no accept attribute

- can't select multiple files (by pressing long) when using documents => gallery
- can't select multilpe files via documents =>google photos (known bug in google photos)
- after selecting multiple files with  google photos the upload file is unusable

- can select multiple files (by pressing long) when using "documents" => recent files: BUT only one file is selected in input.files-array

Hope this helps? I know this bug for over 2 years now ...
Andrey are you still working on this? It would be great if this bug could get fixed.
Flags: needinfo?(andrei.a.lazar)
Yes, I postponed this just because Oreo migration has priority.
Assignee: andrei.a.lazar → nobody
Status: ASSIGNED → NEW
QA Contact: bogdan.surd
Assignee: nobody → andrei.a.lazar
QA Contact: bogdan.surd
Assignee: andrei.a.lazar → nobody
Assignee: nobody → andrei.a.lazar
Flags: needinfo?(andrei.a.lazar)
Priority: -- → P1
Chris, can you find someone to check if this also affects geckoview. 
It also doesn't sound like it's a regression, it's a new feature. So we might investigate a little further for fennec but not give it a top priority especially if it's fennec-only.
Priority: P1 → P2
I just checked in a GV-powered Fennec Custom Tab, and while I can check multiple files in the Android file picker, the page returns an error.
I'm not sure this is supported in Firefox or in Chrome, currently. The workaround is to upload one file at a time.
Priority: P2 → P3
(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #15)
> I'm not sure this is supported in Firefox or in Chrome, currently.

Do you mean that selecting multiple images is not supported in Chrome? My app works in Chrome 70 (and earlier) on Android, and Firefox desktop, but breaks in Firefox mobile.
Sending to GV triage based on Andreas's comment 14.
Keywords: parity-chrome
Whiteboard: [priority:high] → [priority:high][geckoview]
Whiteboard: [priority:high][geckoview] → [priority:high][geckoview:fenix:p3]

Is there a chance this gets picked up any time soon?

Flags: needinfo?(andrei.a.lazar)

Still having this problem with Firefox 66.0.2 on Android 8.1.0. Works fine with Chrome 73 on Android, and both Chrome and Firefox on desktop.

  • If you select multiple files, input.files will only have one file listed
  • If you attempt to use a FileReader to read the file contents, you get an exception with NS_ERROR_FILE_TARGET_DOES_NOT_EXIST
  • If you only select a single file, it shows up correctly and does not throw an exception when reading the file

Here is a codepen demonstrating the issue: https://codepen.io/anon/pen/jRZxmB

This is the exception on Android, captured using WebIDE with USB debugging:

https://i.imgur.com/mCgZnhx.png

NS_ERROR_FILE_TARGET_DOES_NOT_EXIST: pen.js:55
    <anonymous> Node
    forEach self-hosted:262
    <anonymous> Node
    _sendMessage jar:jar:file:///data/app/org.mozilla.firefox-HFFja-D3J4Q0co4WGNeWGw==/base.apk!/assets/omni.ja!/components/FilePicker.js:22

Still having this problem with Firefox 66.0.2 on Android. Please Fix It. Chrome on Android Works Fine.

Duplicate of this bug: 1546325

It's also not working for me on Firefox Mobile 67 and Android 9.
This is a big problem, because everytime i make a public folder for pictures etc. on my Nextcloud, i have to tell the people who use it to use Chrome.
I've not found any other solution for uploading multiple files at once.
Uploading every picture one by one is not really a solution.

Is there anything that i can do to help fixing this problem asap?

Still having this problem with Firefox 67.0.3 on Android 8.0.0.

Duplicate of this bug: 1579678

This bug is also in firefox68. I've got the same problem as @lrdarknesss

Looks like this works now.

Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → WORKSFORME

Nice it works now, but can i ask how and when was this fixed?

Some reference bug(s) and/or commit(s) at the time of closing/resolving would have been appreciated.

Flags: needinfo?(agi)

We completely rewrote the frontend of Firefox for Android, it would be really hard (if not impossible) to pin point what fixed this particular bug.

Flags: needinfo?(agi)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.