1. remove document.getElementById from call to OpenBookmarkURL, use this instead 2. rename innermostBox to NC:PersonalToolbarFolder avoiding NODE_ID function 3. also fixes open in new window for the personal toolbar

I am all for this very neat patch! Few nits: openFolder: function (aSelectedItem) { var mbo = aSelectedItem.boxObject.QueryInterface(Components.interfaces.nsIMenuBoxObject); - mbo.openMenu(true); + setTimeout(mbo.openMenu, 0, true); }, I would simply prefer: openFolder: function (aSelectedItem) { aSelectedItem.firstChild.showPopup(); } openMenu is not used except in popup.xml and is a low level method. The use of the timer should be set in the showPopup method in popup.xml. in navigatorDD.js, personalToolbarDNDObserver.onDrop if (aEvent.target.id == "bookmarks-button") // dropPosition is always DROP_ON parentContainer = RDFUtils.getResource("NC:BookmarksRoot"); else if (aEvent.target.id == "NC:PersonalToolbarFolder") parentContainer = RDFUtils.getResource("NC:PersonalToolbarFolder"); else if (dropPosition == this.DROP_ON) parentContainer = RDFUtils.getResource(aEvent.target.id); else { You can remove the case for NC:PersonalToolbarFolder, since we will have dropPosition == this.DROP_ON and it will be handled by the next case. With that, r=pierrechanial@netscape.net go go go!!!

fyi, bookmarksTree.js (bug 142236) and bookmarksDD.js (bug 140414) are obsolete and should be removed from the CVS repository.

This doesn't include the setTimeout hack, I'll address that in another bug.

CCing pierrechanial@netscape.net for review.

Comment on attachment 90306 [details] [diff] [review] Diffed properly this time... r=pierrechanial@netscape.net

Comment on attachment 90306 [details] [diff] [review] Diffed properly this time... sr=ben@netscape.com

Fix was checked in by bzbarsky@mit.edu