From 74cf2a3c22625409f4ecfdf85f63332db798222a Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Tue, 22 Mar 2016 00:40:59 -0400 Subject: [PATCH] Fix hang on import that includes an HTML attachment Closes #734, for the moment --- chrome/content/zotero/xpcom/attachments.js | 41 ++++++++++------------ test/content/support.js | 2 ++ test/tests/attachmentsTest.js | 41 +++++++++++++++++++++- test/tests/data/book_and_snapshot.rdf | 26 ++++++++++++++ test/tests/fileInterfaceTest.js | 33 +++++++++++++++++ 5 files changed, 119 insertions(+), 24 deletions(-) create mode 100644 test/tests/data/book_and_snapshot.rdf diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index aafaa99742..617a57ac7d 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -1342,35 +1342,30 @@ Zotero.Attachments = new function(){ browser.addEventListener("pageshow", onpageshow, false); } else { - let callback = function(charset, args) { + let callback = Zotero.Promise.coroutine(function* (charset, args) { // ignore spurious about:blank loads if(browser.contentDocument.location.href == "about:blank") return; - // Since the callback can be called during an import process that uses - // Zotero.wait(), wait until we're unlocked - Zotero.unlockPromise - .then(function () { - return Zotero.spawn(function* () { + try { + if (charset) { + charset = Zotero.CharacterSets.toCanonical(charset); if (charset) { - charset = Zotero.CharacterSets.toCanonical(charset); - if (charset) { - item.attachmentCharset = charset; - yield item.saveTx({ - skipNotifier: true - }); - } + item.attachmentCharset = charset; + yield item.saveTx({ + skipNotifier: true + }); } - - yield Zotero.Fulltext.indexDocument(browser.contentDocument, item.id); - Zotero.Browser.deleteHiddenBrowser(browser); - - deferred.resolve(); - }); - }) - .catch(function (e) { + } + + yield Zotero.Fulltext.indexDocument(browser.contentDocument, item.id); + Zotero.Browser.deleteHiddenBrowser(browser); + + deferred.resolve(); + } + catch (e) { deferred.reject(e); - }); - }; + } + }); Zotero.File.addCharsetListener(browser, callback, item.id); } diff --git a/test/content/support.js b/test/content/support.js index 55075d9411..b06a6378f3 100644 --- a/test/content/support.js +++ b/test/content/support.js @@ -200,6 +200,8 @@ function waitForItemEvent(event) { * Wait for a single notifier event and return a promise for the data */ function waitForNotifierEvent(event, type) { + if (!event) throw new Error("event not provided"); + var deferred = Zotero.Promise.defer(); var notifierID = Zotero.Notifier.registerObserver({notify:function(ev, type, ids, extraData) { if(ev == event) { diff --git a/test/tests/attachmentsTest.js b/test/tests/attachmentsTest.js index 63aa726e54..f051d3fa26 100644 --- a/test/tests/attachmentsTest.js +++ b/test/tests/attachmentsTest.js @@ -93,7 +93,7 @@ describe("Zotero.Attachments", function() { }); }) - describe("#linkToFile()", function () { + describe("#linkFromFile()", function () { it("should link to a file in My Library", function* () { var item = yield createDataObject('item'); @@ -112,6 +112,45 @@ describe("Zotero.Attachments", function() { }) }) + describe("#importSnapshotFromFile()", function () { + it("should import an HTML file", function* () { + var item = yield createDataObject('item'); + var file = getTestDataDirectory(); + file.append('test.html'); + var attachment = yield Zotero.Attachments.importSnapshotFromFile({ + title: 'Snapshot', + url: 'http://example.com', + file, + parentItemID: item.id, + contentType: 'text/html', + charset: 'utf-8' + }); + + var matches = yield Zotero.Fulltext.findTextInItems([attachment.id], 'test'); + assert.lengthOf(matches, 1); + assert.propertyVal(matches[0], 'id', attachment.id); + }); + + it("should detect charset for an HTML file", function* () { + var item = yield createDataObject('item'); + var file = getTestDataDirectory(); + file.append('test.html'); + var attachment = yield Zotero.Attachments.importSnapshotFromFile({ + title: 'Snapshot', + url: 'http://example.com', + file, + parentItemID: item.id, + contentType: 'text/html' + }); + + assert.equal(attachment.attachmentCharset, 'utf-8'); + + var matches = yield Zotero.Fulltext.findTextInItems([attachment.id], 'test'); + assert.lengthOf(matches, 1); + assert.propertyVal(matches[0], 'id', attachment.id); + }); + }); + describe("#linkFromDocument", function () { it("should add a link attachment for the current webpage", function* () { var item = yield createDataObject('item'); diff --git a/test/tests/data/book_and_snapshot.rdf b/test/tests/data/book_and_snapshot.rdf new file mode 100644 index 0000000000..e3ce435d03 --- /dev/null +++ b/test/tests/data/book_and_snapshot.rdf @@ -0,0 +1,26 @@ + + + book + + Test + + + attachment + + + + http://example.com/ + + + 2016-03-22 04:55:27 + Test + 1 + text/html + + diff --git a/test/tests/fileInterfaceTest.js b/test/tests/fileInterfaceTest.js index b3435593b7..e3862723e5 100644 --- a/test/tests/fileInterfaceTest.js +++ b/test/tests/fileInterfaceTest.js @@ -38,4 +38,37 @@ describe("Zotero_File_Interface", function() { } assert.deepEqual(savedItems, trueItems, "saved items match inputs") }); + + it('should import an item and snapshot from Zotero RDF', function* () { + var tmpDir = yield getTempDirectory(); + var rdfFile = OS.Path.join(tmpDir, 'test.rdf'); + yield OS.File.copy(OS.Path.join(getTestDataDirectory().path, 'book_and_snapshot.rdf'), rdfFile); + yield OS.File.makeDir(OS.Path.join(tmpDir, 'files')); + yield OS.File.makeDir(OS.Path.join(tmpDir, 'files', 2)); + yield OS.File.copy( + OS.Path.join(getTestDataDirectory().path, 'test.html'), + OS.Path.join(tmpDir, 'files', 2, 'test.html') + ); + + var promise = waitForItemEvent('add'); + Zotero.debug(yield Zotero.File.getContentsAsync(rdfFile)); + yield win.Zotero_File_Interface.importFile(Zotero.File.pathToFile(rdfFile)) + var ids = yield promise; + assert.lengthOf(ids, 1); + + // Check book + var item = Zotero.Items.get(ids[0]); + assert.equal(item.itemTypeID, Zotero.ItemTypes.getID('book')); + + // Check attachment + var ids = item.getAttachments(); + assert.lengthOf(ids, 1); + var attachment = Zotero.Items.get(ids[0]); + assert.equal(attachment.attachmentCharset, 'utf-8'); + + // Check indexing + var matches = yield Zotero.Fulltext.findTextInItems([attachment.id], 'test'); + assert.lengthOf(matches, 1); + assert.propertyVal(matches[0], 'id', attachment.id); + }); }); \ No newline at end of file