Closed Bug 203307 Opened 17 years ago Closed 13 years ago

Implement drag/drop via IStream interface

Categories

(Core :: DOM: Drag & Drop, defect)

x86
Windows 2000
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: nisheeth_mozilla, Assigned: ykovalchuk)

References

Details

(Keywords: helpwanted)

Attachments

(2 files, 2 obsolete files)

See bug 97413 for background info.

Implement drag drop via the
CF_FILEDESCRIPTOR and CF_FILECONTENT clipboard formats and pass the file's data
to the drop target through the IStream interface.  We'll need to serve the 
dragged file through a Necko stream to an IStream that the drop target will 
pull its data from.
cc'ing myself. We need this now that Bug #83803 is fixed which added the ability
to drag mail attachments to the desktop. 

This fails miserably for imap where we have to wait for the attachment to be
downloaded as we spool it to the temp file. If the user releases the drop, the
OS trys to copy the temp file while we are busy writing to it. 

The solution proposed here is how 4.x worked and that will allow things to work
for imap drag and drop.
Blocks: 230455
Following Clipboard (Drag&Drop) Formats should be supported:

"FileGroupDescriptor"
"FileContents"
Feel free to take back if you are still interested in doing this Nisheeth...
Assignee: nisheeth_mozilla → mscott
Keywords: helpwanted
Attached file Suggestion on IStream implementation. (obsolete) —
Hi all!

I've searche alot before found (thanks to Ere Maijala) this bug report. It
seems like it is the only possible solution for large files. Also if we use
current implementation of nsDragService::StartInvokingDragSession (where
::DoDragDrop gets called) it won't help much. By default drag and drop
operation on windows is synchronous, only on Windows 2000 an attempt was made
to fix that issue. Microsoft provided IAsyncOperation interface that must be
exposed by data object to have data extraction take place on a separate thread.

I'm currently working on implementing file dragging from Mozilla to Windows
file system and have somewhat working implementation of an IStream interface
that solves the issue, i do not want to post it yet because it is not completly
functional. The only method that needs implemntation is IStream::Read - as I've
figured Shell does not call any other methods. And I think there is no need to
implement them either since the only purpose of this object is to download data
from internet to user's computer...(or something like that).
So if anyone is still working on this bug please contact me so that we could
find some good solution to this problem.

Thank you!
Yuri Kovalchuk
Blocks: 267426
Assignee: mscott → nobody
QA Contact: pmac
Attached patch Implements dnd via IStream (obsolete) — Splinter Review
This is basically the same patch as my latest on for bug 267426 but with definitions for IAsyncOperation and related stuff. This should also fix mingw build failure (bug 344504).
Attachment #233462 - Flags: superreview?(roc)
Attachment #233462 - Flags: review?
Comment on attachment 233462 [details] [diff] [review]
Implements dnd via IStream

ok, cool
Attachment #233462 - Flags: superreview?(roc)
Attachment #233462 - Flags: superreview+
Attachment #233462 - Flags: review?
Attachment #233462 - Flags: review+
Whiteboard: [checkin needed]
Assignee: nobody → kovalchuk77
Comment on attachment 233462 [details] [diff] [review]
Implements dnd via IStream

mozilla/widget/src/windows/nsDataObj.h 	1.36
mozilla/widget/src/windows/nsDataObj.cpp 	1.82
mozilla/widget/src/windows/nsDragService.cpp 	1.52
mozilla/widget/src/windows/nsDragService.h 	1.27
Attachment #233462 - Attachment is obsolete: true
help with testing please
Assignee: kovalchuk77 → nobody
Whiteboard: [checkin needed]
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060815 Minefield/3.0a1

Tested this with a 4.5mb image file (steves-digicams.com), and I was able to drag and drop onto the desktop and also a removable drive after clearing the cache.

Letting it simmer because at least one more person experienced a crash while testing this.
This seems to have given me the following build error on WinXP SP2/MinGW/cygwin

e:/mozilla/source/mozilla/widget/src/windows/nsDragService.cpp: In member functi
on `PRUint64 nsDragService::GetShellVersion()':
e:/mozilla/source/mozilla/widget/src/windows/nsDragService.cpp:519: error: `majL
L' was not declared in this scope
e:/mozilla/source/mozilla/widget/src/windows/nsDragService.cpp:519: error: `minL
L' was not declared in this scope
e:/mozilla/source/mozilla/widget/src/windows/nsDragService.cpp:517: warning: unu
sed variable 'maji64'
e:/mozilla/source/mozilla/widget/src/windows/nsDragService.cpp:518: warning: unu
sed variable 'mini64'
e:/mozilla/source/mozilla/widget/src/windows/nsDragService.cpp:519: warning: unu
sed variable 'majLL'
e:/mozilla/source/mozilla/widget/src/windows/nsDragService.cpp:519: warning: unu
sed variable 'minLL'
make[6]: *** [nsDragService.o] Error 1
Attachment #162997 - Attachment is obsolete: true
Attachment #233927 - Flags: review?(dgk)
Yup, seems to work fine.
Attachment #233927 - Flags: review?(dgk) → review+
This if against MOZILLA_1_8_BRANCH, please r & sr.
How do I make it go into MOZILLA_1_8_BRANCH?
What are these approval1.8.0.7, approval1.8.0.8 and approval1.8.1? Do I need them at all?
Looks like too many questions... oh well. At least help with the first one please.
Attachment #234197 - Flags: superreview?(roc)
Attachment #234197 - Flags: review?(roc)
You would need to request approval1.8.1 when it has been reviewed. I don't know what the chances are to get it on the 1.8 branch though since it's a quite big patch. When requesting approval, outline what the patch is needed for, what possible risk is involved with this patch and what testing is needed for verify the bug fix (if needed).
Comment on attachment 234197 [details] [diff] [review]
Patch for MOZILLA_1_8_BRANCH

rubber-stamp since this is very much the same as the trunk patch
Attachment #234197 - Flags: superreview?(roc)
Attachment #234197 - Flags: superreview+
Attachment #234197 - Flags: review?(roc)
Attachment #234197 - Flags: review+
I don't think we're going to want this on the 1.8 branch for FF2 at this stage, probably, but you can request blocking status for this bug if you like. We definitely won't want this for 1.8.0 branch.
Attachment #234197 - Flags: approval1.8.1?
Comment on attachment 234197 [details] [diff] [review]
Patch for MOZILLA_1_8_BRANCH

Its too late at this time to take large patches to provide new implementions or new features.
Attachment #234197 - Flags: approval1.8.1? → approval1.8.1-
Assignee: nobody → kovalchuk77
Is bug 336337 the only reason this hasn't been checked in and marked FIXED?
(In reply to comment #19)
It is checked in on trunk. I tried to get it to Firefox 2.0 (MOZILLA_1_8_BRANCH) but it was too late for a patch this big :(
Timeless checked this in a while ago, but I forgot to mark it fixed, doing it now.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
After a "bake" period, can it block an update on 2.0 such as 2.0.1?
Blocks: 245861
This patch looks a bit risky for the 1.8 branch, but we would still like to fix bug 245861.  Can we come up with a simpler patch to address the issue in that bug?
Comment on attachment 234197 [details] [diff] [review]
Patch for MOZILLA_1_8_BRANCH

This has now been baking on the trunk since 08/15. Do we feel the risk level is better or still too risky? 

It fixes Firefox Bug 245861 which there has been interest in getting fixed on the branch.

It also makes drag and drop of mail attachments  to the desktop in Thunderbird work much better on Windows. Currently you have to hold the action until we finish downloading the attachment then you can release the drop. With this patch you can drop it right away and we stream it to the destination. Hence my interest in asking again :).
Attachment #234197 - Flags: approval1.8.1.3?
Comment on attachment 234197 [details] [diff] [review]
Patch for MOZILLA_1_8_BRANCH

approved for 1.8.1.4, a=dveditz for release-drivers
Attachment #234197 - Flags: approval1.8.1.4? → approval1.8.1.4+
Whiteboard: [checkin needed (1.8 branch)]
Comment on attachment 234197 [details] [diff] [review]
Patch for MOZILLA_1_8_BRANCH

mscott has gotten cold feet on this, removing approval1.8.1.4 for now while trunk regressions are investigated.
Attachment #234197 - Flags: approval1.8.1.4+ → approval1.8.1.4-
Whiteboard: [checkin needed (1.8 branch)]
(In reply to comment #26)
> trunk regressions are investigated.

This could be related to bug #358657 perhaps. I used to investigate this issue. Hope this helps.

Duplicate of this bug: 376964
Veditz in comment #26
> (From update of attachment 234197 [details] [diff] [review])
> mscott has gotten cold feet on this, removing approval1.8.1.4 for now while
> trunk regressions are investigated.

what bug# are thought to be regressions?
based on comment 1 this might also fix bug 253711, bug 383893, bug 283452, bug 384590
Depends on: 440911
(In reply to comment #30)
> based on comment 1 this might also fix bug 253711, bug 383893, bug 283452, bug
> 384590

I don't think attachment 234197 [details] [diff] [review] fixes these bugs. These bugs need the patch in https://bugzilla.mozilla.org/show_bug.cgi?id=358657#c16 too.
You need to log in before you can comment on or make changes to this bug.