From 9f91d240b060039c7e89594e232793fd39c19c20 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Thu, 15 May 2014 03:19:49 -0400
Subject: [PATCH] Library switcher in advanced search window

When opening the advanced search window, the current library is
selected, and a different library can be selected to change the search
scope. If a library is read-only, the saved search button is disabled.
For saved searches, the appropriate library is selected and the
drop-down is disabled.

Also:

- Close the advanced search window after a search is saved
- The default name for saved searches ("Untitled 2", etc.) was based on
  collections rather than searches
- Once an initial search has been performed, the drop-downs and
  checkboxes now update the results
- More consistent spacing in advanced search window
- (dev) Zotero.DB.getNextName() now takes a libraryID as its first
  parameter instead of always using My Library; the old parameters are
  deprecated but still work
---
 chrome/content/zotero/advancedSearch.js       | 48 ++++++++---
 chrome/content/zotero/advancedSearch.xul      | 56 ++++++-------
 .../content/zotero/bindings/zoterosearch.xml  | 84 +++++++++++++++++--
 chrome/content/zotero/xpcom/db.js             | 23 ++++-
 chrome/content/zotero/xpcom/search.js         |  3 +-
 chrome/content/zotero/zoteroPane.js           |  1 +
 chrome/locale/en-US/zotero/searchbox.dtd      |  2 +
 .../skin/default/zotero/bindings/search.css   | 40 +++++----
 chrome/skin/default/zotero/overlay.css        |  5 --
 chrome/skin/default/zotero/zotero.css         | 14 ++++
 10 files changed, 198 insertions(+), 78 deletions(-)

diff --git a/chrome/content/zotero/advancedSearch.js b/chrome/content/zotero/advancedSearch.js
index eec434fc8c..4bb5cefe70 100644
--- a/chrome/content/zotero/advancedSearch.js
+++ b/chrome/content/zotero/advancedSearch.js
@@ -43,6 +43,7 @@ var ZoteroAdvancedSearch = new function() {
 		var sbc = document.getElementById('zotero-search-box-container');
 		Zotero.setFontSize(sbc);
 		
+		_searchBox.onLibraryChange = this.onLibraryChange;
 		var io = window.arguments[0];
 		_searchBox.search = io.dataIn.search;
 	}
@@ -50,24 +51,33 @@ var ZoteroAdvancedSearch = new function() {
 	
 	function search() {
 		_searchBox.updateSearch();
+		_searchBox.active = true;
 		
 		// A minimal implementation of Zotero.CollectionTreeView
 		var itemGroup = {
 			isSearchMode: function() { return true; },
 			getItems: function () {
-				//var search = _searchBox.search.clone();
+				var search = _searchBox.search.clone();
 				
-				var s2 = new Zotero.Search();
-				s2.setScope(_searchBox.search);
-				
-				// FIXME: Hack to exclude group libraries for now
-				var groups = Zotero.Groups.getAll();
-				for each(var group in groups) {
-					s2.addCondition('libraryID', 'isNot', group.libraryID);
+				// Hack to create a condition for the search's library --
+				// this logic should really go in the search itself instead of here
+				// and in collectionTreeView.js
+				var conditions = search.getSearchConditions();
+				if (!conditions.some(function (condition) condition.condition == 'libraryID')) {
+					let libraryID = _searchBox.search.libraryID;
+					// TEMP: libraryIDInt
+					if (libraryID) {
+						search.addCondition('libraryID', 'is', libraryID);
+					}
+					else {
+						let groups = Zotero.Groups.getAll();
+						for (let i=0; i<groups.length; i++) {
+							search.addCondition('libraryID', 'isNot', groups[i].libraryID);
+						}
+					}
 				}
 				
-				var ids = s2.search();
-				return Zotero.Items.get(ids);
+				return Zotero.Items.get(search.search());
 			},
 			isLibrary: function () { return false; },
 			isCollection: function () { return false; },
@@ -92,8 +102,11 @@ var ZoteroAdvancedSearch = new function() {
 		document.getElementById('zotero-items-tree').view = null;
 		
 		var s = new Zotero.Search();
+		// Don't clear the selected library
+		s.libraryID = _searchBox.search.libraryID;
 		s.addCondition('title', 'contains', '');
 		_searchBox.search = s;
+		_searchBox.active = false;
 	}
 	
 	
@@ -103,8 +116,12 @@ var ZoteroAdvancedSearch = new function() {
 		var promptService = Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
 								.getService(Components.interfaces.nsIPromptService);
 		
-		var untitled = Zotero.DB.getNextName('collections', 'collectionName',
-			Zotero.getString('pane.collections.untitled'));
+		var untitled = Zotero.DB.getNextName(
+			_searchBox.search.libraryID,
+			'savedSearches',
+			'savedSearchName',
+			Zotero.getString('pane.collections.untitled')
+		);
 		
 		var name = { value: untitled };
 		var result = promptService.prompt(window,
@@ -124,6 +141,13 @@ var ZoteroAdvancedSearch = new function() {
 		var s = _searchBox.search.clone();
 		s.name = name.value;
 		s.save();
+		
+		window.close();
+	}
+	
+	
+	this.onLibraryChange = function (libraryID) {
+		document.getElementById('zotero-search-save').disabled = !Zotero.Libraries.isEditable(libraryID);
 	}
 	
 	
diff --git a/chrome/content/zotero/advancedSearch.xul b/chrome/content/zotero/advancedSearch.xul
index 57b16540ab..953fd40be2 100644
--- a/chrome/content/zotero/advancedSearch.xul
+++ b/chrome/content/zotero/advancedSearch.xul
@@ -25,35 +25,33 @@
 	<script src="advancedSearch.js"/>
 	
 	<vbox id="zotero-search-box-container" flex="1">
-	
-	<hbox>
-		<zoterosearch id="zotero-search-box" oncommand="ZoteroAdvancedSearch.search()" flex="1"/>
-	</hbox>
-	
-	<hbox id="zotero-search-buttons">
-		<button label="&zotero.search.search;" default="true" oncommand="ZoteroAdvancedSearch.search()"/>
-		<button label="&zotero.search.clear;" oncommand="ZoteroAdvancedSearch.clear()"/>
-		<button label="&zotero.search.saveSearch;" oncommand="ZoteroAdvancedSearch.save()"/>
-	</hbox>
-	
-	<tree id="zotero-items-tree" flex="1" hidecolumnpicker="true" seltype="multiple"
-			ondblclick="ZoteroAdvancedSearch.onDblClick(event, this)"
-			ondragstart="if (event.target.localName == 'treechildren') { ZoteroAdvancedSearch.itemsView.onDragStart(event); }">
-		<treecols>
-			<treecol
-				id="zotero-items-column-title" primary="true"
-				label="&zotero.items.title_column;"
-				flex="4" persist="width ordinal hidden sortActive sortDirection"/>
-			<splitter class="tree-splitter"/>
-			<treecol
-				id="zotero-items-column-firstCreator"
-				label="&zotero.items.creator_column;"
-				flex="1" persist="width ordinal hidden sortActive sortDirection"/>
-			<splitter class="tree-splitter"/>
-		</treecols>
-		<treechildren alternatingbackground="true"/>
-	</tree>
-	
+		<vbox id="zotero-search-box-controls">
+			<zoterosearch id="zotero-search-box" oncommand="if (this.active) { ZoteroAdvancedSearch.search(); }" flex="1"/>
+			
+			<hbox id="zotero-search-buttons">
+				<button label="&zotero.search.search;" default="true" oncommand="ZoteroAdvancedSearch.search()"/>
+				<button label="&zotero.search.clear;" oncommand="ZoteroAdvancedSearch.clear()"/>
+				<button id="zotero-search-save" label="&zotero.search.saveSearch;" oncommand="ZoteroAdvancedSearch.save()"/>
+			</hbox>
+		</vbox>
+		
+		<tree id="zotero-items-tree" flex="1" hidecolumnpicker="true" seltype="multiple"
+				ondblclick="ZoteroAdvancedSearch.onDblClick(event, this)"
+				ondragstart="if (event.target.localName == 'treechildren') { ZoteroAdvancedSearch.itemsView.onDragStart(event); }">
+			<treecols>
+				<treecol
+					id="zotero-items-column-title" primary="true"
+					label="&zotero.items.title_column;"
+					flex="4" persist="width ordinal hidden sortActive sortDirection"/>
+				<splitter class="tree-splitter"/>
+				<treecol
+					id="zotero-items-column-firstCreator"
+					label="&zotero.items.creator_column;"
+					flex="1" persist="width ordinal hidden sortActive sortDirection"/>
+				<splitter class="tree-splitter"/>
+			</treecols>
+			<treechildren alternatingbackground="true"/>
+		</tree>
 	</vbox>
 	
 	<keyset>
diff --git a/chrome/content/zotero/bindings/zoterosearch.xml b/chrome/content/zotero/bindings/zoterosearch.xml
index a3387e1a83..ad51467404 100644
--- a/chrome/content/zotero/bindings/zoterosearch.xml
+++ b/chrome/content/zotero/bindings/zoterosearch.xml
@@ -42,6 +42,8 @@
 					<![CDATA[
 						this.searchRef = val;
 						
+						this.buildLibraryMenu();
+						
 						var conditionsBox = this.id('conditions');
 						while(conditionsBox.hasChildNodes())
 							conditionsBox.removeChild(conditionsBox.firstChild);
@@ -74,6 +76,49 @@
 					]]>
 				</setter>
 			</property>
+			
+			<method name="buildLibraryMenu">
+				<body><![CDATA[
+					var menulist = this.id('libraryMenu');
+					var menupopup = menulist.firstChild;
+					
+					while (menupopup.hasChildNodes()) {
+						menupopup.removeChild(menupopup.firstChild);
+					}
+					
+					var libraryID = this.searchRef.libraryID;
+					var libraryIndex = 0;
+					
+					// Add My Library
+					var menuitem = document.createElement('menuitem');
+					menuitem.setAttribute('label', Zotero.getString('pane.collections.library'));
+					menuitem.setAttribute('libraryID', 0);
+					menupopup.appendChild(menuitem);
+					
+					// Add groups
+					var groups = Zotero.Groups.getAll();
+					for (let i=0; i<groups.length; i++) {
+						let group = groups[i];
+						let menuitem = document.createElement('menuitem');
+						menuitem.setAttribute('label', group.name);
+						menuitem.setAttribute('libraryID', group.libraryID);
+						if (group.libraryID == libraryID) {
+							libraryIndex = i + 1;
+						}
+						menupopup.appendChild(menuitem);
+					}
+					
+					menulist.appendChild(menupopup);
+					menulist.selectedIndex = libraryIndex;
+					
+					if (this.searchRef.id) {
+						this.id('libraryMenu').disabled = true;
+					}
+					
+					this.updateLibrary();
+				]]></body>
+			</method>
+			
 			<method name="addCondition">
 				<parameter name="ref"/>
 				<body>
@@ -100,6 +145,7 @@
 					]]>
 				</body>
 			</method>
+			
 			<method name="removeCondition">
 				<parameter name="id"/>
 				<body>
@@ -121,6 +167,21 @@
 					]]>
 				</body>
 			</method>
+			
+			<method name="updateLibrary">
+				<body><![CDATA[
+					var menu = this.id('libraryMenu');
+					var libraryID = parseInt(menu.selectedItem.getAttribute('libraryID'));
+					
+				 	if (this.onLibraryChange) {
+				 		this.onLibraryChange(libraryID);
+					}
+					
+					// TODO: libraryIDInt
+					this.searchRef.libraryID = libraryID ? libraryID : null;
+				]]></body>
+			</method>
+			
 			<method name="updateJoinMode">
 				<body>
 					<![CDATA[
@@ -132,6 +193,7 @@
 					]]>
 				</body>
 			</method>
+			
 			<method name="updateCheckbox">
 				<parameter name="condition"/>
 				<body>
@@ -151,12 +213,12 @@
 					]]>
 				</body>
 			</method>
+			
 			<!-- Calls updateSearch() on all search conditions -->
 			<method name="updateSearch">
 				<body>
 					<![CDATA[
 						var conditionsBox = this.id('conditions');
-						
 						if (conditionsBox.hasChildNodes()) {
 							for(var i = 0, len=conditionsBox.childNodes.length; i < len; i++) {
 								conditionsBox.childNodes[i].updateSearch();
@@ -165,6 +227,7 @@
 					]]>
 				</body>
 			</method>
+			
 			<method name="save">
 				<body>
 					<![CDATA[
@@ -181,6 +244,8 @@
 					switch (event.keyCode) {
 						case event.DOM_VK_RETURN:
 						case event.DOM_VK_ENTER:
+							this.active = true;
+							
 							if (event.shiftKey) {
 								this.addCondition();
 							}
@@ -193,7 +258,6 @@
 				</body>
 			</method>
 			
-			
 			<method name="id">
 				<parameter name="id"/>
 				<body>
@@ -207,10 +271,16 @@
 		<content>
 			<vbox xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
 					id="search-box" flex="1" onkeypress="document.getBindingParent(this).handleKeyPress(event)">
+				<hbox align="center">
+					<label value="&zotero.search.searchInLibrary;" control="libraryMenu"/>
+					<menulist id="libraryMenu" oncommand="document.getBindingParent(this).updateLibrary();">
+						<menupopup/>
+					</menulist>
+				</hbox>
 				<groupbox xbl:inherits="flex">
 					<caption align="center">
 						<label value="&zotero.search.joinMode.prefix;"/>
-						<menulist id="joinModeMenu" oncommand="document.getBindingParent(this).updateJoinMode(); event.stopPropagation()">
+						<menulist id="joinModeMenu" oncommand="document.getBindingParent(this).updateJoinMode();">
 							<menupopup>
 								<menuitem label="&zotero.search.joinMode.any;" value="any"/>
 								<menuitem label="&zotero.search.joinMode.all;" value="all" selected="true"/>
@@ -221,10 +291,12 @@
 					<vbox id="conditions"/>
 				</groupbox>
 				<hbox>
-					<checkbox id="recursiveCheckbox" label="&zotero.search.recursive.label;" oncommand="document.getBindingParent(this).updateCheckbox('recursive'); event.stopPropagation()"/>
-					<checkbox id="noChildrenCheckbox" label="&zotero.search.noChildren;" oncommand="document.getBindingParent(this).updateCheckbox('noChildren'); event.stopPropagation()"/>
+					<checkbox id="recursiveCheckbox" label="&zotero.search.recursive.label;" oncommand="document.getBindingParent(this).updateCheckbox('recursive');"/>
+					<checkbox id="noChildrenCheckbox" label="&zotero.search.noChildren;" oncommand="document.getBindingParent(this).updateCheckbox('noChildren');"/>
+				</hbox>
+				<hbox>
+					<checkbox id="includeParentsAndChildrenCheckbox" label="&zotero.search.includeParentsAndChildren;" oncommand="document.getBindingParent(this).updateCheckbox('includeParentsAndChildren');"/>
 				</hbox>
-				<checkbox id="includeParentsAndChildrenCheckbox" label="&zotero.search.includeParentsAndChildren;" oncommand="document.getBindingParent(this).updateCheckbox('includeParentsAndChildren'); event.stopPropagation()"/>
 			</vbox>
 		</content>
 	</binding>
diff --git a/chrome/content/zotero/xpcom/db.js b/chrome/content/zotero/xpcom/db.js
index 4cc54e2dba..245bc98b54 100644
--- a/chrome/content/zotero/xpcom/db.js
+++ b/chrome/content/zotero/xpcom/db.js
@@ -703,13 +703,28 @@ Zotero.DBConnection.prototype.getNextID = function (table, column) {
 *
 * If _name_ alone is available, returns that
 **/
-Zotero.DBConnection.prototype.getNextName = function (table, field, name)
+Zotero.DBConnection.prototype.getNextName = function (libraryID, table, field, name)
 {
+	if (typeof name == 'undefined') {
+		Zotero.debug("WARNING: The parameters of Zotero.DB.getNextName() have changed -- update your code", 2);
+		[libraryID, table, field, name] = [null, libraryID, table, field];
+	}
+	
 	var sql = "SELECT TRIM(SUBSTR(" + field + ", " + (name.length + 1) + ")) "
 				+ "FROM " + table + " "
-				+ "WHERE " + field + " REGEXP '^" + name + "( [0-9]+)?$' "
-				+ "ORDER BY " + field;
-	var suffixes = this.columnQuery(sql);
+				+ "WHERE " + field + " REGEXP '^" + name + "( [0-9]+)?$' ";
+	if (!libraryID) {
+		// DEBUG: Shouldn't this be replaced automatically with "=?"?
+		sql += " AND libraryID IS NULL";
+		var params = undefined
+	}
+	else {
+		sql += " AND libraryID=?";
+		var params = [libraryID];
+	}
+	sql += " ORDER BY " + field;
+	// TEMP: libraryIDInt
+	var suffixes = this.columnQuery(sql, params);
 	// If none found or first one has a suffix, use default name
 	if (!suffixes || suffixes[0]) {
 		return name;
diff --git a/chrome/content/zotero/xpcom/search.js b/chrome/content/zotero/xpcom/search.js
index 0039441ea3..1853d70f0b 100644
--- a/chrome/content/zotero/xpcom/search.js
+++ b/chrome/content/zotero/xpcom/search.js
@@ -107,7 +107,7 @@ Zotero.Search.prototype._set = function (field, val) {
 			}
 			
 			if (this._loaded) {
-				throw ("Cannot set " + field + " after object is already loaded in Zotero.Search._set()");
+				throw new Error("Cannot set " + field + " after object is already loaded");
 			}
 			//this._checkValue(field, val);
 			this['_' + field] = val;
@@ -379,6 +379,7 @@ Zotero.Search.prototype.save = function(fixGaps) {
 
 Zotero.Search.prototype.clone = function() {
 	var s = new Zotero.Search();
+	s.libraryID = this.libraryID;
 	
 	var conditions = this.getSearchConditions();
 	
diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js
index 709dbeeacd..a00eb44c26 100644
--- a/chrome/content/zotero/zoteroPane.js
+++ b/chrome/content/zotero/zoteroPane.js
@@ -977,6 +977,7 @@ var ZoteroPane = new function()
 		}
 		
 		var s = new Zotero.Search();
+		s.libraryID = this.getSelectedLibraryID();
 		s.addCondition('title', 'contains', '');
 		var io = {dataIn: {search: s}, dataOut: null};
 		window.openDialog('chrome://zotero/content/advancedSearch.xul', '', 'chrome,dialog=no,centerscreen', io);
diff --git a/chrome/locale/en-US/zotero/searchbox.dtd b/chrome/locale/en-US/zotero/searchbox.dtd
index 9ad6166daf..f01c0d13bc 100644
--- a/chrome/locale/en-US/zotero/searchbox.dtd
+++ b/chrome/locale/en-US/zotero/searchbox.dtd
@@ -1,5 +1,7 @@
 <!ENTITY zotero.search.name				"Name:">
 
+<!ENTITY zotero.search.searchInLibrary "Search in library:">
+
 <!ENTITY zotero.search.joinMode.prefix		"Match">
 <!ENTITY zotero.search.joinMode.any		"any">
 <!ENTITY zotero.search.joinMode.all		"all">
diff --git a/chrome/skin/default/zotero/bindings/search.css b/chrome/skin/default/zotero/bindings/search.css
index 100e74658d..1fa36efbb7 100644
--- a/chrome/skin/default/zotero/bindings/search.css
+++ b/chrome/skin/default/zotero/bindings/search.css
@@ -3,8 +3,27 @@
 	width: 60em;
 }
 
+#search-box > hbox {
+	margin-left: 6px;
+}
+
+groupbox {
+	margin-top: 0;
+	padding-top: 0;
+}
+
 caption {
 	font: inherit;
+	padding-left: 0 !important;
+}
+
+label:first-child, checkbox:first-child {
+	margin-left: 0 !important;
+	padding-left: 0 !important;
+}
+
+checkbox {
+	margin-right: .5em;
 }
 
 #search-condition menulist[id="operatorsmenu"]
@@ -48,24 +67,3 @@ caption {
 {
 	min-width: 3em;
 }
-
-#zotero-advanced-search-dialog
-{
-	padding: 8px 8px 14px;
-	height: 400px;
-}
-
-#zotero-advanced-search-dialog #zotero-search-buttons
-{
-	margin: 3px 0;
-}
-
-#zotero-advanced-search-dialog checkbox
-{
-	margin-right: .5em;
-}
-
-#zotero-advanced-search-dialog #zotero-items-tree
-{
-	min-height: 170px;
-}
diff --git a/chrome/skin/default/zotero/overlay.css b/chrome/skin/default/zotero/overlay.css
index eb3b67033e..633bd16288 100644
--- a/chrome/skin/default/zotero/overlay.css
+++ b/chrome/skin/default/zotero/overlay.css
@@ -214,11 +214,6 @@
 	color: inherit;
 }
 
-#zotero-advanced-search-dialog #zotero-items-tree
-{
-	min-height: 250px;
-}
-
 #zotero-items-pane
 {
 	min-width: 290px;
diff --git a/chrome/skin/default/zotero/zotero.css b/chrome/skin/default/zotero/zotero.css
index 59df981c03..aed820b173 100644
--- a/chrome/skin/default/zotero/zotero.css
+++ b/chrome/skin/default/zotero/zotero.css
@@ -340,4 +340,18 @@ label.zotero-text-link {
 	font-weight: bold;
 	color: red;
 	text-align: center;
+}
+
+#zotero-advanced-search-dialog #zotero-search-box-controls {
+	padding: 3px;
+}
+
+#zotero-advanced-search-dialog #zotero-items-tree
+{
+	min-height: 250px;
+}
+
+#zotero-advanced-search-dialog #zotero-search-buttons
+{
+	margin: 3px 0;
 }
\ No newline at end of file