From 8fec5ace3ae3161fe38a63de5e56504a9df2feb4 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Mon, 4 May 2015 02:00:52 -0400 Subject: [PATCH] Fix post-save textbox focusing bugs in right-hand pane Fix a couple cases of lost text field focus after an edit, including focusing of the Title field after using New Item when a field is already being edited and has a changed value. Also, in tests, select My Library and wait for items to load when using the loadZoteroPane() support function. We could add a parameter to skip that or move it to a separate function, but the code to detect it is a bit convoluted and it's a prerequisite for many tests, so it's handy to have a function for it. --- chrome/content/zotero/bindings/itembox.xml | 42 +++++++++++----------- chrome/content/zotero/itemPane.js | 17 +++++++++ chrome/content/zotero/zoteroPane.js | 2 ++ test/content/support.js | 36 ++++++++++++------- test/tests/collectionTreeViewTest.js | 16 ++------- test/tests/itemTreeViewTest.js | 16 ++------- test/tests/zoteroPaneTest.js | 37 +++++++++++++++++++ 7 files changed, 103 insertions(+), 63 deletions(-) create mode 100644 test/tests/zoteroPaneTest.js diff --git a/chrome/content/zotero/bindings/itembox.xml b/chrome/content/zotero/bindings/itembox.xml index a6809d1b8..305733b68 100644 --- a/chrome/content/zotero/bindings/itembox.xml +++ b/chrome/content/zotero/bindings/itembox.xml @@ -1682,8 +1682,6 @@ - - - + @@ -1973,8 +1972,7 @@ throw ("Invalid transform mode '" + mode + "' in zoteroitembox.textTransform()"); } this._setFieldValue(label, newVal); - this._modifyField(label.getAttribute('fieldname'), newVal, this.saveOnEdit); - + this._modifyField(label.getAttribute('fieldname'), newVal, this.saveOnEdit).done(); ]]> @@ -2204,7 +2202,7 @@ return Zotero.spawn(function* () { var textboxes = document.getAnonymousNodes(this)[0].getElementsByTagName('textbox'); if (textboxes && textboxes.length) { - yield this.blurHandler(textboxes[0].inputField); + yield this.blurHandler(textboxes[0]); } }, this); ]]> diff --git a/chrome/content/zotero/itemPane.js b/chrome/content/zotero/itemPane.js index 2a4703af2..ba10b2428 100644 --- a/chrome/content/zotero/itemPane.js +++ b/chrome/content/zotero/itemPane.js @@ -153,6 +153,23 @@ var ZoteroItemPane = new function() { }); + this.blurOpenField = Zotero.Promise.coroutine(function* () { + var tabBox = document.getElementById('zotero-view-tabbox'); + switch (tabBox.selectedIndex) { + case 0: + var box = _itemBox; + break; + + case 2: + var box = _tagsBox; + break; + } + if (box) { + yield box.blurOpenField(); + } + }); + + this.addNote = function (popup) { ZoteroPane_Local.newNote(popup, _lastItem.key); } diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index cf9ea0275..98c88df3a 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -751,6 +751,8 @@ var ZoteroPane = new function() } } + yield ZoteroItemPane.blurOpenField(); + if (row !== undefined && row !== null) { var collectionTreeRow = this.collectionsView.getRow(row); var libraryID = collectionTreeRow.ref.libraryID; diff --git a/test/content/support.js b/test/content/support.js index 522b85505..797a0e0de 100644 --- a/test/content/support.js +++ b/test/content/support.js @@ -32,20 +32,30 @@ function loadBrowserWindow() { } /** - * Loads a Zotero pane in a new window. Returns the containing window. + * Loads a Zotero pane in a new window and selects My Library. Returns the containing window. */ -function loadZoteroPane() { - return loadBrowserWindow().then(function(win) { - win.ZoteroOverlay.toggleDisplay(true); - - // Hack to wait for pane load to finish. This is the same hack - // we use in ZoteroPane.js, so either it's not good enough - // there or it should be good enough here. - return Zotero.Promise.delay(52).then(function() { - return win; - }); - }); -} +var loadZoteroPane = Zotero.Promise.coroutine(function* () { + var win = yield loadBrowserWindow(); + win.ZoteroOverlay.toggleDisplay(true); + + // Hack to wait for pane load to finish. This is the same hack + // we use in ZoteroPane.js, so either it's not good enough + // there or it should be good enough here. + yield Zotero.Promise.delay(52); + + var zp = win.ZoteroPane; + var cv = zp.collectionsView; + var resolve1, resolve2; + var promise1 = new Zotero.Promise(() => resolve1 = arguments[0]); + var promise2 = new Zotero.Promise(() => resolve2 = arguments[0]); + cv.addEventListener('load', () => resolve1()) + yield promise1; + cv.selection.select(0); + zp.addEventListener('itemsLoaded', () => resolve2()); + yield promise2; + + return win; +}); /** * Waits for a window with a specific URL to open. Returns a promise for the window. diff --git a/test/tests/collectionTreeViewTest.js b/test/tests/collectionTreeViewTest.js index 306795361..8fda530c6 100644 --- a/test/tests/collectionTreeViewTest.js +++ b/test/tests/collectionTreeViewTest.js @@ -4,22 +4,10 @@ describe("Zotero.CollectionTreeView", function() { // Load Zotero pane and select library before(function* () { win = yield loadZoteroPane(); - var zp = win.ZoteroPane; - var cv = zp.collectionsView; - var resolve1, resolve2; - var promise1 = new Zotero.Promise(() => resolve1 = arguments[0]); - var promise2 = new Zotero.Promise(() => resolve2 = arguments[0]); - cv.addEventListener('load', () => resolve1()) - yield promise1; - cv.selection.select(0); - zp.addEventListener('itemsLoaded', () => resolve2()); - yield promise2; - collectionsView = zp.collectionsView; + collectionsView = win.ZoteroPane.collectionsView; }); after(function () { - if (win) { - win.close(); - } + win.close(); }); // Select library diff --git a/test/tests/itemTreeViewTest.js b/test/tests/itemTreeViewTest.js index f9f121e1f..ca7acbab0 100644 --- a/test/tests/itemTreeViewTest.js +++ b/test/tests/itemTreeViewTest.js @@ -4,25 +4,13 @@ describe("Zotero.ItemTreeView", function() { // Load Zotero pane and select library before(function* () { win = yield loadZoteroPane(); - var zp = win.ZoteroPane; - var cv = zp.collectionsView; - var resolve1, resolve2; - var promise1 = new Zotero.Promise(() => resolve1 = arguments[0]); - var promise2 = new Zotero.Promise(() => resolve2 = arguments[0]); - cv.addEventListener('load', () => resolve1()) - yield promise1; - cv.selection.select(0); - zp.addEventListener('itemsLoaded', () => resolve2()); - yield promise2; - itemsView = zp.itemsView; + itemsView = win.ZoteroPane.itemsView; var item = new Zotero.Item('book'); existingItemID = yield item.save(); }); after(function () { - if (win) { - win.close(); - } + win.close(); }); describe("#selectItem()", function () { diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js new file mode 100644 index 000000000..092e698da --- /dev/null +++ b/test/tests/zoteroPaneTest.js @@ -0,0 +1,37 @@ +describe("ZoteroPane", function() { + var win, doc, zp; + + // Load Zotero pane and select library + before(function* () { + win = yield loadZoteroPane(); + doc = win.document; + zp = win.ZoteroPane; + }); + + after(function () { + win.close(); + }); + + describe("#newItem", function () { + it("should create an item and focus the title field", function* () { + yield zp.newItem(Zotero.ItemTypes.getID('book'), {}, null, true); + var itemBox = doc.getElementById('zotero-editpane-item-box'); + var textboxes = doc.getAnonymousNodes(itemBox)[0].getElementsByTagName('textbox'); + assert.lengthOf(textboxes, 1); + assert.equal(textboxes[0].getAttribute('fieldname'), 'title'); + textboxes[0].blur(); + yield Zotero.Promise.delay(1); + }) + + it("should save an entered value when New Item is used", function* () { + var value = "Test"; + var item = yield zp.newItem(Zotero.ItemTypes.getID('book'), {}, null, true); + var itemBox = doc.getElementById('zotero-editpane-item-box'); + var textbox = doc.getAnonymousNodes(itemBox)[0].getElementsByTagName('textbox')[0]; + textbox.value = value; + yield itemBox.blurOpenField(); + item = yield Zotero.Items.getAsync(item.id); + assert.equal(item.getField('title'), value); + }) + }); +})