Something must have regressed this recently, actually i'm unable to drag folders on toolbar, no way. Asking blocking to make drivers aware, not sure if it should block but it's primary ui interaction.

As what a quick looks show me it has been regressed between beta 4 and recent builds. I'll have a closer look now.

Regressed between the build 09050404 and 09050504: pass: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c77b7d273dfb fail: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e9e27c40d0af Changesets: http://hg.mozilla.org/releases/mozilla-1.9.1/pushloghtml?fromchange=c77b7d273dfb&tochange=e9e27c40d0af Could this have been regressed by the patch on bug 490401? CC'ing Dao.

This is Windows and Linux only, right?

No, it also stops working on OS X. I did some hg bisect with the following investigations: pass: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/99009fc6ca75 fail: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/02dcf89388a5 Marco's patch doesn't touch any event handling so we think this should be a regression from Neil's patch on bug 488752.

Yeah i think that patch touches menu frame so that it does not consume default event anymore for VK_DOWN. that event is probably the dragstart. if ((keyCode == NS_VK_F4 && !keyEvent->isAlt) || ((keyCode == NS_VK_UP || keyCode == NS_VK_DOWN) && keyEvent->isAlt)) { *aEventStatus = nsEventStatus_eConsumeNoDefault; ToggleMenuState(); }

ops nevermind i misread that code, the problem is about mouse events clearly

So is this all platforms? The checkin implicates Mac-only, but the patch on bug 488752 affects all systems. Assigning to Neil Rashbrook. Enn, if you get to this first, I won't complain. Also, a test to make sure we don't regress this again sure would be nice!

(In reply to comment #7) > So is this all platforms? The checkin implicates Mac-only, but the patch on bug > 488752 affects all systems. Yes, it's not OS X only. Tested also on Windows XP and Ubuntu 8.10. > Assigning to Neil Rashbrook. Enn, if you get to this first, I won't complain. > Also, a test to make sure we don't regress this again sure would be nice! Seems that get lost by moving to another component. Restoring assignee.

Anyone working on this bug? Should we re-assign?

The problematic parts are the changes to mouse events, i can confirm that commenting out those parts solves the problem, but i can't guess why those changes have been pushed with a bug about keyboard navigation, so i would still need to ask to one of Neil's the reasons. Mousedown and mouseup have far more implications than key down and key up due to drag & drop, to me sounds like even if we open a menupopup we should not avoid default handling.

Reassigning to gavin as he has written a patch. As for the event handling, I was trying to achieve consistency.

I'm working on a browser chrome test, not sure when it will be ready since these D&D tests are cumbersome. but hope to attach it here soon. Just removing changes to mouse events handling is enough to restore things here, btw, waiting on gavin's patch.

this doesn't want to be a fully complete test, i started investigating this kind of tests for Places, so it's a first approach that can be used to test this. Gavin if you want to review this, i'm fine with that, since it lives in Places i'm asking dietrich for now.

just for reference this is the patch i've used to test that the test is doing its work, basicly a partial backout of bug 488752.

Talked to Neil on IRC, we don't need to revert the mouseup changes (they aren't relevant to drag events).

Comment on attachment 381389 [details] [diff] [review] patch Comment why we don't prevent default perhaps? (Deleted lines show up really badly in annotate!)

Comment on attachment 381371 [details] [diff] [review] browser-chrome test for dragging bookmarks on the toolbar >+function synthesizeDragWithDirection(aElement, aExpectedDragData, aDirection) { would be great to generalize this and move into the test framework >+ { >+ desc: "Drag a folder on toolbar", >+ run: function() { >+ // Create a test folder to be dragged. >+ var folderId = PlacesUtils.bookmarks >+ .createFolder(PlacesUtils.toolbarFolderId, >+ TEST_TITLE, >+ PlacesUtils.bookmarks.DEFAULT_INDEX); >+ var element = getToolbarNodeForItemId(folderId); >+ isnot(element, null, "Found node on toolbar"); >+ >+ isnot(element.node, null, "Toolbar node has an associated Places node."); >+ var expectedData = getExpectedDataForPlacesNode(element.node); >+ >+ ok(true, "Dragging left"); not so hot on using ok() for debug... to bad there's not an info(). looks fine otherwise, r=me

(In reply to comment #18) > (From update of attachment 381371 [details] [diff] [review]) > > >+function synthesizeDragWithDirection(aElement, aExpectedDragData, aDirection) { > > would be great to generalize this and move into the test framework yes i was thinking in future to group drag tests and provide some common util. but it's too early to have a list of cases that could be handled by common code. EventUtils should probably provide more utils to test dragging, or make synthesizeDragStart more configurable. I would suggest to file a followup to expand it.

This is ready to land, yes?

Yes: https://hg.mozilla.org/mozilla-central/rev/55e06f106d73

I had to land https://hg.mozilla.org/mozilla-central/rev/69b64ba05f94 as a temporary bustage fix. The bookmark drag test was causing a page navigation, which in turn was causing a failure in browser_library_left_pane_commands.js (there were 2 history entries rather than 1 to delete): TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/places/tests/browser/browser_library_left_pane_commands.js | Visit has been removed TEST-UNEXPECTED-FAIL | chrome://mochikit/content/browser/browser/components/places/tests/browser/browser_library_left_pane_commands.js | Visit has been removed http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1244087343.1244090418.14714.gz&fulltext=1 I filed bug 496266.

Also fails on Linux: https://hg.mozilla.org/mozilla-central/rev/9d6f5ab54933 Filed bug 496277 for that.

thank you for taking care of the test, is the first test trying to synthesize a drag so it could effectively have some glitch. i'll take care of followups on it.

Verified fixed on the trunk using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090604 Minefield/3.6a1pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090604 Minefield/3.6a1pre. I can successfully drag folders on the toolbar.

Restoring the fixed collision as I think the mid air blew it away.

Verified fixed on 1.9.1 with builds on all platforms like Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090605 Shiretoko/3.5pre ID:20090605031243