Taking advantage of the new Notifier queuing, group item deletes into a single Notifier event queue -- this makes multi-item deletes much, much faster

I'm doing this manually with Notifier.begin(true)/commit(unlock) instead of putting them all in a DB transaction since Item.erase() erases files too, so rolling back previous deletes would be bad

Notice the finally {...} block -- this ensures that the event queue is unlocked even if there's an error deleting (or else notifications would break until Firefox was restarted)

Other changes:

- Zotero.Items.erase() now supports multiple items and the recursive flag to erase children
- Fixed a few JS strict warnings in the items view

chrome/content/zotero/xpcom/data_access.js

@@ -1828,10 +1828,8 @@ Zotero.Item.prototype.erase = function(deleteChildren){
 	}
 	
 	// If we're not in the middle of a larger commit, trigger the notifier now
-	if (!Zotero.DB.transactionInProgress()){
 	Zotero.Notifier.trigger('delete', 'item', this.getID());
 }
-}
 
 
 Zotero.Item.prototype.isCollection = function(){
@@ -2178,12 +2176,27 @@ Zotero.Items = new function(){
 	
 	
 	/**
-	* Delete item from database and clear from internal array
+	* Delete item(s) from database and clear from internal array
+	*
+	* If _eraseChildren_ is true, erase child items as well
 	**/
-	function erase(id){
+	function erase(ids, eraseChildren){
-		var obj = this.get(id);
+		var unlock = Zotero.Notifier.begin(true);
-		obj.erase(); // calls unload()
+		try {
-		obj = undefined;
+			for each(var id in ids) {
+				var item = this.get(id);
+				if (!item) {
+					Zotero.debug('Item ' + id + ' does not exist in Items.erase()!', 1);
+					Zotero.Notifier.trigger('item', 'delete', id);
+					continue;
+				}
+				item.erase(eraseChildren); // calls unload()
+				item = undefined;
+			}
+		}
+		finally {
+			Zotero.Notifier.commit(unlock);
+		}
 	}
 	
 	

chrome/content/zotero/xpcom/itemTreeView.js

@@ -119,9 +119,13 @@ Zotero.ItemTreeView.prototype.notify = function(action, type, ids)
 		{
 			if(action == 'delete' || !this._itemGroup.ref.hasItem(ids[i]))
 			{
+				// Row might already be gone (e.g. if this is a child and
+				// 'modify' was sent to parent)
+				if (this._itemRowMap[ids[i]] != undefined) {
 					rows.push(this._itemRowMap[ids[i]]);
 				}
 			}
+		}
 			
 		if(rows.length > 0)
 		{
@@ -469,6 +473,7 @@ Zotero.ItemTreeView.prototype.toggleOpenState = function(row)
 		else if(notes)
 			newRows = notes;
 		
+		if (newRows) {
 			newRows = Zotero.Items.get(newRows);
 			
 			for(var i = 0; i < newRows.length; i++)
@@ -477,6 +482,7 @@ Zotero.ItemTreeView.prototype.toggleOpenState = function(row)
 				this._showItem(new Zotero.ItemTreeView.TreeRow(newRows[i], thisLevel + 1, false), row + i + 1); // item ref, before row
 			}
 		}
+	}
 	
 	this._treebox.beginUpdateBatch();
 	
@@ -655,24 +661,27 @@ Zotero.ItemTreeView.prototype.deleteSelection = function(eraseChildren, force)
 	this._refreshHashMap();
 	
 	//create an array of selected items
-	var items = new Array();
+	var ids = [];
-	var start = new Object();
+	var start = {};
-	var end = new Object();
+	var end = {};
 	for (var i=0, len=this.selection.getRangeCount(); i<len; i++)
 	{
 		this.selection.getRangeAt(i,start,end);
 		for (var j=start.value; j<=end.value; j++)
-			items.push(this._getItemAtRow(j));
+			ids.push(this._getItemAtRow(j).ref.getID());
 	}
 	
 	//iterate and erase...
 	this._treebox.beginUpdateBatch();
-	for (var i=0; i<items.length; i++)
+	
-	{
+	// Erase item(s) from DB
-		if(this._itemGroup.isLibrary() || force) //erase item from DB
+	if (this._itemGroup.isLibrary() || force) {
-			items[i].ref.erase(eraseChildren);
+		Zotero.Items.erase(ids, eraseChildren);
-		else if(this._itemGroup.isCollection())
+	}
-			this._itemGroup.ref.removeItem(items[i].ref.getID());
+	else if (this._itemGroup.isCollection()) {
+		for each(var id in ids) {
+			this._itemGroup.ref.removeItem(id);
+		}
 	}
 	this._treebox.endUpdateBatch();
 }