From c6a2057340c010e95f79fad4c69775fc9690ba4f Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Mon, 14 Nov 2016 03:03:21 -0500
Subject: [PATCH] Fixes to hasAttachments column

- Fix #1121, sorting of hasAttachments column -- doesn't work if
  hasAttachments is the active sort column on startup, since file states
  have to be cached, but works for subsequent clicks on the column
- Don't sort linked URL attachments as missing
- Use hiDPI icon in all cases
---
 chrome/content/zotero/xpcom/itemTreeView.js | 39 ++++++++++++++++-----
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js
index bb0a6f5ab..8ca57c620 100644
--- a/chrome/content/zotero/xpcom/itemTreeView.js
+++ b/chrome/content/zotero/xpcom/itemTreeView.js
@@ -1116,6 +1116,7 @@ Zotero.ItemTreeView.prototype.getImageSrc = function(row, col)
 		}
 		
 		var itemID = item.id;
+		let suffix = Zotero.hiDPISuffix;
 		
 		if (treerow.level === 0) {
 			if (item.isRegularItem()) {
@@ -1123,10 +1124,10 @@ Zotero.ItemTreeView.prototype.getImageSrc = function(row, col)
 				if (state !== null) {
 					switch (state) {
 						case 1:
-							return "chrome://zotero/skin/bullet_blue.png";
+							return `chrome://zotero/skin/bullet_blue${suffix}.png`;
 						
 						case -1:
-							return "chrome://zotero/skin/bullet_blue_empty.png";
+							return `chrome://zotero/skin/bullet_blue_empty${suffix}.png`;
 						
 						default:
 							return "";
@@ -1145,7 +1146,6 @@ Zotero.ItemTreeView.prototype.getImageSrc = function(row, col)
 		if (item.isFileAttachment()) {
 			let exists = item.fileExistsCached();
 			if (exists !== null) {
-				let suffix = Zotero.hiDPISuffix;
 				return exists
 					? `chrome://zotero/skin/bullet_blue${suffix}.png`
 					: `chrome://zotero/skin/bullet_blue_empty${suffix}.png`;
@@ -1310,10 +1310,24 @@ Zotero.ItemTreeView.prototype.isSorted = function()
 	return true;
 }
 
-Zotero.ItemTreeView.prototype.cycleHeader = function (column) {
+Zotero.ItemTreeView.prototype.cycleHeader = Zotero.Promise.coroutine(function* (column) {
 	if (this.collectionTreeRow.isFeed()) {
 		return;
 	}
+	if (column.id == 'zotero-items-column-hasAttachment') {
+		Zotero.debug("Caching best attachment states");
+		if (!this._cachedBestAttachmentStates) {
+			let t = new Date();
+			for (let i = 0; i < this._rows.length; i++) {
+				let item = this.getRow(i).ref;
+				if (item.isRegularItem()) {
+					yield item.getBestAttachmentState();
+				}
+			}
+			Zotero.debug("Cached best attachment states in " + (new Date - t) + " ms");
+			this._cachedBestAttachmentStates = true;
+		}
+	}
 	for(var i=0, len=this._treebox.columns.count; i<len; i++)
 	{
 		col = this._treebox.columns.getColumnAt(i);
@@ -1353,7 +1367,7 @@ Zotero.ItemTreeView.prototype.cycleHeader = function (column) {
 	}
 	this._treebox.invalidate();
 	this.selection.selectEventsSuppressed = false;
-}
+});
 
 /*
  *  Sort the items by the currently sorted column.
@@ -1416,20 +1430,23 @@ Zotero.ItemTreeView.prototype.sort = function (itemID) {
 				return Zotero.Items.getSortTitle(item.getDisplayTitle());
 			
 			case 'hasAttachment':
-				if (item.isAttachment()) {
+				if (item.isFileAttachment()) {
 					var state = item.fileExistsCached() ? 1 : -1;
 				}
 				else if (item.isRegularItem()) {
-					var state = item.getBestAttachmentState();
+					var state = item.getBestAttachmentStateCached();
 				}
 				else {
 					return 0;
 				}
 				// Make sort order present, missing, empty when ascending
-				if (state === -1) {
+				if (state === 1) {
 					state = 2;
 				}
-				return state * -1;
+				else if (state === -1) {
+					state = 1;
+				}
+				return state;
 			
 			case 'numNotes':
 				return row.numNotes(false, true) || 0;
@@ -1492,6 +1509,10 @@ Zotero.ItemTreeView.prototype.sort = function (itemID) {
 					if(fieldA !== '' && fieldB === '') return -1;
 				}
 				
+				if (sortField == 'hasAttachment') {
+					return fieldB - fieldA;
+				}
+				
 				return collation.compareString(1, fieldA, fieldB);
 		}
 	}