STR: 1. Open the Library 2. Click "All Bookmarks" in the left tree. 3. Drag the "Bookmarks Menu" folder in the content tree and drop it into itself or into "Bookmarks Menu" in the left tree. Expected: Dropping is not permitted. Actual results: The folder disappears from "All Bookmarks" on the left side. It's still shown on the right side, temporarily. While it's there, you can still drag it back to "All Bookmarks". Otherwise "Bookmarks Menu" also appears in the Bookmarks menu, from where you can drag it.

Blocking for another potential to hoark one's Places database ...

mconnor told a terrific story about how the Mac menu indexer followed this to infinity and beyond.

Postponing review until resolution of bug 384370, due to changes in the wrapNode function.

On top of not allowing a folder to be dropped on it self, the patch will also disable the option of dropping a folder on any of its descendants.

Comment on attachment 309442 [details] [diff] [review] v2 >+ // Get the information of the dragged item >+ var xferable = this._initTransferable(session); >+ session.getData(xferable, 0); >+ var data = { }, flavor = { }; >+ xferable.getAnyTransferData(flavor, data, { }); >+ data.value.QueryInterface(Ci.nsISupportsString); >+ var dragged = PlacesUtils.unwrapNodes(data.value.data, flavor.value)[0]; Each dragged node should be checked (and then you should disallow dropping if _any_ of the items cannot be dropped). >+ // The following loop disallows the dropping of a folder on itself or >+ // on any of its descendants >+ if (dragged.type == PlacesUtils.TYPE_X_MOZ_PLACE_CONTAINER || >+ /^place/.test(dragged.uri)) { nit: "^place:".

Comment on attachment 309457 [details] [diff] [review] v2.1 >Index: browser/components/places/content/controller.js >=================================================================== >- canDrop: function PCDH_canDrop() { >+ canDrop: function PCDH_canDrop(ip) { > var session = this.getSession(); >- if (session) { >- var types = PlacesUIUtils.GENERIC_VIEW_DROP_TYPES; >- for (var i = 0; i < types.length; ++i) { >- if (session.isDataFlavorSupported(types[i])) >- return true; >+ var xferable = this._initTransferable(session); >+ var dropCount = session.numDropItems; >+ if (!session) >+ return false; >+ The order here is wrong, numDropItems should be used only once |session| is known to be non null. r=mano otherwise.

> + for (var i = 0; i < types.length; ++i) { I'm changing this to for(i =0 ... on checkin, |i| is declared in the first loop within this function.

Also, do the type-check first.

Wrong file :-/

I forgot to change the last return statement.

mozilla/browser/base/content/browser-places.js 1.119 mozilla/browser/components/places/content/controller.js 1.222 mozilla/browser/components/places/content/menu.xml 1.122 mozilla/browser/components/places/content/toolbar.xml 1.145 mozilla/browser/components/places/content/treeView.js 1.59 mozilla/toolkit/components/places/src/utils.js 1.4

verified with: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b5pre) Gecko/2008031704 Minefield/3.0b5pre and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre) Gecko/2008031704 Minefield/3.0b5pre Windows displays the circle / symbol. Mac provides no discouraging feedback btu doesn't permit the drop.

Test case 5947 has been updated to take this bug into account.

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