244459 - Drag and drop ical (.ics) attachments from mail onto calendar

Status: VERIFIED FIXED

(Calendar :: General, defect)

Description

[gekacheka]

Dragging and dropping ical attachments from email or news is a useful subgoal of
bug 137093, which covers both whole emails and ical attachments.  

Note that copying and pasting VCALENDAR text from the mail view of the
attachment currently works (it brings up and initializes an event dialog for
each event).

Steps to reproduce:
1. bring up mozilla mailnews and calendar.
2. create a test event
3. email test event to self (context menu "email selected events")
4. delete test event from calendar
5. receive in mozilla mailnews, should have CalendarEvent.ics attachment.
6. drag the CalendarEvent.ics attachment onto the calendar grid.

Actual:  nothing happens
Expected:  Event dialog appears filled with test event data.
(as occurs when you view event.ics text in message pane, copy it, and paste
anywhere in calendar grid.)

Comment 1

[gekacheka]

Attachment: dragDrop.js patch: disable unimplemented drag data flavors( checked in )

This patch is a partial workaround.  It disables advertising the data flavors
for which receiving code is not implemented.  The result is that bestFlavor
becomes application/x-moz-file instead of text/x-moz-url.   

With this patch, dragging the ics attachment from the email pane and dropping
on Day view or Week view grid works (but fails on Multiweek view and Month
view).	Since it receives it as a file, it produces an "Saving" progress
dialog, which is a little annoying, but that is better than not working.

(Issue: why doesn't mail provide the attachment data using the contentType of
the attachment, which is text/calendar?)

Comment 2

[Mostafa Hosseini]

Comment on attachment 149171 [details] [diff] [review]
dragDrop.js patch: disable unimplemented drag data flavors( checked in )

Patch checked in.

Comment 3

[Rolf Sponsel]

Attachment: Screen shot after trying to DnD ics from Mozilla Mail to Calendar

I can confirm this on Windows/NT sp6a w/ Mozilla 1.7.2 and Calendar 2004080916.


I'm providing a Screen Shot showing Saving and Alert widgets upon DnD attempt
of ics Mail Attachment file to Calendar (w/ MultiWeek view displaying). (Note,
nothing is actually saved on disk, so no wonder it fails. After clicking OK in
Alert an new Alert w/ "aDataStream is not defined" is displayed.)

BTW, should saving actually be done in 'C:' by default, as indicated by the
screen shot? IMHO, wouldn't e.g. 'C:\temp\' be a better place for this on
Windows? (as it IS a temporary file).

Comment 4

[Rolf Sponsel]

Attachment: Screen shot after trying to DnD ics from Mozilla Mail to Calendar when Month View is showing in Calendar

The previous screen shot is valid for Day view, Week view, and MultiWeek view.
Note, that happens for all views, but for the Month view.

This screen shot displays what happens when the Month view is active.
I.e. the ics attachment is actually saved on disk (as 'C:\CalendarEvent.ics'),
but then Calendar launches the 'Add New Calendar' widget instead of importing
the event (which I, of course, don't wan't to do).

Comment 5

[Joey Minta]

Attachment: fun with dropping

Patch implements support for dropping local files, urls, text, and mail attachments in Sunbird and Lightning (mail attachments only work in Lightning).

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

Could you write down (spec) what dropping should do? A patch like this is fine, but i'd like to know what you think it should do.

Comment 7

[Joey Minta]

(In reply to comment #6)
> Could you write down (spec) what dropping should do? A patch like this is fine,
> but i'd like to know what you think it should do.
> 
http://wiki.mozilla.org/Calendar:Feature_Implementations:Drag%27n%27Drop Is this what you're looking for?

Comment 8

[Matthew (lilmatt) Willis]

Comment on attachment 226264 [details] [diff] [review]
fun with dropping

Moving reviews to ctalbert and lilmatt per dmose

Comment 9

[cmtalbert]

(In reply to comment #5)
> Created an attachment (id=226264) [edit]
> fun with dropping
> 
> Patch implements support for dropping local files, urls, text, and mail
> attachments in Sunbird and Lightning (mail attachments only work in Lightning).
> 
This looks like a great fix, but would you mind un-bitroting it?
Here is a listing of the errors when I attempt to apply it, if that is helpful. (Attempted on a 20061023 Sunbird tree that has been updated to current base).

Thanks!

E:\BUILD\sunbird\mozilla\calendar>patch -p0 <dndpatch.txt
(Stripping trailing CRs from patch.)
patching file base/content/calDropListener.js
(Stripping trailing CRs from patch.)
patching file lightning/jar.mn
Hunk #1 FAILED at 24.
1 out of 1 hunk FAILED -- saving rejects to file lightning/jar.mn.rej
(Stripping trailing CRs from patch.)
patching file lightning/content/messenger-overlay-sidebar.xul
Hunk #1 FAILED at 63.
Hunk #2 FAILED at 112.
Hunk #3 succeeded at 204 (offset 43 lines).
2 out of 3 hunks FAILED -- saving rejects to file lightning/content/messenger-overlay-sidebar.xul.rej
(Stripping trailing CRs from patch.)
patching file resources/jar.mn
Hunk #1 FAILED at 13.
1 out of 1 hunk FAILED -- saving rejects to file resources/jar.mn.rej
(Stripping trailing CRs from patch.)
can't find file to patch at input line 354
Perhaps you used the wrong -p or --strip option?
The text leading up to this was:
--------------------------
|Index: resources/content/calendar.xul
|===================================================================
|RCS file: /cvsroot/mozilla/calendar/resources/content/calendar.xul,v
|retrieving revision 1.207
|diff -p -U8 -r1.207 calendar.xul
|--- resources/content/calendar.xul     19 Jun 2006 17:03:47 -0000      1.207
|+++ resources/content/calendar.xul     20 Jun 2006 01:17:51 -0000
--------------------------
File to patch: sunbird/base/content/calendar.xul
patching file sunbird/base/content/calendar.xul
Hunk #1 FAILED at 232.
Hunk #2 succeeded at 440 with fuzz 2 (offset -52 lines).
1 out of 2 hunks FAILED -- saving rejects to file sunbird/base/content/calendar.xul.rej
(Stripping trailing CRs from patch.)
patching file sunbird/base/content/calendar-scripts.inc
Hunk #1 FAILED at 77.
1 out of 1 hunk FAILED -- saving rejects to file sunbird/base/content/calendar-scripts.inc.rej
(Stripping trailing CRs from patch.)
patching file sunbird/base/content/calendar.xul
Hunk #1 FAILED at 418.
1 out of 1 hunk FAILED -- saving rejects to file sunbird/base/content/calendar.xul.rej

Comment 10

[Joey Minta]

I'd like to wait on bug 224977 before attaching a new version of this patch, since nsTransferable is going away.

Comment 11

[Matthew (lilmatt) Willis]

(In reply to comment #10)
It's gone. :)

Comment 12

[cmtalbert]

Attachment: An updated patch that applies easier.

This is a new patch. It doesn't add or subtract anything from jminta's previous patch. I have carried the reviews forward on this patch, and marked the previous one as obsolete.

In doing this merge, I have reviewed the code, and I found it ok. However, when I applied it to my Lightning build. I found that dragging a text file with a VEVENT encapsulated in it from my file system into the month view of lightning caused this error (attempted with both a .txt and a .ics file on Mac OS X):
JavaScript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "JS frame :: chrome://calendar/content/calDropListener.js :: calDNDDrop :: line 68"  data: no]

Then I attempted to drag an ICS attachment from a mail message to the month view of Lightning, and I got these errors:
###!!! ASSERTION: NS_Alloc of size 0: 'size', file /Users/ctalbert/Programs/lightning/mozilla/xpcom/base/nsMemoryImpl.cpp, line 335
Break: at file /Users/ctalbert/Programs/lightning/mozilla/xpcom/base/nsMemoryImpl.cpp, line 335
Error parsing: '                    ': 7 (FILE: An operation on a file failed. Check errno for more detail.)
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "Component returned failure code: 0x804a0107 [calIICSService.parseICS]"  nsresult: "0x804a0107 (<unknown>)"  location: "JS frame :: file:///Users/ctalbert/Library/Thunderbird/Profiles/8ak3slaf.TEST2/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/js/calIcsImportExport.js :: ics_importFromStream :: line 104"  data: no]
************************************************************
JavaScript error: , line 0: uncaught exception: [Exception... "Component returned failure code: 0x804a0107 [calIICSService.parseICS]"  nsresult: "0x804a0107 (<unknown>)"  location: "JS frame :: file:///Users/ctalbert/Library/Thunderbird/Profiles/8ak3slaf.TEST2/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/js/calIcsImportExport.js :: ics_importFromStream :: line 104"  data: no]
************************************************************
* Call to xpconnect wrapped JSObject produced this error:  *
[Exception... "'[JavaScript Error: "gMsgFolderSelected has no properties" {file: "chrome://messenger/content/msgMail3PaneWindow.js" line: 227}]' when calling method: [nsIFolderListener::OnItemEvent]"  nsresult: "0x80570021 (NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)"  location: "<unknown>"  data: yes]
************************************************************

So, I am going to attach this patch, but I want to withhold my review until we can figure out what is going on here with these errors.

Comment 13

[Matthew (lilmatt) Willis]

Comment on attachment 244974 [details] [diff] [review]
An updated patch that applies easier.

Removing r2 request, since this patch needs the errors fixed.

Comment 14

[Omar Bajraszewski]

Blocking 0.5?

Comment 15

[Matthew (lilmatt) Willis]

We won't hold 0.5 for this.

Comment 20

[Michael Büttner (no reviews TFN)]

Attachment: final patch v1

This is an updated version of the patch including all the necessary bits and pieces to actually make this work. There's just the skeleton of the original patch left intact, I rewrote a lot of this stuff. Dropping an arbitrary ics file from the local file system as well as throwing an email attachment into the calendar view now works as advertised.

Comment 21

[Daniel Boelzle [:dbo]]

Comment on attachment 268938 [details] [diff] [review]
final patch v1

>+            case "text/calendar":
>+                var icssrv = Cc["@mozilla.org/calendar/ics-service;1"]
>+                               .getService(Ci.calIICSService);
>+                var calComp = icssrv.parseICS(data);
...
use calIcsParser for that; it minds contained exception records which this code does not.

>+            case "text/unicode":
...
try {
>+
>+                var items = importer.importFromStream(inputStream, {});
} finally {
>+                inputStream.close();

mind exception safety here, too; who calls endBatchTransaction in case doTransaction throws an exception?
>+                startBatchTransaction();
>+                for each (item in items) {
>+                    doTransaction('add', item, destCal, null, null);
>+                }
>+                endBatchTransaction();
>+                break;

>+            case "application/x-moz-file-promise":
>+            case "text/x-moz-url":
>+                var ioService = Components.classes["@mozilla.org/network/io-service;1"]
>+                                          .getService(Components.interfaces.nsIIOService);
>+                var uri = ioService.newURI(data.toString(), null, null);
>+                var channel = ioService.newChannelFromURI(uri);
>+                channel.loadFlags |= Components.interfaces.nsIRequest.LOAD_BYPASS_CACHE;

>+                var streamLoader = Components.classes["@mozilla.org/network/stream-loader;1"]
>+                                             .createInstance(Components.interfaces.nsIStreamLoader);
...
Reading unicode data, I recommend to use unichar-stream-loader (http://www.xulplanet.com/references/xpcomref/comps/c_networkunicharstreamloader1.html), so no stream conversion code is necessary.

Comment 22

[Simon Paquet [:sipaq]]

This should go into 0.7

Comment 23

[Simon Paquet [:sipaq]]

Mickey, are you going to finish this for 0.7 or can we live without it? Does the patch take the new UI (different modes) into account?

Comment 24

[Michael Büttner (no reviews TFN)]

(In reply to comment #23)
> Mickey, are you going to finish this for 0.7 or can we live without it? Does
> the patch take the new UI (different modes) into account?
I'll come up with a revised version of this patch probably tomorrow (08-29).

Comment 25

[Michael Büttner (no reviews TFN)]

Attachment: final patch v2

This is an updated version of the patch with the above comments being addressed.

Comment 26

[Daniel Boelzle [:dbo]]

Comment on attachment 279068 [details] [diff] [review]
final patch v2

>+   onDrop: function calDNDDrop(aEvent, aTransferData, aDragSession) {
indent one space

>+                var parser = Cc["@mozilla.org/calendar/ics-parser;1"].
>+                             createInstance(Ci.calIIcsParser);
move dot to second line

>+                try {
>+                    startBatchTransaction();
move startBatchTransaction() before the try-block: in case it throws, we must not call endBatchTransaction()

>+                var inputStream = Components.classes[
>+                                  "@mozilla.org/network/file-input-stream;1"]
same line IMO looks better

>+                inputStream.init(localFileInstance, MODE_RDONLY, 0444, {});
IMO last parameter behaviourFlags doesn't fit, see <http://www.xulplanet.com/references/xpcomref/ifaces/nsIFileInputStream.html#method_init>

>+                //XXX support csv
>+                var importer = Cc["@mozilla.org/calendar/import;1?type=ics"]
>+                                .getService(Ci.calIImporter);
move importer into try-block below

>+                try {
>+                    var items = importer.importFromStream(inputStream, {});
>+                    try {
>+                        startBatchTransaction();
startBatchTransaction() ^^^^

>+                        for each (item in items) {
>+                            doTransaction('add', item, destCal, null, null);
>+                        }
>+                    }
>+                    finally {
>+                        endBatchTransaction();
>+                    }
>+                }
>+                finally {
>+                    inputStream.close();
>+                }
>+
>+                break;
>+            case "application/x-moz-file-promise":
>+            case "text/x-moz-url":
>+                var ioService = Components.classes["@mozilla.org/network/io-service;1"]
>+                                          .getService(Components.interfaces.nsIIOService);
use Cc/Ci exor verbose names in the file; I prefer the latter

>+                var uri = ioService.newURI(data.toString(), null, null);
>+                var loader = Components.classes["@mozilla.org/network/unichar-stream-loader;1"]
>+                                       .createInstance(Components.interfaces.nsIUnicharStreamLoader);
>+                channel = ioService.newChannelFromURI(uri);
>+                channel.loadFlags |= Components.interfaces.nsIRequest.LOAD_BYPASS_CACHE;
>+        
>+                var listener = {
>+
>+                  // nsIUnicharStreamLoaderObserver:
>+                  onDetermineCharset: function(loader, context, firstSegment, length) {
indentation

>+                          try {
>+                              startBatchTransaction();
^^^^

>+                              for each (var item in parser.getItems({})) {
>+                                  doTransaction('add', item, destCal, null, null);
>+                              }
>+                          }
>+                          finally {
>+                              endBatchTransaction();
>+                          }

>+                try {
>+                    loader.init(channel, listener, null, 0);
>+                } catch(e) {}
I'd prefer Component.utils.reportError(e);

>+                break;
>+            default:
>+                dump("unknown data flavour:" + bestFlavor.value+'\n');
spend a break; .)

r=dbo with that fixed

Comment 27

[Michael Büttner (no reviews TFN)]

patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED

Comment 28

[Stefan Sitter [:ssitter]]

Michael, could you add a few sentence hot this feature is supposed to work? For example what can be dragged and where can it be dropped? This would make it easier for testing and validation.

Comment 29

[Michael Büttner (no reviews TFN)]

(In reply to comment #28)
> Michael, could you add a few sentence hot this feature is supposed to work? For
> example what can be dragged and where can it be dropped?
Sure. This patch allows ics files to be dropped onto any available calendar view (day, week, multiweek, month). The source can be a file (from the desktop, explorer, etc.) or an email attachment (from thunderbird's compose window). This currently works for ics files only, but the infrastructure for other formats is now in place as well.

Comment 30

[Omar Bajraszewski]

Verified with lightning 2007090504