From dd477e15b8b649020fee74da9cd93c2912cdef4e Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Wed, 7 May 2014 04:22:49 -0400
Subject: [PATCH] Configurable secondary sorting and other improvements

- Each column in the middle pane can now have its own persistent
  secondary sort column, configurable from a new submenu in the column
  picker menu (top right of items list). The settings are stored in
  extensions.zotero.secondarySort.[primaryField]. The submenu title
  includes the current primary field (e.g., "Secondary Sort (Creator)"),
  which is pretty weird, and I'm not sure I want to keep it, but it does
  convey that the setting is specific to the selected column.

- The fallback sort fields (firstCreator, date, title, dateAdded) are
  now configurable via the extensions.zotero.fallbackSort. Setting that
  pref to an empty string avoids all fallback sorts, which
  allows reverse-order clicking to set the order, as requested by
  @aurimasv in #275.

- The previous behavior of sorting based on the exact Creator string
  (rather than the actual creators) can now be restored with the
  extensions.zotero.sortCreatorAsString pref. (It simply circumvents all
  the newer code, so it's pretty safe.) This setting should result in
  faster sorting in large libraries that have many items with the same
  Creator string.

- Some of the lesser fields in the column picker menu are now in the
  More Columns submenu (which is now alphabetical)

- The "Type" column is now the less-ambiguous "Item Type".

- This uses a different method to modify the column picker menu that is
  simultaneously less and more hacky. (It no longer has to duplicate
  Mozilla code in a custom XBL binding that wouldn't reflect future
  upstream changes, and instead it bushwhacks its way through various
  boxObject properties to get to the underlying menupopup.)
---
 .../content/zotero/bindings/columnpicker.xml  | 146 -----
 .../zotero/integration/addCitationDialog.xul  |   2 +-
 chrome/content/zotero/selectItemsDialog.xul   |   2 +-
 chrome/content/zotero/xpcom/itemTreeView.js   | 497 +++++++++++-------
 chrome/content/zotero/zoteroPane.js           |  19 +
 chrome/content/zotero/zoteroPane.xul          |   6 +-
 chrome/locale/en-US/zotero/zotero.dtd         |   2 +-
 chrome/locale/en-US/zotero/zotero.properties  |   2 +
 chrome/skin/default/zotero/zotero.css         |   5 -
 defaults/preferences/zotero.js                |   3 +
 10 files changed, 341 insertions(+), 343 deletions(-)
 delete mode 100644 chrome/content/zotero/bindings/columnpicker.xml

diff --git a/chrome/content/zotero/bindings/columnpicker.xml b/chrome/content/zotero/bindings/columnpicker.xml
deleted file mode 100644
index c406f2213..000000000
--- a/chrome/content/zotero/bindings/columnpicker.xml
+++ /dev/null
@@ -1,146 +0,0 @@
-<?xml version="1.0"?>
-<!--
-    ***** BEGIN LICENSE BLOCK *****
-    
-    Copyright © 2012 Center for History and New Media
-                     George Mason University, Fairfax, Virginia, USA
-                     http://zotero.org
-    
-    The Initial Developer of this code is Gracile. Portions created by the
-    Initial Developer are Copyright © 2012 the Initial Developer.
-    
-    This file contains code derived from Mozilla's tree.xml, © 2012 its
-    developers.
-    
-    This file is part of Zotero.
-    
-    Zotero is free software: you can redistribute it and/or modify
-    it under the terms of the GNU Affero General Public License as published by
-    the Free Software Foundation, either version 3 of the License, or
-    (at your option) any later version.
-    
-    Zotero is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU Affero General Public License for more details.
-    
-    You should have received a copy of the GNU Affero General Public License
-    along with Zotero.  If not, see <http://www.gnu.org/licenses/>.
-    
-    ***** END LICENSE BLOCK *****
--->
-
-<!DOCTYPE bindings SYSTEM "chrome://zotero/locale/zotero.dtd">
-
-<bindings xmlns="http://www.mozilla.org/xbl"
-			xmlns:xbl="http://www.mozilla.org/xbl"
-			xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
-	<binding id="extended-columnpicker" display="xul:button"
-				 extends="chrome://global/content/bindings/tree.xml#tree-base">
-		<content>
-			<xul:image class="tree-columnpicker-icon"/>
-			<xul:menupopup anonid="zotero-items-column-main-menu">
-				<xul:menuseparator/>
-				<xul:menu label="&zotero.items.moreColumns.label;">
-					<xul:menupopup anonid="zotero-items-column-sub-menu"/>
-				</xul:menu>
-				<xul:menuseparator/>
-				<xul:menuitem anonid="menuitem" label="&zotero.items.restoreColumnOrder.label;"/>
-			</xul:menupopup>
-		</content>
-
-		<implementation implements="nsIAccessibleProvider">
-			<property name="accessibleType" readonly="true">
-				<getter>
-					return Components.interfaces.nsIAccessibleProvider.XULButton;
-				</getter>
-			</property>
-
-			<method name="buildPopup">
-				<parameter name="aPopup"/>
-				<parameter name="bPopup"/>
-				<body>
-					<![CDATA[
-					// We no longer cache the picker content, remove the old content.
-					while (aPopup.childNodes.length > 4) {
-						aPopup.removeChild(aPopup.firstChild);
-					}
-					while (bPopup.childNodes.length > 0) {
-						bPopup.removeChild(bPopup.firstChild);
-					}
-					var refChild = aPopup.firstChild;
-					var refChild2 = bPopup.firstChild;
-					
-					var tree = this.parentNode.parentNode;
-					for (var currCol = tree.columns.getFirstColumn(); currCol; currCol = currCol.getNext()) {
-						var currElement = currCol.element;
-					
-						// Construct an entry for each column in the row, unless
-						// it is not being shown.
-						if ((!currElement.hasAttribute("ignoreincolumnpicker")) && (!currElement.hasAttribute("submenu"))) {
-							var popupChild = document.createElement("menuitem");
-							popupChild.setAttribute("type", "checkbox");
-							var columnName = currElement.getAttribute("display") || currElement.getAttribute("label");
-							popupChild.setAttribute("label", columnName);
-							popupChild.setAttribute("colindex", currCol.index);
-							if (currElement.getAttribute("hidden") != "true") {
-								popupChild.setAttribute("checked", "true");
-							}
-							if (currCol.primary) {
-								popupChild.setAttribute("disabled", "true");
-							}
-							aPopup.insertBefore(popupChild, refChild);
-						}
-					
-						//Idem for the submenu
-						if ((!currElement.hasAttribute("ignoreincolumnpicker")) && (currElement.hasAttribute("submenu"))) {
-							var popupChild = document.createElement("menuitem");
-							popupChild.setAttribute("type", "checkbox");
-							var columnName = currElement.getAttribute("display") || currElement.getAttribute("label");
-							popupChild.setAttribute("label", columnName);
-							popupChild.setAttribute("colindex", currCol.index);
-							if (currElement.getAttribute("hidden") != "true") {
-								popupChild.setAttribute("checked", "true");
-							}
-							bPopup.insertBefore(popupChild, refChild2);
-						}
-					}
-					]]>
-				</body>
-			</method>
-		</implementation>
-
-		<handlers>
-			<handler event="command">
-				<![CDATA[
-				if (event.originalTarget == this) {
-					var popup = document.getAnonymousElementByAttribute(this, "anonid", "zotero-items-column-main-menu");
-					var popup2 = document.getAnonymousElementByAttribute(this, "anonid", "zotero-items-column-sub-menu");
-					this.buildPopup(popup, popup2);
-					popup.showPopup(this, -1, -1, "popup", "bottomright", "topright");
-
-				} else {
-					var tree = this.parentNode.parentNode;
-					tree.stopEditing(true);
-					var menuitem = document.getAnonymousElementByAttribute(this, "anonid", "menuitem");
-					if (event.originalTarget == menuitem) {
-						tree.columns.restoreNaturalOrder();
-						tree._ensureColumnOrder();
-					} else {
-						var colindex = event.originalTarget.getAttribute("colindex");
-						var column = tree.columns[colindex];
-						if (column) {
-							var element = column.element;
-							if (element.getAttribute("hidden") == "true") {
-								element.setAttribute("hidden", "false");
-							} else {
-								element.setAttribute("hidden", "true");
-							}
-						}
-					}
-				}
-				]]>
-			</handler>
-		</handlers>
-	</binding>
-</bindings>
\ No newline at end of file
diff --git a/chrome/content/zotero/integration/addCitationDialog.xul b/chrome/content/zotero/integration/addCitationDialog.xul
index 1888eec78..7e4bab6d0 100644
--- a/chrome/content/zotero/integration/addCitationDialog.xul
+++ b/chrome/content/zotero/integration/addCitationDialog.xul
@@ -90,7 +90,7 @@
 									flex="1" persist="width ordinal hidden sortActive sortDirection"/>
 								<splitter class="tree-splitter"/>
 								<treecol
-									id="zotero-items-column-type" hidden="true"
+									id="zotero-items-column-itemType" hidden="true"
 									label="&zotero.items.type_column;"
 									width="40" persist="width ordinal hidden sortActive sortDirection"/>
 								<splitter class="tree-splitter"/>
diff --git a/chrome/content/zotero/selectItemsDialog.xul b/chrome/content/zotero/selectItemsDialog.xul
index 5967999cb..e356ce528 100644
--- a/chrome/content/zotero/selectItemsDialog.xul
+++ b/chrome/content/zotero/selectItemsDialog.xul
@@ -82,7 +82,7 @@
 					flex="1" persist="width ordinal hidden sortActive sortDirection"/>
 				<splitter class="tree-splitter"/>
 				<treecol
-					id="zotero-items-column-type" hidden="true"
+					id="zotero-items-column-itemType" hidden="true"
 					label="&zotero.items.type_column;"
 					width="40" persist="width ordinal hidden sortActive sortDirection"/>
 				<splitter class="tree-splitter"/>
diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
index 8ad2453fa..ceb4e7d16 100644
--- a/chrome/content/zotero/xpcom/itemTreeView.js
+++ b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -938,8 +938,7 @@ Zotero.ItemTreeView.prototype.getCellText = function(row, column)
 	if (column.id === "zotero-items-column-hasAttachment") {
 		return;
 	}
-	else if(column.id == "zotero-items-column-type")
-	{
+	else if (column.id == "zotero-items-column-itemType") {
 		val = Zotero.ItemTypes.getLocalizedString(obj.ref.itemTypeID);
 	}
 	// Year column is just date field truncated
@@ -1295,14 +1294,14 @@ Zotero.ItemTreeView.prototype.cycleHeader = function(column)
  */
 Zotero.ItemTreeView.prototype.sort = function(itemID)
 {
+	var t = new Date;
+	
 	// If Zotero pane is hidden, mark tree for sorting later in setTree()
 	if (!this._treebox.columns) {
 		this._needsSort = true;
 		return;
 	}
-	else {
-		this._needsSort = false;
-	}
+	this._needsSort = false;
 	
 	// Single child item sort -- just toggle parent open and closed
 	if (itemID && this._itemRowMap[itemID] &&
@@ -1313,36 +1312,19 @@ Zotero.ItemTreeView.prototype.sort = function(itemID)
 		return;
 	}
 	
-	var columnField = this.getSortField();
-	var order = this.getSortDirection() == 'descending';
+	var primaryField = this.getSortField();
+	var sortFields = this.getSortFields();
+	var dir = this.getSortDirection();
+	var order = dir == 'descending' ? -1 : 1;
 	var collation = Zotero.getLocaleCollation();
+	var sortCreatorAsString = Zotero.Prefs.get('sortCreatorAsString');
 	
-	// Year is really the date field truncated
-	var originalColumnField = columnField;
-	if (columnField == 'year') {
-		columnField = 'date';
-	}
-	
-	// The visible fields affect the secondary sorting
-	var visibleFields = {};
-	this.getVisibleFields().forEach(function (val) {
-		visibleFields[val] = true;
-	});
-	
-	// Some fields (e.g. dates) need to be retrieved unformatted for sorting
-	switch (columnField) {
-		case 'date':
-			var unformatted = true;
-			break;
-		
-		default:
-			var unformatted = false;
-	}
+	Zotero.debug("Sorting items list by " + sortFields.join(", ") + " " + dir);
 	
 	// Set whether rows with empty values should be displayed last,
 	// which may be different for primary and secondary sorting.
 	var emptyFirst = {};
-	switch (columnField) {
+	switch (primaryField) {
 	case 'title':
 		emptyFirst.title = true;
 		break;
@@ -1357,36 +1339,22 @@ Zotero.ItemTreeView.prototype.sort = function(itemID)
 	// Cache primary values while sorting, since base-field-mapped getField()
 	// calls are relatively expensive
 	var cache = {};
+	sortFields.forEach(function (x) cache[x] = {})
 	
 	// Get the display field for a row (which might be a placeholder title)
-	var getField;
-	switch (originalColumnField) {
-		case 'title':
-			getField = function (row) {
-				var field;
-				var type = row.ref.itemTypeID;
-				switch (type) {
-					case 8: // letter
-					case 10: // interview
-					case 17: // case
-						field = row.ref.getDisplayTitle();
-						break;
-					
-					default:
-						field = row.getField(columnField, unformatted);
-				}
-				// Ignore some leading and trailing characters when sorting
-				return Zotero.Items.getSortTitle(field);
-			};
-			break;
+	function getField(field, row) {
+		var item = row.ref;
 		
-		case 'hasAttachment':
-			getField = function (row) {
-				if (row.ref.isAttachment()) {
-					var state = row.ref.fileExists() ? 1 : -1;
+		switch (field) {
+			case 'title':
+				return Zotero.Items.getSortTitle(item.getDisplayTitle());
+			
+			case 'hasAttachment':
+				if (item.isAttachment()) {
+					var state = item.fileExists() ? 1 : -1;
 				}
-				else if (row.ref.isRegularItem()) {
-					var state = row.ref.getBestAttachmentState();
+				else if (item.isRegularItem()) {
+					var state = item.getBestAttachmentState();
 				}
 				else {
 					return 0;
@@ -1396,20 +1364,23 @@ Zotero.ItemTreeView.prototype.sort = function(itemID)
 					state = 2;
 				}
 				return state * -1;
-			};
-			break;
-		
-		case 'numNotes':
-			getField = function (row) {
-				// Sort descending by default
-				order = !order;
+			
+			case 'numNotes':
 				return row.numNotes(false, true) || 0;
-			};
-			break;
-		
-		case 'year':
-			getField = function (row) {
-				var val = row.getField(columnField, unformatted);
+			
+			// Use unformatted part of date strings (YYYY-MM-DD) for sorting
+			case 'date':
+				var val = row.ref.getField('date', true);
+				if (val) {
+					val = val.substr(0, 10);
+					if (val.indexOf('0000') == 0) {
+						val = "";
+					}
+				}
+				return val;
+			
+			case 'year':
+				var val = row.ref.getField('date', true);
 				if (val) {
 					val = val.substr(0, 4);
 					if (val == '0000') {
@@ -1417,127 +1388,58 @@ Zotero.ItemTreeView.prototype.sort = function(itemID)
 					}
 				}
 				return val;
-			};
-			break;
-		
-		default:
-			getField = function (row) row.getField(columnField, unformatted);
+			
+			default:
+				return row.ref.getField(field, false, true);
+		}
 	}
 	
 	var includeTrashed = this._itemGroup.isTrash();
 	
-	var me = this,
-		isEmptyFirst = emptyFirst[columnField];
-	function rowSort(a, b) {
-		var cmp,
-			aItemID = a.id,
-			bItemID = b.id,
-			fieldA = cache[aItemID],
-			fieldB = cache[bItemID];
+	function fieldCompare(a, b, sortField) {
+		var aItemID = a.id;
+		var bItemID = b.id;
+		var fieldA = cache[sortField][aItemID];
+		var fieldB = cache[sortField][bItemID];
 		
-		switch (columnField) {
-			case 'date':
-				fieldA = getField(a).substr(0, 10);
-				fieldB = getField(b).substr(0, 10);
-				
-				cmp = strcmp(fieldA, fieldB);
-				if (cmp !== 0) {
-					return cmp;
-				}
-				break;
-			
+		switch (sortField) {
 			case 'firstCreator':
-				cmp = creatorSort(a, b);
-				if (cmp !== 0) {
-					return cmp;
-				}
-				break;
+				return creatorSort(a, b);
 			
-			case 'type':
+			case 'itemType':
 				var typeA = Zotero.ItemTypes.getLocalizedString(a.ref.itemTypeID);
 				var typeB = Zotero.ItemTypes.getLocalizedString(b.ref.itemTypeID);
-				
-				cmp = (typeA > typeB) ? -1 : (typeA < typeB) ? 1 : 0;
-				if (cmp !== 0) {
-					return cmp;
-				}
-				break;
+				return (typeA > typeB) ? 1 : (typeA < typeB) ? -1 : 0;
 				
 			default:
 				if (fieldA === undefined) {
-					cache[aItemID] = fieldA = getField(a);
+					cache[sortField][aItemID] = fieldA = getField(sortField, a);
 				}
 				
 				if (fieldB === undefined) {
-					cache[bItemID] = fieldB = getField(b);
+					cache[sortField][bItemID] = fieldB = getField(sortField, b);
 				}
 				
 				// Display rows with empty values last
-				if (!isEmptyFirst) {
-					if(fieldA === '' && fieldB !== '') return -1;
-					if(fieldA !== '' && fieldB === '') return 1;
+				if (!emptyFirst[sortField]) {
+					if(fieldA === '' && fieldB !== '') return 1;
+					if(fieldA !== '' && fieldB === '') return -1;
 				}
 				
-				cmp = collation.compareString(1, fieldB, fieldA);
-				if (cmp !== 0) {
-					return cmp;
-				}
+				return collation.compareString(1, fieldA, fieldB);
 		}
-		
-		if (columnField !== 'firstCreator') {
-			cmp = creatorSort(a, b);
+	}
+	
+	function rowSort(a, b) {
+		var sortFields = Array.slice(arguments, 2);
+		var sortField;
+		while (sortField = sortFields.shift()) {
+			let cmp = fieldCompare(a, b, sortField);
 			if (cmp !== 0) {
 				return cmp;
 			}
 		}
-		
-		if (columnField !== 'date') {
-			// If year is visible and not date, don't use full date
-			if (visibleFields.year && !visibleFields.date) {
-				fieldA = a.getField('date', true).substr(0, 4);
-				if (fieldA == '0000') {
-					fieldA = "";
-				}
-				fieldB = b.getField('date', true).substr(0, 4);
-				if (fieldB == '0000') {
-					fieldB = "";
-				}
-				
-				cmp = strcmp(fieldA, fieldB);
-				if (cmp !== 0) {
-					return cmp;
-				}
-			}
-			// Otherwise use full date, even if Date column is hidden
-			else {
-				fieldA = a.getField('date', true).substr(0, 10);
-				fieldB = b.getField('date', true).substr(0, 10);
-				
-				cmp = strcmp(fieldA, fieldB);
-				if (cmp !== 0) {
-					return cmp;
-				}
-			}
-		}
-		
-		if (columnField !== 'title') {
-			fieldA = a.getField('title', true);
-			fieldB = b.getField('title', true);
-			
-			if (!emptyFirst.title) {
-				if (fieldA === '' && fieldB !== '') return -1;
-				if (fieldA !== '' && fieldB === '') return 1;
-			}
-			
-			cmp = collation.compareString(1, fieldB, fieldA);
-			if (cmp !== 0) {
-				return cmp;
-			}
-		}
-		
-		fieldA = a.getField('dateAdded');
-		fieldB = b.getField('dateAdded');
-		return (fieldA > fieldB) ? -1 : (fieldA < fieldB) ? 1 : 0;
+		return 0;
 	}
 	
 	var firstCreatorSortCache = {};
@@ -1565,8 +1467,8 @@ Zotero.ItemTreeView.prototype.sort = function(itemID)
 			return 0;
 		}
 		
-		var cmp = strcmp(fieldA, fieldB, true);
-		if (cmp !== 0) {
+		var cmp = strcmp(fieldA, fieldB);
+		if (cmp !== 0 || sortCreatorAsString) {
 			return cmp;
 		}
 		
@@ -1663,14 +1565,14 @@ Zotero.ItemTreeView.prototype.sort = function(itemID)
 			// Compare names
 			fieldA = Zotero.Items.getSortTitle(aCreators[aPos].ref.lastName);
 			fieldB = Zotero.Items.getSortTitle(bCreators[bPos].ref.lastName);
-			cmp = strcmp(fieldA, fieldB, true);
+			cmp = strcmp(fieldA, fieldB);
 			if (cmp) {
 				return cmp;
 			}
 			
 			fieldA = Zotero.Items.getSortTitle(aCreators[aPos].ref.firstName);
 			fieldB = Zotero.Items.getSortTitle(bCreators[bPos].ref.firstName);
-			cmp = strcmp(fieldA, fieldB, true);
+			cmp = strcmp(fieldA, fieldB);
 			if (cmp) {
 				return cmp;
 			}
@@ -1728,14 +1630,10 @@ Zotero.ItemTreeView.prototype.sort = function(itemID)
 	
 	function strcmp(a, b, collationSort) {
 		// Display rows with empty values last
-		if(a === '' && b !== '') return -1;
-		if(a !== '' && b === '') return 1;
+		if (a === '' && b !== '') return 1;
+		if (a !== '' && b === '') return -1;
 		
-		if (collationSort) {
-			return collation.compareString(1, b, a);
-		}
-		
-		return (a > b) ? -1 : (a < b) ? 1 : 0;
+		return collation.compareString(1, a, b);
 	}
 	
 	// Need to close all containers before sorting
@@ -1748,23 +1646,19 @@ Zotero.ItemTreeView.prototype.sort = function(itemID)
 	
 	// Single-row sort
 	if (itemID) {
-		var row = this._itemRowMap[itemID];
-		for (var i=0, len=this._dataItems.length; i<len; i++) {
+		let row = this._itemRowMap[itemID];
+		for (let i=0, len=this._dataItems.length; i<len; i++) {
 			if (i === row) {
 				continue;
 			}
 			
-			if (order) {
-				var cmp = -1*rowSort(this._dataItems[i], this._dataItems[row]);
-			}
-			else {
-				var cmp = rowSort(this._dataItems[i], this._dataItems[row]);
-			}
+			let cmp = rowSort.apply(this,
+				[this._dataItems[i], this._dataItems[row]].concat(sortFields)) * order;
 			
 			// As soon as we find a value greater (or smaller if reverse sort),
 			// insert row at that position
-			if (cmp < 0) {
-				var rowItem = this._dataItems.splice(row, 1);
+			if (cmp > 0) {
+				let rowItem = this._dataItems.splice(row, 1);
 				this._dataItems.splice(row < i ? i-1 : i, 0, rowItem[0]);
 				this._treebox.invalidate();
 				break;
@@ -1772,7 +1666,7 @@ Zotero.ItemTreeView.prototype.sort = function(itemID)
 			
 			// If greater than last row, move to end
 			if (i == len-1) {
-				var rowItem = this._dataItems.splice(row, 1);
+				let rowItem = this._dataItems.splice(row, 1);
 				this._dataItems.splice(i, 0, rowItem[0]);
 				this._treebox.invalidate();
 			}
@@ -1780,8 +1674,9 @@ Zotero.ItemTreeView.prototype.sort = function(itemID)
 	}
 	// Full sort
 	else {
-		this._dataItems.sort(rowSort);
-		if(!order) this._dataItems.reverse();
+		this._dataItems.sort(function (a, b) {
+			return rowSort.apply(this, [a, b].concat(sortFields)) * order;
+		}.bind(this));
 	}
 	
 	this._refreshHashMap();
@@ -1793,6 +1688,8 @@ Zotero.ItemTreeView.prototype.sort = function(itemID)
 		this.selection.selectEventsSuppressed = false;
 		this._treebox.endUpdateBatch();
 	}
+	
+	Zotero.debug("Sorted items list in " + (new Date - t) + " ms");
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -2374,6 +2271,39 @@ Zotero.ItemTreeView.prototype.getSortField = function() {
 }
 
 
+Zotero.ItemTreeView.prototype.getSortFields = function () {
+	var fields = [this.getSortField()];
+	var secondaryField = this.getSecondarySortField();
+	if (secondaryField) {
+		fields.push(secondaryField);
+	}
+	try {
+		var fallbackFields = Zotero.Prefs.get('fallbackSort')
+			.split(',')
+			.map((x) => x.trim())
+			.filter((x) => x !== '');
+	}
+	catch (e) {
+		Zotero.debug(e, 1);
+		Components.utils.reportError(e);
+		// This should match the default value for the fallbackSort pref
+		var fallbackFields = ['firstCreator', 'date', 'title', 'dateAdded'];
+	}
+	fields = Zotero.Utilities.arrayUnique(fields.concat(fallbackFields));
+	
+	// If date appears after year, remove it, unless it's the explicit secondary sort
+	var yearPos = fields.indexOf('year');
+	if (yearPos != -1) {
+		let datePos = fields.indexOf('date');
+		if (datePos > yearPos && secondaryField != 'date') {
+			fields.splice(datePos, 1);
+		}
+	}
+	
+	return fields;
+}
+
+
 /*
  * Returns 'ascending' or 'descending'
  */
@@ -2386,6 +2316,197 @@ Zotero.ItemTreeView.prototype.getSortDirection = function() {
 }
 
 
+Zotero.ItemTreeView.prototype.getSecondarySortField = function () {
+	var primaryField = this.getSortField();
+	var secondaryField = Zotero.Prefs.get('secondarySort.' + primaryField);
+	if (!secondaryField || secondaryField == primaryField) {
+		return false;
+	}
+	return secondaryField;
+}
+
+
+Zotero.ItemTreeView.prototype.setSecondarySortField = function (secondaryField) {
+	var primaryField = this.getSortField();
+	var currentSecondaryField = this.getSecondarySortField();
+	var sortFields = this.getSortFields();
+	
+	if (primaryField == secondaryField) {
+		return false;
+	}
+	
+	if (currentSecondaryField) {
+		// If same as the current explicit secondary sort, ignore
+		if (currentSecondaryField == secondaryField) {
+			return false;
+		}
+		
+		// If not, but same as first implicit sort, remove current explicit sort
+		if (sortFields[2] && sortFields[2] == secondaryField) {
+			Zotero.Prefs.clear('secondarySort.' + primaryField);
+			return true;
+		}
+	}
+	// If same as current implicit secondary sort, ignore
+	else if (sortFields[1] && sortFields[1] == secondaryField) {
+		return false;
+	}
+	
+	Zotero.Prefs.set('secondarySort.' + primaryField, secondaryField);
+	return true;
+}
+
+
+/**
+ * Build the More Columns and Secondary Sort submenus while the popup is opening
+ */
+Zotero.ItemTreeView.prototype.onColumnPickerShowing = function (event) {
+	var menupopup = event.originalTarget;
+	
+	var ns = 'http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul';
+	var prefix = 'zotero-column-header-';
+	var doc = menupopup.ownerDocument;
+	
+	var anonid = menupopup.getAttribute('anonid');
+	if (anonid.indexOf(prefix) == 0) {
+		return;
+	}
+	
+	var lastChild = menupopup.lastChild;
+	
+	// More Columns menu
+	try {
+		let id = prefix + 'more-menu';
+		
+		let moreMenu = doc.createElementNS(ns, 'menu');
+		moreMenu.setAttribute('label', Zotero.getString('pane.items.columnChooser.moreColumns'));
+		moreMenu.setAttribute('anonid', id);
+		
+		let moreMenuPopup = doc.createElementNS(ns, 'menupopup');
+		moreMenuPopup.setAttribute('anonid', id + '-popup');
+		
+		let treecols = menupopup.parentNode.parentNode;
+		let subs = [x.getAttribute('label') for (x of treecols.getElementsByAttribute('submenu', 'true'))];
+		
+		var moreItems = [];
+		
+		for (let i=0; i<menupopup.childNodes.length; i++) {
+			let elem = menupopup.childNodes[i];
+			if (elem.localName == 'menuseparator') {
+				break;
+			}
+			if (elem.localName == 'menuitem' && subs.indexOf(elem.getAttribute('label')) != -1) {
+				moreItems.push(elem);
+			}
+		}
+		
+		// Sort fields and move to submenu
+		var collation = Zotero.getLocaleCollation();
+		moreItems.sort(function (a, b) {
+			return collation.compareString(1, a.getAttribute('label'), b.getAttribute('label'));
+		});
+		moreItems.forEach(function (elem) {
+			moreMenuPopup.appendChild(menupopup.removeChild(elem));
+		});
+		
+		moreMenu.appendChild(moreMenuPopup);
+		menupopup.insertBefore(moreMenu, lastChild);
+	}
+	catch (e) {
+		Components.utils.reportError(e);
+		Zotero.debug(e, 1);
+	}
+	
+	// Secondary Sort menu
+	try {
+		let id = prefix + 'sort-menu';
+		let primaryField = this.getSortField();
+		let sortFields = this.getSortFields();
+		let secondaryField = false;
+		if (sortFields[1]) {
+			secondaryField = sortFields[1];
+		}
+		
+		// Get localized names from treecols, since the names are currently done via .dtd
+		let treecols = menupopup.parentNode.parentNode;
+		let primaryFieldLabel = treecols.getElementsByAttribute('id',
+			'zotero-items-column-' + primaryField)[0].getAttribute('label');
+		
+		let sortMenu = doc.createElementNS(ns, 'menu');
+		sortMenu.setAttribute('label',
+			Zotero.getString('pane.items.columnChooser.secondarySort', primaryFieldLabel));
+		sortMenu.setAttribute('anonid', id);
+		
+		let sortMenuPopup = doc.createElementNS(ns, 'menupopup');
+		sortMenuPopup.setAttribute('anonid', id + '-popup');
+		
+		// Generate menuitems
+		let sortOptions = [
+			'title',
+			'firstCreator',
+			'itemType',
+			'date',
+			'year',
+			'publisher',
+			'publicationTitle',
+			'dateAdded',
+			'dateModified'
+		];
+		for (let i=0; i<sortOptions.length; i++) {
+			let field = sortOptions[i];
+			// Hide current primary field, and don't show Year for Date, since it would be a no-op
+			if (field == primaryField || (primaryField == 'date' && field == 'year')) {
+				continue;
+			}
+			let label = treecols.getElementsByAttribute('id',
+				'zotero-items-column-' + field)[0].getAttribute('label');
+			
+			let sortMenuItem = doc.createElementNS(ns, 'menuitem');
+			sortMenuItem.setAttribute('fieldName', field);
+			sortMenuItem.setAttribute('label', label);
+			sortMenuItem.setAttribute('type', 'checkbox');
+			if (field == secondaryField) {
+				sortMenuItem.setAttribute('checked', 'true');
+			}
+			sortMenuItem.setAttribute('oncommand',
+				'var view = ZoteroPane.itemsView; '
+				+ 'if (view.setSecondarySortField(this.getAttribute("fieldName"))) { view.sort(); }');
+			sortMenuPopup.appendChild(sortMenuItem);
+		}
+		
+		sortMenu.appendChild(sortMenuPopup);
+		menupopup.insertBefore(sortMenu, lastChild);
+	}
+	catch (e) {
+		Components.utils.reportError(e);
+		Zotero.debug(e, 1);
+	}
+	
+	sep = doc.createElementNS(ns, 'menuseparator');
+	sep.setAttribute('anonid', prefix + 'sep');
+	menupopup.insertBefore(sep, lastChild);
+}
+
+
+Zotero.ItemTreeView.prototype.onColumnPickerHidden = function (event) {
+	var menupopup = event.originalTarget;
+	var prefix = 'zotero-column-header-';
+	
+	for (let i=0; i<menupopup.childNodes.length; i++) {
+		let elem = menupopup.childNodes[i];
+		if (elem.getAttribute('anonid').indexOf(prefix) == 0) {
+			try {
+				menupopup.removeChild(elem);
+			}
+			catch (e) {
+				Zotero.debug(e, 1);
+			}
+			i--;
+		}
+	}
+}
+
+
 ////////////////////////////////////////////////////////////////////////////////
 ///
 ///  Command Controller:
diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js
index 1d4192dc0..709dbeeac 100644
--- a/chrome/content/zotero/zoteroPane.js
+++ b/chrome/content/zotero/zoteroPane.js
@@ -1185,6 +1185,25 @@ var ZoteroPane = new function()
 			this.itemsView.addCallback(_setTagScope);
 			document.getElementById('zotero-items-tree').view = this.itemsView;
 			this.itemsView.selection.clearSelection();
+			
+			// Add events to treecolpicker to update menu before showing/hiding
+			try {
+				let treecols = document.getElementById('zotero-items-columns-header');
+				let treecolpicker = treecols.boxObject.firstChild.nextSibling;
+				let menupopup = treecolpicker.boxObject.firstChild.nextSibling;
+				let attr = menupopup.getAttribute('onpopupshowing');
+				if (attr.indexOf('Zotero') == -1) {
+					menupopup.setAttribute('onpopupshowing', 'ZoteroPane.itemsView.onColumnPickerShowing(event);')
+						// Keep whatever else is there
+						+ ' ' + attr;
+					menupopup.setAttribute('onpopuphidden', 'ZoteroPane.itemsView.onColumnPickerHidden(event);')
+						// Keep whatever else is there
+						+ ' ' + menupopup.getAttribute('onpopuphidden');
+				}
+			}
+			catch (e) {
+				Zotero.debug(e);
+			}
 		}
 		finally {
 			Zotero.UnresponsiveScriptIndicator.enable();
diff --git a/chrome/content/zotero/zoteroPane.xul b/chrome/content/zotero/zoteroPane.xul
index ef20159f9..467b4ea48 100644
--- a/chrome/content/zotero/zoteroPane.xul
+++ b/chrome/content/zotero/zoteroPane.xul
@@ -359,7 +359,7 @@
 									flex="1" zotero-persist="width ordinal hidden sortActive sortDirection"/>
 								<splitter class="tree-splitter"/>
 								<treecol
-									id="zotero-items-column-type" hidden="true"
+									id="zotero-items-column-itemType" hidden="true"
 									label="&zotero.items.type_column;"
 									width="40" zotero-persist="width ordinal hidden sortActive sortDirection"/>
 								<splitter class="tree-splitter"/>
@@ -385,21 +385,25 @@
 								<splitter class="tree-splitter"/>
 								<treecol
 									id="zotero-items-column-journalAbbreviation" hidden="true"
+									submenu="true"
 									label="&zotero.items.journalAbbr_column;"
 									flex="1" zotero-persist="width ordinal hidden sortActive sortDirection"/>
 								<splitter class="tree-splitter"/>
 								<treecol
 									id="zotero-items-column-language" hidden="true"
+									submenu="true"
 									label="&zotero.items.language_column;"
 									flex="1" zotero-persist="width ordinal hidden sortActive sortDirection"/>
 								<splitter class="tree-splitter"/>
 								<treecol
 									id="zotero-items-column-accessDate" hidden="true"
+									submenu="true"
 									label="&zotero.items.accessDate_column;"
 									flex="1" zotero-persist="width ordinal hidden sortActive sortDirection"/>
 								<splitter class="tree-splitter"/>
 								<treecol
 									id="zotero-items-column-libraryCatalog" hidden="true"
+									submenu="true"
 									label="&zotero.items.libraryCatalog_column;"
 									flex="1" zotero-persist="width ordinal hidden sortActive sortDirection"/>
 								<splitter class="tree-splitter"/>
diff --git a/chrome/locale/en-US/zotero/zotero.dtd b/chrome/locale/en-US/zotero/zotero.dtd
index cd18041e4..2db5d59c5 100644
--- a/chrome/locale/en-US/zotero/zotero.dtd
+++ b/chrome/locale/en-US/zotero/zotero.dtd
@@ -45,7 +45,7 @@
 <!ENTITY zotero.collections.showUnfiledItems			"Show Unfiled Items">
 
 <!ENTITY zotero.items.itemType							"Item Type">
-<!ENTITY zotero.items.type_column						"Type">
+<!ENTITY zotero.items.type_column						"Item Type">
 <!ENTITY zotero.items.title_column						"Title">
 <!ENTITY zotero.items.creator_column					"Creator">
 <!ENTITY zotero.items.date_column						"Date">
diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties
index 8efb3a5a9..b05da82a3 100644
--- a/chrome/locale/en-US/zotero/zotero.properties
+++ b/chrome/locale/en-US/zotero/zotero.properties
@@ -192,6 +192,8 @@ tagColorChooser.numberKeyInstructions          = You can add this tag to selecte
 tagColorChooser.maxTags                        = Up to %S tags in each library can have colors assigned.
 
 pane.items.loading				= Loading items list…
+pane.items.columnChooser.moreColumns = More Columns
+pane.items.columnChooser.secondarySort = Secondary Sort (%S)
 pane.items.attach.link.uri.title	 	= Attach Link to URI
 pane.items.attach.link.uri		= Enter a URI:
 pane.items.trash.title			= Move to Trash
diff --git a/chrome/skin/default/zotero/zotero.css b/chrome/skin/default/zotero/zotero.css
index b833c493c..59df981c0 100644
--- a/chrome/skin/default/zotero/zotero.css
+++ b/chrome/skin/default/zotero/zotero.css
@@ -174,11 +174,6 @@ zoteroguidancepanel
 	-moz-binding: url('chrome://zotero/content/bindings/guidancepanel.xml#guidancepanel');
 }
 
-#zotero-items-columns-header  > treecolpicker
- {
-	-moz-binding: url('chrome://zotero/content/bindings/columnpicker.xml#extended-columnpicker');
-}
-
 zoterofilesyncstatus {
 	-moz-binding: url('chrome://zotero/content/bindings/filesyncstatus.xml#file-sync-status');
 }
diff --git a/defaults/preferences/zotero.js b/defaults/preferences/zotero.js
index 4054fb6f8..c7d94842b 100644
--- a/defaults/preferences/zotero.js
+++ b/defaults/preferences/zotero.js
@@ -61,6 +61,9 @@ pref("extensions.zotero.lastViewedFolder", 'L');
 pref("extensions.zotero.lastLongTagMode", 0);
 pref("extensions.zotero.lastLongTagDelimiter", ";");
 
+pref("extensions.zotero.fallbackSort", 'firstCreator,date,title,dateAdded');
+pref("extensions.zotero.sortCreatorAsString", false);
+
 //Tag Cloud
 pref("extensions.zotero.tagCloud", false);