drag & drop an .eml file into a mailbox folder via thread pane drop

RESOLVED FIXED in Thunderbird 3.0b4

Status

enhancement
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: philbaseless-firefox, Assigned: philbaseless-firefox)

Tracking

(Blocks 1 bug)

Trunk
Thunderbird 3.0b4
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 7 obsolete attachments)

Assignee

Description

10 years ago
drag and drop external message files into the thread pane
Assignee

Comment 1

10 years ago
much handier and will be used more.
Drag and drop to the open threadpane.
Attachment #385526 - Flags: review?(mkmelin+mozilla)
Assignee

Updated

10 years ago
Blocks: 500917
Assignee

Updated

10 years ago
Attachment #385526 - Flags: review?(mkmelin+mozilla)
Assignee

Comment 2

10 years ago
Comment on attachment 385526 [details] [diff] [review]
drag and drop external files to threadpane

will fix indents and resubmit. this patch is good for testing though
Assignee

Comment 3

10 years ago
Fixed some indents.
Attachment #385526 - Attachment is obsolete: true
Attachment #385603 - Flags: review?(mkmelin+mozilla)
Assignee

Comment 4

10 years ago
Missed a space before {
Attachment #385603 - Attachment is obsolete: true
Attachment #385604 - Flags: review?(mkmelin+mozilla)
Attachment #385603 - Flags: review?(mkmelin+mozilla)
Assignee

Updated

10 years ago
No longer blocks: 500917
Attachment #385604 - Flags: review?(mkmelin+mozilla) → review-
Comment on attachment 385604 [details] [diff] [review]
drag and drop external files to threadpane

If i drop on an empty folder pane i keep getting

Error: dt.mozGetDataAt("application/x-moz-file", 0).QueryInterface is not a function
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 1139


>-                    <treechildren ondraggesture="threadPaneOnDragStart(event);"/>
>+                    <treechildren ondraggesture="threadPaneOnDragStart(event);"
>+                                  ondragover="threadPaneOnDragOver(event);"
>+                                  ondrop="threadPaneOnDrop(event);"/>

These method names should be capitalized (please also fix the existing threadPaneOnDragStart).



>+      if (len > 4 && extFile.leafName.substr(len-4).toLowerCase() == '.eml')

len - 4 and please use double quotes around .eml (single quotes only when necessary)
I meant "on an empty *thread* pane, of course.
Assignee

Comment 7

10 years ago
Posted patch fix some review remarks (obsolete) — Splinter Review
I can't seem to get your error. But I did other fixes.

Note I added a test to each file for .eml on the drop.
Attachment #385604 - Attachment is obsolete: true
Attachment #386158 - Flags: review?(mkmelin+mozilla)
Comment on attachment 386158 [details] [diff] [review]
fix some review remarks

Sorry, still the same exception when the folder pane is emtpy.

>+  let dt = aEvent.dataTransfer;
>+  if (Array.indexOf(dt.mozTypesAt(0), "application/x-moz-file") != -1) {
>+    let extFile = dt.mozGetDataAt("application/x-moz-file", 0)
>+                    .QueryInterface(Components.interfaces.nsILocalFile);

Should this be nsIFile too. Or is there a reason it's nsILocalFile?
Attachment #386158 - Flags: review?(mkmelin+mozilla) → review-
Assignee

Comment 9

10 years ago
Sorry, can't dupe the error using latest build with only this patch added. I had other patches in place that seemed to be irrelevant so I unloaded those and still can't get an error.

This is starting with new empty folder in gmail IMAP build af of 6/30/09.
This is same code as 171907. Magnus try dnd a eml file to the folderpane of an empty IMAP folder as per 171907 fix.
Assignee

Comment 10

10 years ago
Magnus,
changed nsILocalFile to nsIFile but also  I notice this patch had a 'fuzz' factor but applied ok when I brought it to the tip.  Maybe there's an chance the diff wasn't good the way I had it before.
Attachment #386158 - Attachment is obsolete: true
Attachment #386411 - Flags: review?(mkmelin+mozilla)
Tried it some more, and I still get the error quite a lot. Some of the moves do succeed though, so it's a bit odd. 

Error: dt.mozGetDataAt("application/x-moz-file", 0).QueryInterface is not a function
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 1157
Error: dt.mozGetDataAt("application/x-moz-file", 0) is null
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 1156

In the native console i get
(thunderbird-bin:10170): Gtk-CRITICAL **: gtk_drag_get_data: assertion `GTK_IS_WIDGET (widget)' failed

... but i'm not sure that's the cause or just some symptom.
Btw, read-only folders, (e.g. news), and dropping on the thread pane of saved searches don't seem to be handled.
Assignee

Comment 13

10 years ago
should the d&d of files be limited to local store only?  It appears that would be best at this point.  Also is there more limits that should be placed on the drop?  Drag and drop to the folder tree had various limits in place that I didn't have to worry about, and now I believe I'll need some help on what kind of limits need to be in this drop action.
Assignee

Comment 14

10 years ago
works good in windows. this may satisfy review comments regarding folders that should be forbidden.
Attachment #386411 - Attachment is obsolete: true
Attachment #389367 - Flags: review?(mkmelin+mozilla)
Attachment #386411 - Flags: review?(mkmelin+mozilla)
Assignee

Comment 15

10 years ago
Attachment #389367 - Attachment is obsolete: true
Attachment #389802 - Flags: review?(mkmelin+mozilla)
Attachment #389367 - Flags: review?(mkmelin+mozilla)
Assignee

Updated

10 years ago
Status: NEW → ASSIGNED
Comment on attachment 389802 [details] [diff] [review]
change previous patch to allow drop file to folder of server type rss

I get the usual 
Error: dt.mozGetDataAt("application/x-moz-file", 0) is null
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 1139

Error: dt.mozGetDataAt("application/x-moz-file", 0).QueryInterface is not a function
Source File: chrome://messenger/content/msgMail3PaneWindow.js
Line: 1140

and one i don't recall seeing before:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80520006 (NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) [nsIFile.isFile]"  nsresult: "0x80520006 (NS_ERROR_FILE_TARGET_DOES_NOT_EXIST)"  location: "JS frame :: chrome://messenger/content/msgMail3PaneWindow.js :: ThreadPaneOnDragOver :: line 1141"  data: no]

>+    if (!gFolderDisplay.displayedFolder.canFileMessages)
>+      return;

Remove the spaces from the empty line ^^

It does work occasionally though.

I don't know why it's not working for me. You can try having someone else reviewing it. If it works on other platforms I think it's ok to take this - the exceptions doesn't cause much harm (except for that the drop doesn't work) afaikt.
Attachment #389802 - Flags: review?(mkmelin+mozilla)
Assignee

Comment 17

10 years ago
Need to find a Mac tester and maybe bienvenu can confirm it is ok on windows.

I removed the nit on spaces, but also moved up or earlier in the function, a test for canFileMessages.
Attachment #389802 - Attachment is obsolete: true
Assignee

Comment 18

10 years ago
Comment on attachment 390758 [details] [diff] [review]
remove excess spaces and moved a test up in a function

could you review this on MAC platform?
Attachment #390758 - Flags: review?(bugzilla)
Assignee

Comment 19

10 years ago
Comment on attachment 390758 [details] [diff] [review]
remove excess spaces and moved a test up in a function

could you review this on windows and confirm it works. linux seems to have problems so I better have it confirmed on windows and/or vista if possible
Attachment #390758 - Flags: review?(bienvenu)

Comment 20

10 years ago
Comment on attachment 390758 [details] [diff] [review]
remove excess spaces and moved a test up in a function

works on windows, thx for the patch.
Attachment #390758 - Flags: review?(bienvenu) → review+
Comment on attachment 390758 [details] [diff] [review]
remove excess spaces and moved a test up in a function

This works on Mac as well (no errors in the console).

>+  let dt = aEvent.dataTransfer;
>+  if (Array.indexOf(dt.mozTypesAt(0), "application/x-moz-file") != -1) {
>+    let extFile = dt.mozGetDataAt("application/x-moz-file", 0)
>+                    .QueryInterface(Components.interfaces.nsIFile);

I can't see anything wrong with these lines but for some reason mozGetDataAt is sometimes returning null, and sometimes returning something like a string of unprintable characters.

Can you raise a core bug with the symptoms and errors we get on Linux please?

Otherwise this looks and works fine. Thanks for the patch. r=Standard8 with the core bug filed.
Attachment #390758 - Flags: review?(bugzilla) → review+
Assignee

Comment 22

10 years ago
(thunderbird-bin:10170): Gtk-CRITICAL **: gtk_drag_get_data: assertion
`GTK_IS_WIDGET (widget)' failed

I marked checkin needed. Tokoe tried this and confirmed this backend linux problem. I also asked him to file a bug when this gets checked in.
Assignee

Updated

10 years ago
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/585420c2f9d4
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.0b4

Comment 24

10 years ago
Added new bug report #507607 that addresses the Linux problem
You need to log in before you can comment on or make changes to this bug.