Closed
Bug 1160279
Opened 9 years ago
Closed 9 years ago
[e10s] Dragging a file into an e10s tab doesn't load that file
Categories
(Core :: DOM: Events, defect)
Tracking
()
RESOLVED
FIXED
mozilla40
People
(Reporter: mstange, Assigned: mossop)
References
Details
Attachments
(1 file)
1.44 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
Steps to reproduce: 0. Have an e10s tab open, for example this bug. 1. Open a Finder window of a directory that contains html files. 2. Drag such an html file over into the tab content area. Expected results: The html page should be loaded in the tab. Actual results: Nothing happens.
Comment 1•9 years ago
|
||
WFM on Linux, and I was testing also on Windows when writing the initial dnd support for e10s. I don't have access to any OSX devices.
Comment 2•9 years ago
|
||
In other words, I'm hoping someone could help here ;)
Updated•9 years ago
|
tracking-e10s:
--- → m8+
Assignee | ||
Comment 3•9 years ago
|
||
This annoys the heck out of me so I'll try to take a quick look.
Flags: needinfo?(dtownsend)
Comment 4•9 years ago
|
||
(In reply to Dave Townsend [:mossop] from comment #3) > This annoys the heck out of me so I'll try to take a quick look. Hi Dave, If you don't mind, I would like to share my finding here. It seems the problem is at [1]. In content process, I think the type of file is not nsIFile, it's FileImpl. We just need to find a way to get the correct url from FileImpl. [1] https://dxr.mozilla.org/mozilla-central/source/dom/base/contentAreaDropListener.js#46
Assignee | ||
Comment 5•9 years ago
|
||
So yeah the problem is that when creating the drag session we convert everything to things that can be sent over IPC, for nsIFiles this means FileImpls. We then never convert back so contentAreaDropListener doesn't know what to do. I have no idea why this apparently works in Windows and Linux, it shouldn't from what I see. There are two options. One is to convert contentAreaDropListener (and any other code that handles dropping of files) to use dataTransfer.getFiles (which doesn't look like it is exposed to JS) and find some way to get a url from a File (I can't see one right now). This is probably the right long-term solution given sandboxing. The alternative is just to convert FileImpls back to nsIFiles when setting up the content side drag session. This patch does that and seems like a good quick fix. What do you think smaug?
Flags: needinfo?(dtownsend)
Attachment #8602377 -
Flags: review?(bugs)
Comment 6•9 years ago
|
||
(In reply to Dave Townsend [:mossop] from comment #5) > I have no idea why this apparently works in Windows and > Linux, it shouldn't from what I see. We have totally crazy setup for dnd. Platform specific code all over, and the best example is that Windows backend relies on certain OSX specific "flavor" to be in the transferable, but doesn't actually use it for anything. At least on Linux dnd code tends to rely on urls, not files, IIRC. Will look at the patch after lunch or so. DataTransfer::GetFiles() is very much exposed to scripts: http://mxr.mozilla.org/mozilla-central/source/dom/webidl/DataTransfer.webidl#28
Comment 7•9 years ago
|
||
Comment on attachment 8602377 [details] [diff] [review] patch Could you perhaps just make contentAreaDropListener to use normal blobs, or maybe event dataTransfer.files? I guess we should convert dom::FileImpl to dom::File in DataTransfer::MozGetDataAt, and then contentAreaDropListener could just use normal DOM File, and not nsIFile. DataTransfer::GetFiles does similar conversion. DOM File has mozFullPath for chrome JS usage, http://mxr.mozilla.org/mozilla-central/source/dom/webidl/File.webidl "application/x-moz-file" is rarely used, and looks like in case of dnd only contentAreaDropListener in e10s/childprocess. (I think for sandboxing purposing using raw nsIFile in child process may become a bit difficult.)
Attachment #8602377 -
Flags: review?(bugs) → review-
Comment 8•9 years ago
|
||
Comment on attachment 8602377 [details] [diff] [review] patch Sounds like we don't have any sane way to convert path to URI without nsIFile, so let's take this for now.
Attachment #8602377 -
Flags: review- → review+
Assignee | ||
Comment 9•9 years ago
|
||
Filed bug 1162775 for the follow-up work. https://hg.mozilla.org/integration/fx-team/rev/b62e2d74176f
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/b62e2d74176f
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Assignee | ||
Comment 11•9 years ago
|
||
Backed out for causing bug 1163375. Will do the real fix in bug 1162775.
Depends on: 1162775
Assignee | ||
Comment 12•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/8d78c47be72c
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 13•9 years ago
|
||
Fixed by bug 1162775
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
QA Whiteboard: [good first verify][verify in Nightly only]
You need to log in
before you can comment on or make changes to this bug.
Description
•