From 269a250b4f950bf917c3a680f1d2c5aad2c75cd5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adomas=20Ven=C4=8Dkauskas?= <adomas.ven@gmail.com>
Date: Fri, 31 Mar 2017 14:27:22 +0300
Subject: [PATCH] Fetch a style if it is not installed on document preferences
 load

---
 chrome/content/zotero/bibliography.js      |  14 +--
 chrome/content/zotero/xpcom/file.js        |   4 +-
 chrome/content/zotero/xpcom/integration.js | 101 +++++++++---------
 chrome/content/zotero/xpcom/style.js       |  32 ++++--
 test/tests/data/cell.csl                   | 116 +++++++++++++++++++++
 test/tests/styleTest.js                    |  45 ++++++++
 6 files changed, 245 insertions(+), 67 deletions(-)
 create mode 100644 test/tests/data/cell.csl
 create mode 100644 test/tests/styleTest.js

diff --git a/chrome/content/zotero/bibliography.js b/chrome/content/zotero/bibliography.js
index 0ff60b8f4..13c591a54 100644
--- a/chrome/content/zotero/bibliography.js
+++ b/chrome/content/zotero/bibliography.js
@@ -67,23 +67,25 @@ var Zotero_File_Interface_Bibliography = new function() {
 			_io.style = Zotero.Prefs.get("export.lastStyle");
 		}
 		
+		// Initialize styles and try to load the style, attempting a download
+		yield Zotero.Styles.init();
+		if (!Zotero.Styles.get(_io.style)) {
+			yield Zotero.Styles.install({url: _io.style}, data.style.styleID, true);
+		}
+		
 		// add styles to list
 		
-		yield Zotero.Styles.init();
 		var styles = Zotero.Styles.getVisible();
-		var index = 0;
-		var nStyles = styles.length;
 		var selectIndex = null;
-		for(var i=0; i<nStyles; i++) {
+		for (let i=0; i < styles.length; i++) {
 			var itemNode = document.createElement("listitem");
 			itemNode.setAttribute("value", styles[i].styleID);
 			itemNode.setAttribute("label", styles[i].title);
 			listbox.appendChild(itemNode);
 			
 			if(styles[i].styleID == _io.style) {
-				selectIndex = index;
+				selectIndex = i;
 			}
-			index++;
 		}
 		
 		let requestedLocale;
diff --git a/chrome/content/zotero/xpcom/file.js b/chrome/content/zotero/xpcom/file.js
index b9289e700..b9a39e159 100644
--- a/chrome/content/zotero/xpcom/file.js
+++ b/chrome/content/zotero/xpcom/file.js
@@ -335,8 +335,8 @@ Zotero.File = new function(){
 	/*
 	 * Return a promise for the contents of a URL as a string
 	 */
-	this.getContentsFromURLAsync = function (url) {
-		return Zotero.HTTP.request("GET", url, { responseType: "text" })
+	this.getContentsFromURLAsync = function (url, options={}) {
+		return Zotero.HTTP.request("GET", url, Object.assign(options, { responseType: "text" }))
 		.then(function (xmlhttp) {
 			return xmlhttp.response;
 		});
diff --git a/chrome/content/zotero/xpcom/integration.js b/chrome/content/zotero/xpcom/integration.js
index 2f746acb5..bae225e10 100644
--- a/chrome/content/zotero/xpcom/integration.js
+++ b/chrome/content/zotero/xpcom/integration.js
@@ -928,7 +928,7 @@ Zotero.Integration.Document.prototype._createNewSession = function _createNewSes
  *    dontRunSetDocPrefs is true and no session was found, or rejected with
  *    Zotero.Exception.UserCancelled if the document preferences window was cancelled.
  */
-Zotero.Integration.Document.prototype._getSession = function _getSession(require, dontRunSetDocPrefs) {
+Zotero.Integration.Document.prototype._getSession = Zotero.Promise.coroutine(function *(require, dontRunSetDocPrefs) {
 	var dataString = this._doc.getDocumentData(),
 		data,
 		me = this;
@@ -967,7 +967,7 @@ Zotero.Integration.Document.prototype._getSession = function _getSession(require
 		
 		// Set doc prefs if no data string yet
 		this._session = this._createNewSession(data);
-		this._session.setData(data);
+		yield this._session.setData(data);
 		if(dontRunSetDocPrefs) return Zotero.Promise.resolve(false);
 		
 		return this._session.setDocPrefs(this._doc, this._app.primaryFieldType,
@@ -1012,7 +1012,7 @@ Zotero.Integration.Document.prototype._getSession = function _getSession(require
 		} else {
 			this._session = this._createNewSession(data);
 			try {
-				this._session.setData(data);
+				yield this._session.setData(data);
 			} catch(e) {
 				// make sure style is defined
 				if(e instanceof Zotero.Exception.Alert && e.name === "integration.error.invalidStyle") {
@@ -1032,7 +1032,7 @@ Zotero.Integration.Document.prototype._getSession = function _getSession(require
 		}
 		return Zotero.Promise.resolve(this._session);
 	}
-};
+});
 
 /**
  * Adds a citation to the current document.
@@ -2066,30 +2066,35 @@ Zotero.Integration.Session.prototype.resetRequest = function(doc) {
  *     regardless of whether it has changed. This is desirable if the
  *     automaticJournalAbbreviations or locale has changed.
  */
-Zotero.Integration.Session.prototype.setData = function(data, resetStyle) {
+Zotero.Integration.Session.prototype.setData = Zotero.Promise.coroutine(function *(data, resetStyle) {
 	var oldStyle = (this.data && this.data.style ? this.data.style : false);
 	this.data = data;
-	if(data.style.styleID && (!oldStyle || oldStyle.styleID != data.style.styleID || resetStyle)) {
+	if (data.style.styleID && (!oldStyle || oldStyle.styleID != data.style.styleID || resetStyle)) {
 		this.styleID = data.style.styleID;
 		try {
+			yield Zotero.Styles.init();
 			var getStyle = Zotero.Styles.get(data.style.styleID);
+			if (!getStyle) {
+				yield Zotero.Styles.install({url: data.style.styleID}, data.style.styleID, true);
+				getStyle = Zotero.Styles.get(data.style.styleID);
+			}
 			data.style.hasBibliography = getStyle.hasBibliography;
 			this.style = getStyle.getCiteProc(data.style.locale, data.prefs.automaticJournalAbbreviations);
 			this.style.setOutputFormat("rtf");
 			this.styleClass = getStyle.class;
 			this.dateModified = new Object();
-		} catch(e) {
+		} catch (e) {
 			Zotero.logError(e);
 			data.style.styleID = undefined;
 			throw new Zotero.Exception.Alert("integration.error.invalidStyle");
 		}
 		
 		return true;
-	} else if(oldStyle) {
+	} else if (oldStyle) {
 		data.style = oldStyle;
 	}
 	return false;
-}
+});
 
 /**
  * Displays a dialog to set document preferences
@@ -2097,12 +2102,12 @@ Zotero.Integration.Session.prototype.setData = function(data, resetStyle) {
  *    if there wasn't, or rejected with Zotero.Exception.UserCancelled if the dialog was
  *    cancelled.
  */
-Zotero.Integration.Session.prototype.setDocPrefs = function(doc, primaryFieldType, secondaryFieldType) {
+Zotero.Integration.Session.prototype.setDocPrefs = Zotero.Promise.coroutine(function* (doc, primaryFieldType, secondaryFieldType) {
 	var io = new function() {
 		this.wrappedJSObject = this;
 	};
 	
-	if(this.data) {
+	if (this.data) {
 		io.style = this.data.style.styleID;
 		io.locale = this.data.style.locale;
 		io.useEndnotes = this.data.prefs.noteType == 0 ? 0 : this.data.prefs.noteType-1;
@@ -2114,45 +2119,43 @@ Zotero.Integration.Session.prototype.setDocPrefs = function(doc, primaryFieldTyp
 		io.requireStoreReferences = !Zotero.Utilities.isEmpty(this.embeddedItems);
 	}
 	
-	var me = this;
-	return Zotero.Integration.displayDialog(doc,
-	'chrome://zotero/content/integration/integrationDocPrefs.xul', '', io)
-	.then(function() {
-		if (!io.style || !io.fieldType) {
-			throw new Zotero.Exception.UserCancelled("document preferences window");
-		}
-		
-		// set data
-		var oldData = me.data;
-		var data = new Zotero.Integration.DocumentData();
-		data.sessionID = oldData.sessionID;
-		data.style.styleID = io.style;
-		data.style.locale = io.locale;
-		data.prefs.fieldType = io.fieldType;
-		data.prefs.storeReferences = io.storeReferences;
-		data.prefs.automaticJournalAbbreviations = io.automaticJournalAbbreviations;
-		
-		var forceStyleReset = oldData
-			&& (
-				oldData.prefs.automaticJournalAbbreviations != data.prefs.automaticJournalAbbreviations
-				|| oldData.style.locale != io.locale
-			);
-		me.setData(data, forceStyleReset);
+	yield Zotero.Integration.displayDialog(doc,
+		'chrome://zotero/content/integration/integrationDocPrefs.xul', '', io);
+	
+	if (!io.style || !io.fieldType) {
+		throw new Zotero.Exception.UserCancelled("document preferences window");
+	}
+	
+	// set data
+	var oldData = this.data;
+	var data = new Zotero.Integration.DocumentData();
+	data.sessionID = oldData.sessionID;
+	data.style.styleID = io.style;
+	data.style.locale = io.locale;
+	data.prefs.fieldType = io.fieldType;
+	data.prefs.storeReferences = io.storeReferences;
+	data.prefs.automaticJournalAbbreviations = io.automaticJournalAbbreviations;
+	
+	var forceStyleReset = oldData
+		&& (
+			oldData.prefs.automaticJournalAbbreviations != data.prefs.automaticJournalAbbreviations
+			|| oldData.style.locale != io.locale
+		);
+	yield this.setData(data, forceStyleReset);
 
-		// need to do this after setting the data so that we know if it's a note style
-		me.data.prefs.noteType = me.style && me.styleClass == "note" ? io.useEndnotes+1 : 0;
-		
-		if(!oldData || oldData.style.styleID != data.style.styleID
-				|| oldData.prefs.noteType != data.prefs.noteType
-				|| oldData.prefs.fieldType != data.prefs.fieldType
-				|| oldData.prefs.automaticJournalAbbreviations != data.prefs.automaticJournalAbbreviations) {
-			// This will cause us to regenerate all citations
-			me.oldCitationIDs = {};
-		}
-		
-		return oldData || null;
-	});
-}
+	// need to do this after setting the data so that we know if it's a note style
+	this.data.prefs.noteType = this.style && this.styleClass == "note" ? io.useEndnotes+1 : 0;
+	
+	if (!oldData || oldData.style.styleID != data.style.styleID
+			|| oldData.prefs.noteType != data.prefs.noteType
+			|| oldData.prefs.fieldType != data.prefs.fieldType
+			|| oldData.prefs.automaticJournalAbbreviations != data.prefs.automaticJournalAbbreviations) {
+		// This will cause us to regenerate all citations
+		this.oldCitationIDs = {};
+	}
+	
+	return oldData || null;
+})
 
 /**
  * Reselects an item to replace a deleted item
diff --git a/chrome/content/zotero/xpcom/style.js b/chrome/content/zotero/xpcom/style.js
index 927385891..f73a3c54a 100644
--- a/chrome/content/zotero/xpcom/style.js
+++ b/chrome/content/zotero/xpcom/style.js
@@ -190,7 +190,7 @@ Zotero.Styles = new function() {
 		}
 		
 		return _styles[id] || false;
-	}
+	};
 	
 	/**
 	 * Gets all visible styles
@@ -238,24 +238,36 @@ Zotero.Styles = new function() {
 	/**
 	 * Installs a style file, getting the contents of an nsIFile and showing appropriate
 	 * error messages
-	 * @param {String|nsIFile} style An nsIFile representing a style on disk, or a string
-	 *     containing the style data
+	 * @param {Object} style An object with one of the following properties
+	 * 		- file: An nsIFile representing a style on disk
+	 * 		- url: A url of the location of the style (local or remote)
+	 * 		- string: A string containing the style data
 	 * @param {String} origin The origin of the style, either a filename or URL, to be
 	 *     displayed in dialogs referencing the style
 	 * @param {Boolean} [silent=false] Skip prompts
 	 */
 	this.install = Zotero.Promise.coroutine(function* (style, origin, silent=false) {
 		var styleTitle;
+		var warnDeprecated;
+		if (style instanceof Components.interfaces.nsIFile) {
+			warnDeprecated = true;
+			style = {file: style};
+		} else if (typeof style == 'string') {
+			warnDeprecated = true;
+			style = {string: style};
+		}
+		if (warnDeprecated) {
+			Zotero.debug("Zotero.Styles.install() now takes a style object as first argument -- update your code", 2);
+		}
 		
 		try {
-			if (style instanceof Components.interfaces.nsIFile) {
-				// handle nsIFiles
-				var url = Services.io.newFileURI(style);
-				var xmlhttp = yield Zotero.HTTP.request("GET", url.spec);
-				styleTitle = yield _install(xmlhttp.responseText, style.leafName, false, silent);
-			} else {
-				styleTitle = yield _install(style, origin, false, silent);
+			if (style.file) {
+				style.string = yield Zotero.File.getContentsAsync(style.file);
 			}
+			else if (style.url) {
+				style.string = yield Zotero.File.getContentsFromURLAsync(style.url);
+			}
+			styleTitle = yield _install(style.string, origin, false, silent);
 		}
 		catch (error) {
 			// Unless user cancelled, show an alert with the error
diff --git a/test/tests/data/cell.csl b/test/tests/data/cell.csl
new file mode 100644
index 000000000..1262840da
--- /dev/null
+++ b/test/tests/data/cell.csl
@@ -0,0 +1,116 @@
+<?xml version="1.0" encoding="utf-8"?>
+<style xmlns="http://purl.org/net/xbiblio/csl" version="1.0" class="in-text" default-locale="en-US" demote-non-dropping-particle="sort-only" page-range-format="expanded">
+  <info>
+    <title>Cell</title>
+    <id>http://www.zotero.org/styles/cell</id>
+    <link href="http://www.zotero.org/styles/cell" rel="self"/>
+    <link href="http://www.cell.com/authors" rel="documentation"/>
+    <author>
+      <name>Adam Mark</name>
+      <email>a.mark@uoguelph.ca</email>
+    </author>
+    <contributor>
+      <name>Julian Onions</name>
+      <email>julian.onions@gmail.com</email>
+    </contributor>
+    <contributor>
+      <name>Aurimas Vinckevicius</name>
+      <email>aurimas.dev@gmail.com</email>
+    </contributor>
+    <category citation-format="author-date"/>
+    <category field="biology"/>
+    <issn>0092-8674</issn>
+    <eissn>1097-4172</eissn>
+    <summary>The Cell journal style. Original by Julian Onions.</summary>
+    <updated>2014-09-06T22:02:33+00:00</updated>
+    <rights license="http://creativecommons.org/licenses/by-sa/3.0/">This work is licensed under a Creative Commons Attribution-ShareAlike 3.0 License</rights>
+  </info>
+  <macro name="author-short">
+    <names variable="author">
+      <name form="short" and="text"/>
+    </names>
+  </macro>
+  <macro name="author-count">
+    <names variable="author">
+      <name form="count"/>
+    </names>
+  </macro>
+  <macro name="author">
+    <names variable="author">
+      <name name-as-sort-order="all" initialize-with="." and="text" delimiter-precedes-last="always"/>
+    </names>
+  </macro>
+  <macro name="issued">
+    <date variable="issued">
+      <date-part name="year"/>
+    </date>
+  </macro>
+  <macro name="publisher">
+    <group prefix="(" delimiter=": " suffix=")">
+      <text variable="publisher-place"/>
+      <text variable="publisher"/>
+    </group>
+  </macro>
+  <macro name="editor">
+    <names variable="editor">
+      <name initialize-with="." and="text" delimiter-precedes-last="always"/>
+      <label form="short" prefix=", "/>
+    </names>
+  </macro>
+  <citation et-al-min="3" et-al-use-first="1" disambiguate-add-year-suffix="true" collapse="year">
+    <sort>
+      <key macro="author-short" names-min="1" names-use-first="1"/>
+      <key macro="author-count" names-min="3" names-use-first="3"/>
+      <key macro="author" names-min="3" names-use-first="1"/>
+      <key macro="issued"/>
+      <key variable="title"/>
+    </sort>
+    <layout prefix="(" suffix=")" delimiter="; ">
+      <group delimiter=", ">
+        <text macro="author-short"/>
+        <text macro="issued"/>
+      </group>
+    </layout>
+  </citation>
+  <bibliography et-al-min="11" et-al-use-first="10">
+    <sort>
+      <key macro="author-short" names-min="1" names-use-first="1"/>
+      <key macro="author-count" names-min="3" names-use-first="3"/>
+      <key macro="author" names-min="3" names-use-first="1"/>
+      <key macro="issued"/>
+    </sort>
+    <layout suffix=".">
+      <group delimiter=" ">
+        <text macro="author"/>
+        <text macro="issued" prefix="(" suffix=")."/>
+        <choose>
+          <if type="article article-magazine article-newspaper article-journal review" match="any">
+            <text variable="title" suffix="."/>
+            <text variable="container-title" form="short" text-case="title"/>
+            <group delimiter=", ">
+              <text variable="volume" font-style="italic"/>
+              <text variable="page"/>
+            </group>
+          </if>
+          <else-if type="chapter paper-conference" match="any">
+            <text variable="title" suffix="."/>
+            <text variable="container-title" prefix="In " suffix="," text-case="title"/>
+            <text macro="editor"/>
+            <text macro="publisher" suffix=","/>
+            <label variable="page" form="short"/>
+            <text variable="page"/>
+          </else-if>
+          <else-if type="thesis">
+            <text variable="title" suffix="."/>
+            <text variable="genre" suffix="."/>
+            <text variable="publisher"/>
+          </else-if>
+          <else>
+            <text variable="title"/>
+            <text macro="publisher"/>
+          </else>
+        </choose>
+      </group>
+    </layout>
+  </bibliography>
+</style>
diff --git a/test/tests/styleTest.js b/test/tests/styleTest.js
new file mode 100644
index 000000000..1e627a401
--- /dev/null
+++ b/test/tests/styleTest.js
@@ -0,0 +1,45 @@
+"use strict";
+
+describe("Zotero.Styles", function() {
+	var styleID = "http://www.zotero.org/styles/cell";
+	var stylePath = OS.Path.join(getTestDataDirectory().path, 'cell.csl');
+	var styleFile = Zotero.File.pathToFile(stylePath);
+	var style;
+	
+	before(function* () {
+		yield Zotero.Styles.init();
+		style = yield Zotero.File.getContentsAsync(stylePath);
+	});
+	
+	describe("Zotero.Styles.install", function() {
+		afterEach(function* (){
+			assert.isOk(Zotero.Styles.get(styleID));
+			yield Zotero.Styles.get(styleID).remove();
+		});
+		
+		it("should install the style from string", function* () {
+			yield Zotero.Styles.install(style, styleID, true);
+		});
+		
+		it("should install the style from nsIFile", function* () {
+			yield Zotero.Styles.install(styleFile, styleID, true);
+		});
+
+		it("should install the style from url", function* () {
+			var getContentsFromURLAsync = Zotero.File.getContentsFromURLAsync;
+			sinon.stub(Zotero.File, 'getContentsFromURLAsync', function(style) {
+				if (style.url == styleID) {
+					return Zotero.Promise.resolve(style);
+				} else {
+					return getContentsFromURLAsync.apply(Zotero.File, arguments);
+				}
+			});
+			yield Zotero.Styles.install({url: styleID}, styleID, true);
+			Zotero.File.getContentsFromURLAsync.restore();
+		});
+		
+		it("should install the style from file path", function* () {
+			yield Zotero.Styles.install({file: stylePath}, styleID, true);
+		})
+	});
+});