1293949 - Can't drag and drop folders to upload on dropbox

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Jake]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160726073904

Steps to reproduce:

Drag and drop a folder onto dropbox.com.


Actual results:

nothing, it won't upload.


Expected results:

The folder should be uploaded.
I thought dropbox didn't have this feature for a long time untill I tried it with Chrome today.

Comment 1

[Tooru Fujisawa [:arai]]

Looks like a part of bug 1188880.

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Tooru Fujisawa [:arai] from comment #1)
> Looks like a part of bug 1188880.

Hey :baku, what do you think?

Comment 3

[Andrea Marchesini [:baku]]

Yes. this is a duplicate.

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

Is this happening with Nightly?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

And is this really a dup of bug 1188880. That one is about the upload API proposal.
Nightly and Aurora should have already the Entries API and webkitdirectory..


I don't have dropbox account. Anyone willing to test?

Comment 6

[Olli Pettay [:smaug][[e-mail]]]

David, perhaps you could test latest Nightly or Aurora (assuming I recall correctly your current employer)?

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

So, reopening, since this certainly isn't a dup of bug 1188880.
Possibly depending on bug 1288683.

Comment 8

[David Zbarsky (:dzbarsky)]

(In reply to Olli Pettay [:smaug] from comment #6)
> David, perhaps you could test latest Nightly or Aurora (assuming I recall
> correctly your current employer)?

Sorry for the delay here, I filter bugmail pretty agressively now and this one didn't make it through. In the future, feel free to email my work accounts for things of this nature (last name at dropbox)

I'll take a look on Tuesday!

Comment 9

[Andrea Marchesini [:baku]]

I contacted some of the dropbox developers and they will help me with this issue. I'll give update at the end of September.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Hopefully dz could push this in dropbox a bit faster.

Comment 11

[David Zbarsky (:dzbarsky)]

OK, took a look at it. Note that reporter is on Windows and I'm on Mac, so there may be differences.
Dragging a folder from Finder into a folder on the dropbox website works correctly. Also, dropping it on the page outside of the file/folder list works correctly, uploading it to your top-level folder. The individual files in your toplevel folders are not really drop targets, and dropping on there uploads to the closest folder - one could argue that this isn't expected, but the interface does make it clear what will happen and I certainly wouldn't categorize that as "nothing, it won't upload". Chrome exhibits the same exact behavior.

I am testing a new homepage that hasn't shipped yet, but I think the these bits are shared. I will also try testing what we're shipping, just need to figure out how to opt myself out.

Comment 12

[David Zbarsky (:dzbarsky)]

(In reply to Andrea Marchesini [:baku] from comment #9)
> I contacted some of the dropbox developers and they will help me with this
> issue. I'll give update at the end of September.

Who did you contact? I can certainly get things moving faster if there's an actual bug that we can reproduce

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

(and btw, we're aiming to ship webkitdirectory and Entries API in FF50.)

Comment 14

[Andrea Marchesini [:baku]]

> Who did you contact? I can certainly get things moving faster if there's an
> actual bug that we can reproduce

Lennart Jansson and Mindy Zhang. And I was testing the dropbox website with nightly on linux and it was not working.

Comment 15

[Andrea Marchesini [:baku]]

Attachment: serializable.patch

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8794118 [details] [diff] [review]
serializable.patch

honza, this is rather high priority bug. Could you please take a look, or forward to someone who knows about this stuff.

And baku, please explain what is happening here and why this is the right patch :)

Comment 17

[Andrea Marchesini [:baku]]

The reason why this patch is needed is because in ::Serialize() we don't consider the DEFER open. This is done everywhere else (Seek, Tell, Available, Read, and so on), but not here. For this reason mFD is null, but mDeferredOpen is true.
This simple fix does the DoPendingOpen() also for ::Serialize(). Without this we send a wrong FD to the parent process, the HttpChannel fails and the XHR will fail as well.

Comment 18

[Honza Bambas (:mayhemer)]

Comment on attachment 8794118 [details] [diff] [review]
serializable.patch

Review of attachment 8794118 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/base/nsFileStreams.cpp
@@ +578,5 @@
>                               FileDescriptorArray& aFileDescriptors)
>  {
>      FileInputStreamParams params;
>  
> +    if (NS_SUCCEEDED(DoPendingOpen()) && mFD) {

nit: maybe keep the form as:

{
  rv = DoPendingOpen();
  NS_ENSURE_SUCCESS(rv, rv);

  do stuff..
}

Comment 19

[Andrea Marchesini [:baku]]

> nit: maybe keep the form as

I cannot. this function doesn't return a value and the FileInputStreamParams must be populated also if an error occurs.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c0ca8b0d966e
nsFileInputStream must do the pending open step before been serialized, r=mayhemer

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

[Tracking Requested - why for this release]:
Based on the feedback from baku, with the patch dnd would work with dropbox both with and without e10s.

Comment 22

[Andrea Marchesini [:baku]]

Comment on attachment 8794118 [details] [diff] [review]
serializable.patch

Approval Request Comment
[Feature/regressing bug #]: IPC, e10s
[User impact if declined]: XHR in content process, when sending real-file Blobs, will fail because underlying nsIInputStream is not correctly opened. This fix is important for shipping Entries API: Google Drive, Dropbox and other important websites depend on this API.
[Describe test coverage new/current, TreeHerder]: green on try.
[Risks and why]: none
[String/UUID change made/needed]: none.

Comment 23

[Marcia Knous [:marcia]]

Tracking 52+ as this is a fairly common operation.

Comment 24

[Gerry Chang [:gchang]]

Track 51+ as this is a common operation for users.

Comment 25

[Iris Hsiao [:ihsiao]]

https://hg.mozilla.org/mozilla-central/rev/c0ca8b0d966e

Comment 26

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8794118 [details] [diff] [review]
serializable.patch

Adding a support for a pretty common scenario, Aurora51+, Beta50+

Comment 27

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/92d639e21ef4

Comment 28

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/68b9c9dc4612

Comment 29

[Bogdan Maris, Desktop Test Engineering]

Reproduced the initial issue using Firefox 48.0 RC, verified that the issue does not reproduce anymore using Firefox 50 beta 2, latest Developer Edition 51.0a2 and latest Nightly 52.0a1 across platforms (Windows 10 64-bit, Mac OS X 10.10.5 and Ubuntu 16.04 32-bit).