+++ This bug was initially created as a clone of Bug #500564 +++ drag and drop external message files into the thread pane

not sure about the naming of functions. these are same as shredder patch.

see other description. Last was a bad patch.

Comment on attachment 385596 [details] [diff] [review] drag and drop external files to threadpane >+ if (extFile.isFile()){ need space before {

Comment on attachment 386172 [details] [diff] [review] fix nits and add test for ".eml" before drop and changed func name Works great, "even" with Linux and IMAP. Just some nits, mostly similar to those in 499878... First of all, please follow the prevalent bracing style in messengerdnd.js, which is "{ go on their own line, vertically aligned with }". >diff --git a/mailnews/base/resources/content/messengerdnd.js b/mailnews/base/resources/content/messengerdnd.js >+function DragOverThreadPane(aEvent) { >+ let ds = Components.classes["@mozilla.org/widget/dragservice;1"] >+ .getService(Components.interfaces.nsIDragService) >+ .getCurrentSession(); >+ ds.canDrop = false; >+ let dt = aEvent.dataTransfer; >+ if (Array.indexOf(dt.mozTypesAt(0), "application/x-moz-file") != -1) { MIME type should be "message/rfc822". >+ cs.CopyFileMessage(extFile, gMsgFolderSelected, null, false, 1, "", null, >+ msgWindow); This looks odd, just put the msgWindow one line up, despite the line length. Thanks for tackling this!

thanks for review. we need the x-moz-file flavor for transfer if files per our nsiTransferable.idl. for some reason I thought I needed nsILocalFile functionality but nsIFile is official transfer flavor data so I changed it to that.

Comment on attachment 387373 [details] [diff] [review] fix per review and changed flavor data interface > we need the x-moz-file flavor for transfer if files per our > nsiTransferable.idl. Yeah, I realized that after commenting. Sorry about that. :(

Comment on attachment 387373 [details] [diff] [review] fix per review and changed flavor data interface You need SR for shared code in /mailnews.

Comment on attachment 387373 [details] [diff] [review] fix per review and changed flavor data interface Karsten can you look at the problems with this patch on TB before sr. Bug 500564

this patch prevents or forbids drop of the file on 'rss' feed folder thread panes and also those that canfilemessage is false. Also patched to new location in c-c hg tree.

Comment on attachment 389418 [details] [diff] [review] forbid drop on certain folders I feel like I reviewed this before, but in any case, you should be able to drop into rss folders since they're just local folders.

this is the SeaMonkey version of a similiar patch reviewed and checked-in in TB.

I set this to depend on Bug 499878 hoping that gets check in first and avoid the bit rot if this had been a diff to the tip. Hope that's kosher

Comment on attachment 393112 [details] [diff] [review] drop file on rss folder is ok I'm not the right person for SM reviews.

Comment on attachment 393112 [details] [diff] [review] drop file on rss folder is ok >+ let ds = Components.classes["@mozilla.org/widget/dragservice;1"] >+ .getService(Components.interfaces.nsIDragService) >+ .getCurrentSession(); >+ ds.canDrop = false; Isn't this the default? Anyway, I understand we should be using the new API for this where possible (see below). >+ let dt = aEvent.dataTransfer; Nit: should set the .effectAllowed to "copy" >+ if (!gMsgFolderSelected.canFileMessages) >+ return; Should do this check earlier. >+ let len = extFile.leafName.length; >+ if (len > 4 && extFile.leafName.substr(len - 4).toLowerCase() == ".eml") if (/\.eml$/.test(extFile.leafName)) [both times of course] >+ ds.canDrop = true; dev.m.o says event.preventDefault(); is the new API for this. Note: it's slightly confusing that you can drop a group of files if the first one is a .eml file but other files get imported too, but if the first one is not a .eml file then the drop is not permitted. Check all the files maybe?

rerequest review due to following >Nit: should set the .effectAllowed to "copy" didn't add this for a few reasons. TB2 doesn't and our patch in TB3 doesn't bar move symbol (copy still occurs either way). But more important, due to some other defect, user can ctrl-drag for copy but once in pane it has no effect and releasing or pressing ctrl just cycles '+' symbol and it drops/copies source either way.

Comment on attachment 405784 [details] [diff] [review] change per review with exceptions >+ if (extFile.isFile()) >+ if (/\.eml$/.test(extFile.leafName)) >+ if (extFile.isFile()) >+ if (/\.eml$/.test(extFile.leafName)) Nit: These could be combined into a single test using && if (/\.eml$/.test(extFile.leafName) && extFile.isFile())

I added effectAllowed="copy". In windows it doesn't do anything. Maybe it works in mac or unix so added per our api docs for future implementation on windows.

Need approval-sm2.0 before checkin.

Comment on attachment 406798 [details] [diff] [review] for checkin [Checkin: Comment 22] 1) Please "forward" the review/superreview flags in the future, by setting them yourself on patches that were changed according to review comments, and noting e.g. "forward r+sr=Neil" so that someone like me, who is only coming in here for approval, can easily see what's up. 2) Please land on comm-central before landing on 1.9.1, comm-central doesn't need approval now. 3) 2.0 final is done, approving for 2.0.1, which is what current comm-1.9.1 is headed for.

Comment on attachment 406798 [details] [diff] [review] for checkin [Checkin: Comment 22] http://hg.mozilla.org/comm-central/rev/eca2aaf19ffa http://hg.mozilla.org/releases/comm-1.9.1/rev/9feb8020af55