400664 - Cannot drag folders in Bookmarks menu

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P1)

Description

[RenegadeX]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102105 Minefield/3.0a9pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102105 Minefield/3.0a9pre

Although it currently works on the Bookmarks Toolbar and in Places Organizer, using a drag n drop key-modifier on a bookmarks *FOLDER* in the Bookmarks Menu does not work.

Reproducible: Always

Steps to Reproduce:
1. Open the Bookmarks menu and navigate to a bookmarks folder ("Places" will do, or create a new one)
2. Try to move or copy the folder using either SHIFT or CTRL keys as a modifier (ie: hold down SHIFT or CTRL then left-click-and-hold then drag.
Actual Results:  
Nothing happens. Can't move or copy a folder. The pointer does not change to its expected modified state of '+'(copy) or rectangle(move).


Obviously, no drop indicator is shown - which is another bug, actually Bug 389290, https://bugzilla.mozilla.org/show_bug.cgi?id=389290 but possibly tied in.

Also be aware that the Ctrl key modifier is partially-broken anyhow: https://bugzilla.mozilla.org/show_bug.cgi?id=400661

And that bookmarks menu sub-menus(folders) no longer expand when the mouse is held over the -> arrow during drag n drop. I'm about to file this Bug too....

Comment 1

[Marco Bonardo [:mak]]

actually there is no way to D&D folders in the bookmarks menu, it is due to controller.js

  handleEvent: function(aEvent) {
    switch (aEvent.type) {
    case "draggesture":
      if (aEvent.target.localName != "menu" && aEvent.target.node) {
        // TODO--allow menu drag if shift (or alt??) key is down
        nsDragAndDrop.startDrag(aEvent, this);
      }
      break;

is the key modifier still needed to move folders in the bookmark menu? 
i've tried to change the code to  

      if (aEvent.target.node) {
        nsDragAndDrop.startDrag(aEvent, this);
      }

and D&D is working fine for folders in bookmark menu and toolbar menus (not toolbar itself)... 

this makes reordering items in bookmark menu not possible, so asking blocking

Comment 2

[Marco Bonardo [:mak]]

Attachment: proposed change

Comment 3

[Marco Bonardo [:mak]]

actually, in firefox, 2 bookmarks menu containers are draggable without any key modifier.

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

I think this is a dupe, but blocking just in case not. We shouldn't be regressing any functionality here.

Comment 5

[Bryan Henry]

Yep, marking dupe. If this should be blocking, then the other one's going to need to be marked for blocking. I'll put in a request on it.

Comment 6

[Marco Bonardo [:mak]]

why a dupe? in the firefox 2 menu i can drag without any modifier, how can this be a dupe of a bookmark toolbar MAC OS X only bug that requires a modifier?

Comment 7

[Bryan Henry]

Sorry about that Marco, no more triage this early for me...thanks for catching the mistake. *sigh*

Comment 8

[Marco Bonardo [:mak]]

Attachment: patch

We should allow draggin folders in the bookmarks menu without any key modifier, since this is how Firefox 2 works.

Comment 9

[Marco Bonardo [:mak]]

oh, modifiers work fine with this (so you can move with SHIFT and copy with CTRL)

Comment 10

[Asaf Romano (gone)]

Comment on attachment 302803 [details] [diff] [review]
patch

You forgot menuseparators, I think.

Does the various other d&d code know how to handle container nodes?

Comment 11

[Marco Bonardo [:mak]]

(In reply to comment #10)
> (From update of attachment 302803 [details] [diff] [review])
> You forgot menuseparators, I think.

oops!

> Does the various other d&d code know how to handle container nodes? 

i've tried dragging the container from the menu to toolbar (ok) to library right pane (ok) to sidebar (ok). Everythings looks fine, at least this should not have any bad behaviour like profile corruption, and tester will be able to give us feedback on related problems.

the only "problems" i've seen so far are when dragging to location bar (the folder title is used like an address (the same happens in firefox 2) and on current page (folder title used like an address, while in firefox 2 it does nothing).

Have i lost some important drag path?
Will come with fixed patch, i hope we can put this in to start testing soon, we actually don't have enough feedback on D&D problems since most D&d features are disabled (see for example the Library)

Comment 12

[Asaf Romano (gone)]

Within the menu, within sub menus, from one menu to a sub menu.

Comment 13

[Marco Bonardo [:mak]]

Attachment: patch

added menuseparator

well since this was a menu bug the first thing i tested was the menu, so i did not report about them :)
dragging into a submenu is quite difficult since the submenu does not open on dragover (thread manager native events bug), but i can drag from a submenu to the main menu, and from the main menu over a folder (the folder is appended at the end of the submenu).

So this should really work, imho we should put this in asap, to have feedback from testers and QA

Comment 14

[Asaf Romano (gone)]

Comment on attachment 303013 [details] [diff] [review]
patch

So, I would just do if if (aEvent.target.node).

r=mano on that ;)

Comment 15

[Marco Bonardo [:mak]]

Attachment: patch

as you will.
I added checks because i thought in future we could add some other element there... but really we can change that in future :)

Comment 16

[Reed Loden [:reed]]

Checking in browser/components/places/content/controller.js;
/cvsroot/mozilla/browser/components/places/content/controller.js,v  <--  controller.js
new revision: 1.202; previous revision: 1.201
done

Comment 17

[Tony Chung [:tchung]]

Verified fix in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b4pre) Gecko/2008022104 Minefield/3.0b4pre

Comment 18

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv