This appears to be Linux only. If you click on Bookmarks in the main browser window and then left click on an item and start to move it within the bookmarks menu, after about 5 seconds the menu will be dismissed and the selected item will still show until you release the mouse button. At this point things are hosed in such a manner that the windows close button on the browser window no longer functions. You can, however, close the window via File -> Quit.

Sounds pretty bad. Reed/Ventnor, can you look into this and see if you can find a regression range?

I might have some time to try to narrow this down tonight.

I found the checkin that caused the regression, but that is not really very useful information. It is bug 381281, which is the bug that fixed drag and drop within the bookmarks menu to work at all. However, prior to that checkin, drag and drop within the bookmarks menu was never available under Linux. This was a Windows only feature prior to the places landing. Under places it seems the enable test was changed from windows only to everything except MAC. SO, although it would be good to fix this, the good news is that if this can't be figured out before Firefox 3 is done, we can always go back to making this be a windows only feature without regressing any Firefox 2 behavior.

I have a patch to fix this. I will attach it here as soon as I make sure it works under windows as well.

Bill, i'm about changing much code in menu.xml for the dropmarker (and fixing bogus overfolder code) so please don't rely on that. About this problem i see that mainly is because onDragExit in PlacesMenuDNDController is completely broken, it should not fire when dragging on a child of the menu (notice that the same code is also serving the toolbar)

for "it should not fire" i mean "it should not set closeTime"

This is the original patch I made to test to see if this would regress windows. This is sufficient to avoid the issue here and also avoids the regressions I reported in bug 389931 after trying the patch for that bug. This makes a lot of code in browser-places.js be essentially dead. Evidently that code was either never needed in the first place, or a subsequent fix elsewhere made it no longer necessary. I am working on a patch to remove all the code that is no longer executed with this patch in place, but have yet to get that to work quite right. I should have something either later today or tomorrow that will be ready for review.

This removes the code the workaraound made non-reachable.

mh, what actually happens if you drag on the menu and then drag out of it on the left/right without moving to the actual bookmarks menu? is the menu dismissed?

The menu is dismissed, and the item you were dragging then opens because you dragged it to the content area. That kind of makes sense to me. Let me wait for my windows build to complete to make sure it behaves the same way there.

(In reply to comment #10) > The menu is dismissed, and the item you were dragging then opens because you > dragged it to the content area. That kind of makes sense to me. Let me wait > for my windows build to complete to make sure it behaves the same way there. > Oh dear. in that case you still end up with the non-functioning close button. so although I fixed all the premature closing issues, the issue of the non-functional close button under Linux still remains.

i mean dragging over "bookmarks" in the menubar, then move on the side (toward "history"), sorry i was not clear enough...

It is odder that that. It seems almost all the time the menu does not dismiss. In the cases where it does not, the close button works fine. It is on the rare occasions where it dismisses that the close button gets hosed. So there is something else amiss here.

(In reply to comment #12) > i mean dragging over "bookmarks" in the menubar, then move on the side (toward > "history"), sorry i was not clear enough... > That seems to work fine.

(In reply to comment #11) > (In reply to comment #10) > > The menu is dismissed, and the item you were dragging then opens because you > > dragged it to the content area. That kind of makes sense to me. Let me wait > > for my windows build to complete to make sure it behaves the same way there. > > > > Oh dear. in that case you still end up with the non-functioning close button. > so although I fixed all the premature closing issues, the issue of the > non-functional close button under Linux still remains. > I seem to be completely unable to duplicate this behavior. I suspect that when I tried this I was accidentally running a build without my patch.

(In reply to comment #13) > It is odder that that. It seems almost all the time the menu does not dismiss. > In the cases where it does not, the close button works fine. It is on the > rare occasions where it dismisses that the close button gets hosed. So there > is something else amiss here. > OK. I have decided that the one time I got the menu closing by itself and the non-functional close button I must have been running the wrong build because I am completely unable to duplicate that now with a build containing my patch. I also did testing on a windows Firefox 2 build and the behavior that dragging form the bookmarks menu to the content area opens the link without dismissing the bookmarks menu is identical to the Firefox 2 behavior. So,k everything seems to behave as expected so far in my testing without any of this code. The code was all originally added to fix bug 329733.

(In reply to comment #16) > So,k everything seems to behave as expected so far in my testing without any of > this code. Well this was not very clear what I was trying to say is everything seems to work as expected with this patch installed which removes all of this code. I did not mean things work without the patch.

I figured out what was breaking the close window button here and it is not as bad as it originally seemed. The problem was that besides this timer code closing the menu in cases when it should not have, it was not doing it correctly either. It is perfectly normal for the close button to be ignored when a menu is open. A click on the close button is treated the same as any other click outside the menu, it does not close the window, but merely dismisses the menu. The bogus close code based on these timers was kind of closing the menu but not clearing all the appropriate menu is open conditions such that the click on the close button was still being ignored because a flag was set saying a menu was open but the click was not clearing the flag since there was not really a menu open. So, not matter how many times you clicked, the window would not close.

I should have figured it was too good to be true that all of that code could be removed. Someone noted a regression using my test build. "if I drag an item from the toolbar/location bar to the menu the menu doesn't open like in trunk." This is a more conservative patch which merely removes the timer that causes the issue. I had the tester verify that this cleared the new regression that the previous patch introduced.

this is about the same thing i've done in the last month to test bookmarks menus D&D so i can somehow confirm that makes things better.

Comment on attachment 306614 [details] [diff] [review] patch v2 r=mano

Mano: should we also remove the _closePopups functions since it's no more used (is called by that timer) or do you want to retain it for future work?

mozilla/browser/base/content/browser-menubar.inc 1.134 mozilla/browser/base/content/browser-places.js 1.103 mozilla/browser/base/content/browser.xul 1.438

No, I don't mind, too bad i noticed only after checkin...

Comment on attachment 306614 [details] [diff] [review] patch v2 This completely landed without approval. Please see the top of tinderbox or http://wiki.mozilla.org/TreeStatus before landing things without approval.

Backed out.

As long as this needs to be re-done, may as well remove _closePopups.

Comment on attachment 306785 [details] [diff] [review] patch v3 a=beltzner for 1.9b4, please land it tonight to get it into nightlies

Checking in browser/base/content/browser-menubar.inc; /cvsroot/mozilla/browser/base/content/browser-menubar.inc,v <-- browser-menubar.inc new revision: 1.136; previous revision: 1.135 done Checking in browser/base/content/browser-places.js; /cvsroot/mozilla/browser/base/content/browser-places.js,v <-- browser-places.js new revision: 1.105; previous revision: 1.104 done Checking in browser/base/content/browser.xul; /cvsroot/mozilla/browser/base/content/browser.xul,v <-- browser.xul new revision: 1.440; previous revision: 1.439 done

I ran into this a while ago and filed it as bug #407633. I wonder if I should close that bug as a duplicate of this one and assume my analysis was wrong, or if there's still a core bug lurking.

My advice would be to try to run builds not containing the patch you have int hat bug and try to trigger the issue. If you can document the steps to reproduce in that bug. If you are unable to trigger the issue, I would then probably close it as a dupe.

Reproduced on OS: Linux/Ubuntu 8.04 build: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008060309 Firefox/3.0 I played around quite a bit before I got it, but if you're not on top of the Bookmarks drop-down while dragging, menu disappears and the browser window doesn't close from the button (but closes from File -> Quit). Reopened bug + marked as minor.

Paul: please file a new bug for that issue? It's not clear that it's the same problem as the one fixed here, and we're not going to be backing this patch out.

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