Drag and Drop support for E-Mails from Microsoft Outlook

NEW
Unassigned

Status

()

Core
Drag and Drop
--
enhancement
7 years ago
3 months ago

People

(Reporter: michael.schoendorfer, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [platform-rel-Microsoft][platform-rel-Outlook])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; de; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; de; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6

Firefox is not able to handle Drag and Drop of objects with the CFSTR_FILEDESCRIPTOR or CFSTR_FILECONTENTS format, so I cannot import E-Mails from Microsoft Outlook to a web application as I can do with Files from Windows Explorer.

Reproducible: Always

Steps to Reproduce:
1. Create an HTML document that handles the ondrop event and outputs all dropped files of event.datatransfer.
2. Open this document in Firefox.
3. Open Microsoft Outlook 2007.
4. Drag an E-Mail from Outlook to the HTML document in Firefox.
Actual Results:  
The ondrop event is triggered but event.datatransfer.files has no elements.

Expected Results:  
The ondrop event is triggered and event.datatransfer.files has as many elements as E-Mails were dropped. The filenames of these files are outputted.
(Reporter)

Updated

7 years ago
Version: unspecified → 3.6 Branch
(Reporter)

Comment 1

7 years ago
Created attachment 459348 [details]
Testcase
(Reporter)

Comment 2

7 years ago
Comment on attachment 459348 [details]
Testcase

This HTML document outputs the names of all files that are dropped onto the rectangle.
Attachment #459348 - Attachment description: This HTML document outputs the names of all files that are dropped onto the rectangle. → Testcase
(Reporter)

Comment 3

7 years ago
Created attachment 459755 [details] [diff] [review]
Patch to support Drag and Drop of E-Mails from Outlook

I added support for the CFSTR_FILEGROUPDESCRIPTOR and CFSTR_FILECONTENTS Clipboard Format.
When someone requests the dropped files from event.datatransfer.files and the filenames are not available via CF_HDROP, Firefox checks if virtual files are available via CFSTR_FILEGROUPDESCRIPTOR and CFSTR_FILECONTENTS. The contents are written to temporary files in %TMP% and the filenames are returned as the data.

The JavaScript-Program does not know the difference whether this are real files or objects from the Clipboard.

With this change it is possible to drag and drop E-Mails, Contacts, Notes, etc. from Outlook to a Webapplication as if they were files on the filesystem.
Attachment #459755 - Flags: review?(vladimir)
Comment on attachment 459755 [details] [diff] [review]
Patch to support Drag and Drop of E-Mails from Outlook

I'm going to bounce this over to Jim Mathies -- I believe he knows the D&D code better than I do..
Attachment #459755 - Flags: review?(vladimir) → review?(jmathies)

Comment 5

7 years ago
Comment on attachment 459755 [details] [diff] [review]
Patch to support Drag and Drop of E-Mails from Outlook

>+ if (S_OK == hres) {

Generally could we switch to using success macros with these where it applies? If not, lets at least start getting this code better in line with mozilla coding standard and set the comparison order right, so |if (hres != S_OK)| good, |if (S_OK == hres)| bad.


Also, in the default: block, could you try breaking that up into a set of functions or methods as much as possible? It's grown pretty wild and crazy.


>+ if (wcslen(tmpname) == 0) {

|if (!wcslen(tmpname))| or actually, maybe tighten this up a bit:

LPFILEGROUPDESCRIPTOR fgdesc = (LPFILEGROUPDESCRIPTOR) GlobalLock(stm.hGlobal);
if (fgdesc) {
  GetTempFilepath((fgdesc->fgd)[aIndex].cFileName, tmpname);
}
hres = (!wcslen(tmpname) ? E_FAIL : S_OK);
GlobalUnlock(stm.hGlobal);


or.. how about GetTempFilepath return a result for hres? I think if we work through a bit we could cut down on the amount of code quite a it.


>+                    if (S_OK == hres) {
>+                      hres = StgCreateDocfile(tmpname, STGM_CREATE | STGM_READWRITE | STGM_SHARE_EXCLUSIVE, 0, &file);
>+                    }
>...
>+                      file->Release();
>+                    }

Could we tighten this up? Looks like all the file manipulation code could sit in that first if block. This and the else block could likely be broken out into functions.

>+                  if (buffer) {
>...
>+                  else {
>+                    result = NS_ERROR_OUT_OF_MEMORY;
>+                  }

If you've already released, might as well check for error first and just return.

On GetTempFilepath, we have some code in nsDataObj for generating a random filename string. Maybe we could re-use that. Not sure, but I don't like this repetitive opening of files with names from 1-10000. :) Pick a string, try to open it, and fail generally with a warning maybe if it doesn't work. You might look at nsILocalFileWin, we might have a getuniquetempfile routine in there that could replace all of this.

>+          else {
>+            *aNumItems = 1;
>+          }
>+          ReleaseStgMedium(&stm);
>+        }
>+        else {
>+          *aNumItems = 1;
>+        }
>+      }
>+      else {
>+        *aNumItems = 1;
>+      }
>+    }

Maybe set *aNumItems = 1 and reset it to zero on error? 

Mostly just code format / logic nits here so far Michael. It's a great start, we need support for this. Please clean things up a bit and repost so we can get to the meat of what you're trying to do here.
Attachment #459755 - Flags: review?(jmathies) → review-
(Reporter)

Comment 6

7 years ago
Created attachment 462345 [details] [diff] [review]
Patch to support Drag and Drop of E-Mails from Outlook

I took your advice and made some changes.

1. I made two seperate methods SaveIStorage and SaveIStream.
2. GetTempFilepath now returns a nsresult and uses the CreateUnique-Method of nsILocalFile.
3. I changed "if (S_OK == hres)" to "if (FAILED(hres))" but I could not instantly return on error because there is a ReleaseStgMedium at the end of the function which has to be called.
4. StgCreateDocfile was deprecated so I changed it to StgCreateStorageEx and made some other minor tweaks.
Attachment #459755 - Attachment is obsolete: true
Attachment #462345 - Flags: review?(jmathies)

Comment 7

7 years ago
Hi,

Is there any Timetable, when this feature could be integrated into a release build?

best regards 
martin

Comment 8

7 years ago
Comment on attachment 462345 [details] [diff] [review]
Patch to support Drag and Drop of E-Mails from Outlook

Review of attachment 462345 [details] [diff] [review]:

Is there privacy leakage potential here? Who cleans up the files we create? Maybe if the call wants a file this is ok as they will clean it up? 

(I promise I'll swing back around on this sooner this time, sorry for the review delay.)

::: widget/src/windows/nsClipboard.cpp
@@ +420,5 @@
   hres = FillSTGMedium(aDataObject, format, &fe, &stm, TYMED_HGLOBAL);
 
+  // If the format is CF_HDROP and we haven't found any files we can try looking
+  // for virtual files with FILEDESCRIPTOR.
+  if ((hres != S_OK) && (format == CF_HDROP)) {

No need for the added parens here, just

if (hres != S_OK && format == CF_HDROP) {

or

if (FAILED(hres) && format == CF_HDROP) {

@@ +526,5 @@
+                  hres = GetTempFilepath(nsDependentString((fgdesc->fgd)[aIndex].cFileName), tmppath);
+                }
+                else {
+                  hres = E_FAIL;
+                }

nit - 

hres = E_FAIL;
if (fgdesc) {
  hres = GetTempFilep...
}

@@ +1018,5 @@
+
+  NS_ENSURE_ARG_POINTER(aDataObject);
+  SET_FORMATETC(fe, RegisterClipboardFormat(CFSTR_FILECONTENTS), 0, DVASPECT_CONTENT, aIndex, TYMED_ISTORAGE);
+  hres = aDataObject->QueryGetData(&fe);
+  NS_ENSURE_SUCCESS(hres, hres);

NS_ENSURE_SUCCESS checks mozilla return results, but in here looks like we are dealing with COM error codes? In which case this would have unexpected results. (There are three more below this, and a couple more in SaveIStream.)

@@ +1033,5 @@
+  file->Commit(0);
+  file->Release(); /* Should we release the IStorage in case CopyTo failes? */
+  ReleaseStgMedium(&stm);
+
+  return hres;

Mixing return values here, we are returning an HRESULT as an nsresult. Same for SaveIStream.

@@ +1051,5 @@
+
+  hres = aDataObject->GetData(&fe, &stm);
+  NS_ENSURE_SUCCESS(hres, hres);
+
+  char buffer[4096] = {0}; /* What would be a good buffersize? */

Let's define that above the method, or use some msdn constant instead.

::: widget/src/windows/nsDragService.cpp
@@ +427,5 @@
+      FORMATETC fe2;
+      SET_FORMATETC(fe2, nsClipboard::CF_FILEDESCRIPTOR, 0, DVASPECT_CONTENT, -1, TYMED_HGLOBAL);
+      if (mDataObject->QueryGetData(&fe2) == S_OK) {
+        STGMEDIUM stm;
+        if (mDataObject->GetData(&fe2, &stm) == S_OK) {

SUCCEEDED() on these two please.
Attachment #462345 - Flags: review?(jmathies) → review-

Comment 9

7 years ago
Hi,  

this patch was deployed by our former employee. Is there anything left we should submit so that this patch will have a chance to get into the release build?

regards,
Martin
Assignee: nobody → netzen

Comment 10

6 years ago
Hi, 

Are there still open tasks before this patch will be applied?

Bye,
Juergen
Assignee: netzen → nobody

Comment 11

4 years ago
Really looking forward to this, right now there is no way to drag and drop Mails into HTML forms. 
Any chance to get this included anytime soon?
I'm working on some other drag-and-drop issues so I'll take a look at this
Assignee: nobody → tabraldes
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

Comment 13

4 years ago
It would be very helpful, if your fix would also work for the drag and drop of email attachments.

Comment 14

4 years ago
(In reply to Tim Abraldes [:TimAbraldes] [:tabraldes] from comment #12)
> I'm working on some other drag-and-drop issues so I'll take a look at this

Hi Tim, 

  Where are you with this issue?  Do you need any assistance?  Our company may be available to help get this patch ready for release.

Brendan
Flags: needinfo?(tabraldes)
(In reply to Brendan Doherty from comment #14)
>   Where are you with this issue?  Do you need any assistance?  Our company
> may be available to help get this patch ready for release.

Half-a-year ago I was working on an effort to rewrite our clipboard and drag&drop functionality on Windows, which is why I picked up this bug. That rewrite lost steam as other priorities picked up and is now in a state of bitrot. I still think our clipboard and d&d code needs an overhaul and I'd like to work on this in the future, but I'm unassigning myself for now since I'm not working on it currently and haven't been for about 6 months.
Assignee: tabraldes → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(tabraldes)

Comment 16

2 years ago
Sorry to jump in so late in the day!! My company has recently moved away from Lotus Notes for which the drag-and-drop from an email worked fine, but now we have migrated to Outlook the drag-and-drop from an email no longer works. 

Just wondering if there are any potential solutions on the horizons??

Thanks,
Liam

Comment 17

2 years ago
This isn't a Firefox-specific solution, but there's a product called Outlook2Web which you can install on Windows PCs and will enable drag and drop to websites in Firefox and other browsers. It's at http://outlook2web.com/
Whiteboard: [platform-rel-Microsoft][platform-rel-Outlook]
platform-rel: --- → ?
platform-rel: ? → ---
Component: Shell Integration → Drag and Drop
Product: Firefox → Core
Version: 3.6 Branch → unspecified
You need to log in before you can comment on or make changes to this bug.