25589 - [FEATURE - D&D] Drag and Drop of Folders

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Message Display, defect, P2)

Description

[chuang]

Separate bug for folder drag & drop.  Need back end MoveFolder exposed to Java 
Script.

Comment 1

[lchiang]

move to M16.  Not M15 stoppers

Comment 2

[selmer (gone)]

Mass moving M16 to M17 - look for nsbeta2 before anything else.

Comment 3

[scottputterman]

If this isn't going to require more than exposing an API and calling it the 
appropriate place, then I think we should consider this for nsbeta2 as this is 
the only way to rearrange your folders.

Comment 4

[scottputterman]

moving to future.

Comment 5

[laurel]

nominating this for 6.5, it is a very handy thing to have and we receive a lot
of reports on this

Comment 6

[laurel]

*** Bug 61403 has been marked as a duplicate of this bug. ***

Comment 7

[Karen Huang]

*** Bug 47487 has been marked as a duplicate of this bug. ***

Comment 8

[Karen Huang]

Without this fix, I cannot move (promote/demote) the folders from the folder 
pane....

Comment 9

[scottputterman]

marking nsbeta1+

Comment 10

[scottputterman]

moving to mozilla0.8 milestone.

Comment 11

[laurel]

*** Bug 63838 has been marked as a duplicate of this bug. ***

Comment 12

[Matthew Tuck [:CodeMachine]]

I really need this feature to present, but I really need this feature to be
absent.

I occasionally want to move folders around, but I always accidentally drag
folders around, and it's annoying.

Maybe this shouldn't occur on left drag?  Maybe shift-left drag or right-drag? 
The trick then is letting the user know this, of course.

At the very least I'd like a "Move To Parent" option on the right click menu,
because I often have the following:

A
->B
->C
-> ... lots of folders ...
->D
->E

If I drag E into D by accident, as I often do, it would be nice not to have to
scroll all the way up to A, as I have so many folders it's often not on the
screen.  Although I believe N4 Linux doesn't scroll up when you hover over the
top, that feature might be helpful here too.

Comment 13

[Matthew Tuck [:CodeMachine]]

Oh, and by the way ... please don't reimplement the "do you want to change the
filters" anti-feature, just do it.

Comment 14

[chuang]

reassign to Navin.  Need to have Move/Copy folder API as CopyMessages in 
nsIMessenger.  

Comment 15

[laurel]

*** Bug 64789 has been marked as a duplicate of this bug. ***

Comment 16

[scottputterman]

*** Bug 64299 has been marked as a duplicate of this bug. ***

Comment 17

[Navin Gupta]

Candice, have you implemented the FE for this feature.I may need it as I 
implement the back end.

Comment 18

[Håkan Waara]

*** Bug 66765 has been marked as a duplicate of this bug. ***

Comment 19

[laurel]

*** Bug 66764 has been marked as a duplicate of this bug. ***

Comment 20

[[e-mail] (gone - use [e-mail] instead)]

*** Bug 67324 has been marked as a duplicate of this bug. ***

Comment 21

[Navin Gupta]

Attachment: Proposed fix

Comment 22

[timeless]

the idl tab indentation and javascript { placement is really non standard and 
reminds me of travis. please fix before committing

Comment 23

[Navin Gupta]

Feature checked in. 

Comment 24

[Matthew Tuck [:CodeMachine]]

How does this currently work with filters?

Comment 25

[Navin Gupta]

Drag and drop of a folder is like renaming folder. Therefore you will
have update your filters manually.

Comment 26

[Matthew Tuck [:CodeMachine]]

Is there a bug # to do this properly, or should I file?

Comment 27

[Navin Gupta]

This is not a bug. cc'ing gayatri for her opinion

Comment 28

[Matthew Tuck [:CodeMachine]]

Not a bug.  Yeah right.

To start with, a user would not want a filter pointing to a folder that doesn't
exist.

I've never wanted the filters to not change, and I doubt other people would have
either.

Comment 29

[Peter "jag" Annema]

Reopening bug.

naving, please go over what you checked in and fix all the outstanding
tab/whitespace issues. It makes reading your code a "fun" experience.

Comment 30

[Navin Gupta]

Attachment: patch that was checked in

Comment 31

[Navin Gupta]

I think the latest patch is okay. This was checked in yesterday. 

Comment 32

[Håkan Waara]

Navin: sometimes when dragging a folder over other folders the stop-cursor (i.e
the circle with a line across it) is shown.

Comment 33

[Navin Gupta]

That means you cannot drop a folder there. For example, you cannot drop 
a parent folder on its child (folder).  

Comment 34

[Brendan Eich [:brendan]]

What jag said about the tabs.  Please conform to the Emacs modeline comment at
the top of the file.  The indent-tabs-mode: nil means there should be no tabs in
the source file.  If your editor is vi, switch to vim and set expandtabs.  If
you are using MSVC, Tools : Options : Tabs : Expand spaces (or something like
that, my memory is fuzzy).  I hear the latest CodeWarrior can also be persuaded
to expand tabs to the right number of spaces.

/be

Comment 35

[Peter "jag" Annema]

Please read your patch as attached here with Netscape / mozilla...

A few excerpts from your patch:

Arguments / if criterium parts not lining up (looks like tabs):

+       void CopyFolders(in nsIRDFCompositeDataSource database,
+                      in nsIRDFResource dstFolder,
+                      in nsISupportsArray folders,
+                      in boolean isMoveFolder);

+       if (event.target.localName != "treecell" &&
+          event.target.localName != "treeitem") {        
+          event.preventBubble();
+          return false;
+       }

Mix of 2, 3 and 4 space indents:

+       if (folderFlavor)
+       {
+         debugDump("***isFolderFlavor == true \n");
+        
+         if (event.ctrlKey)              //ctrlkey does not apply to folder 
drag
+             return(false);
+
+         var canCreateSubfolders = 
treeItem.getAttribute('CanCreateSubfolders');
+         if ( canCreateSubfolders == "false")  // if cannot create subfolders 
then a folder cannot be dropped here     
+         {
+            debugDump("***canCreateSubfolders == false \n");
+            return(false);
+         }

Mix of tab / space indent:
+       if ( event.target == tree )
+               return(true); // continue propagating the event

And a bit lower:
+       if (flavor == "text/nsfolder")
+       {
...
+          gSrcCanRename = treeItem.getAttribute('CanRename');  //used in 
DragOverTree

5 space indent?

+        if (event.target.localName != "treecell" &&
+            event.target.localName != "treeitem")
+             return false;

No idea.. :

+               if (!ctrlKeydown)
+               {
+                       messageTree = GetThreadTree();
+                       nextMessage = 
GetNextMessageAfterDelete(messageTree.selectedItems);
+                       if(nextMessage)
+                       gNextMessageAfterDelete = 
nextMessage.getAttribute('id');
+                       else
+                               gNextMessageAfterDelete = null;
+               }
+              try {
+                       messenger.CopyMessages(treeDatabase,
+                                          sourceResource,
+                                                  targetNode, list, 
!ctrlKeydown);
+                  }
+              catch(e)
+                  {
+                      gNextMessageAfterDelete = null;
+                      dump ( "Exception : CopyMessages \n");
+                  }

and more:

+           if (sourceServer == targetServer)
+                  moveFolder = true;
+               
+               try 
+               {
+                  
messenger.CopyFolders(treeDatabase,targetNode,list,moveFolder);
+               }
+               catch(e)
+               {
+                  dump ("Exception : CopyFolder \n");
+              }

I'm sure you get the idea...

Comment 36

[Peter "jag" Annema]

Also keep an eye on spaces near parens... Generally a space goes between the 
statement and the parens, but not between the parens and the first argument. No 
space between a function call or name and the opening parens though.

Comment 37

[Ben Goodger (use ben at mozilla dot org for email)]

jag - your last comments:

placement of spaces around function params is a matter of personal style and 
should not be dictated, HOWEVER, one should strive to be consistent within one's 
own code ;) 

Comment 38

[Peter "jag" Annema]

I had just made the same comment, then collided ;-)

Comment 39

[scottputterman]

marking as fixed again.  Maybe someone can file a bug about whitespace being
wrong. I'm not saying it shouldn't be fixed, I just don't think we should reopen
this bug because of this.

Regarding filters, I don't know what bug number it is, but there's definitely a
bug out there about how when a folder doesn't exist (through renaming, deleting,
etc) that filters need to be updated to the new location or removed/disabled if
the location no longer exists.  That problem exists regardless of the work done
in this bug. Of course, this new code helps to make it a little more obvious.

Comment 40

[scottputterman]

I filed 67637 so that jag's comments aren't lost when I marked this as FIXED.

Comment 41

[Matthew Tuck [:CodeMachine]]

Bug #41720 is about updating filters.

Comment 42

[Karen Huang]

Verified fix on WinNT 02-19-06-Mtrunk
Verified fix on Linux 02-19-08-Mtrunk
Verified problem is still existing on Mac 02-19-04-Mtrunk
Now users can use drag & drop feature from the folder pane on Windows & Linux 
platforms, but problem is still existing on Mac platform.
Reopening this bug, change platform to Mac, adding pp for the keywords since it 
failed on Mac ONLY platform.....

Comment 43

[Navin Gupta]

*** Bug 68958 has been marked as a duplicate of this bug. ***

Comment 44

[Navin Gupta]

fix checked in

Comment 45

[[e-mail] (gone - use [e-mail] instead)]

This works on Mac OS 9.1 with build 2001040208 now, too.  Marking VERIFIED.