commit 8fec5ace3ae3161fe38a63de5e56504a9df2feb4
parent 590649fd49977612544d926be5674965d6abdbee
Author: Dan Stillman <dstillman@zotero.org>
Date: Mon, 4 May 2015 02:00:52 -0400
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.
Diffstat:
7 files changed, 103 insertions(+), 63 deletions(-)
diff --git a/chrome/content/zotero/bindings/itembox.xml b/chrome/content/zotero/bindings/itembox.xml
@@ -1682,8 +1682,6 @@
<parameter name="textbox"/>
<body><![CDATA[
return Zotero.spawn(function* () {
- if (textbox.localName == 'textarea') return;
-
Zotero.debug('Hiding editor');
// Prevent autocomplete breakage in Firefox 3
@@ -1785,11 +1783,7 @@
}
}
- elem = this.createValueElement(
- val,
- fieldName,
- tabindex
- );
+ var newVal = val;
}
// Fields
@@ -1854,18 +1848,23 @@
}
}
- this._modifyField(fieldName, value, this.saveOnEdit);
+ yield this._modifyField(fieldName, value, this.saveOnEdit);
+ var newVal = this.item.getField(fieldName);
+ }
+
+ // If box is still open (due to field not being modified and there not being
+ // a refresh), close it manually
+ if (textbox && textbox.parentNode) {
elem = this.createValueElement(
this.item.getField(fieldName),
fieldName,
tabindex
);
+ var box = textbox.parentNode;
+ box.replaceChild(elem,textbox);
}
- var box = textbox.parentNode;
- box.replaceChild(elem,textbox);
-
if(field === 'creator') {
// Reset creator mode settings here so that flex attribute gets reset
this.switchCreatorMode(row, (otherFields.fieldMode ? 1 : 0), true);
@@ -1912,14 +1911,14 @@
<parameter name="field"/>
<parameter name="value"/>
<parameter name="save"/>
- <body>
- <![CDATA[
- this.item.setField(field,value);
- if (save) {
- this.item.save();
- }
- ]]>
- </body>
+ <body><![CDATA[
+ return Zotero.spawn(function* () {
+ this.item.setField(field, value);
+ if (save) {
+ yield this.item.save();
+ }
+ }, this);
+ ]]></body>
</method>
@@ -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();
]]>
</body>
</method>
@@ -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);
]]></body>
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
+ })
+ });
+})
