From 7575cd8b29ab6823997a67948c8de072eccb9fe9 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Thu, 27 Dec 2018 07:11:15 -0500 Subject: [PATCH] Don't accept keyboard input before new-collection prompt appears Fixes #1613 --- chrome/content/zotero/advancedSearch.js | 14 +++-- chrome/content/zotero/xpcom/data/searches.js | 6 +- .../zotero/xpcom/utilities_internal.js | 56 ++++++++++++++----- chrome/content/zotero/zoteroPane.js | 42 ++++++++------ test/tests/utilities_internalTest.js | 18 ++++-- 5 files changed, 92 insertions(+), 44 deletions(-) diff --git a/chrome/content/zotero/advancedSearch.js b/chrome/content/zotero/advancedSearch.js index 9d6d841a60..ddf476a8b5 100644 --- a/chrome/content/zotero/advancedSearch.js +++ b/chrome/content/zotero/advancedSearch.js @@ -107,14 +107,16 @@ var ZoteroAdvancedSearch = new function() { var promptService = Components.classes["@mozilla.org/embedcomp/prompt-service;1"] .getService(Components.interfaces.nsIPromptService); - var untitled = yield Zotero.DB.getNextName( - _searchBox.search.libraryID, - 'savedSearches', - 'savedSearchName', - Zotero.getString('pane.collections.untitled') + var libraryID = _searchBox.search.libraryID; + + var searches = yield Zotero.Searches.getAll(libraryID) + var prefix = Zotero.getString('pane.collections.untitled'); + var name = Zotero.Utilities.Internal.getNextName( + prefix, + searches.map(s => s.name).filter(n => n.startsWith(prefix)) ); - var name = { value: untitled }; + var name = { value: name }; var result = promptService.prompt(window, Zotero.getString('pane.collections.newSavedSeach'), Zotero.getString('pane.collections.savedSearchName'), name, "", {}); diff --git a/chrome/content/zotero/xpcom/data/searches.js b/chrome/content/zotero/xpcom/data/searches.js index e402e7d987..b4c21c41e0 100644 --- a/chrome/content/zotero/xpcom/data/searches.js +++ b/chrome/content/zotero/xpcom/data/searches.js @@ -104,8 +104,8 @@ Zotero.Searches = function() { this.getNextName = async function (libraryID, name) { - // Trim '(1)', etc. - var matches = name.match(/^(.+) \(\d+\)$/); + // Trim '1', etc. + var matches = name.match(/^(.+) \d+$/); if (matches) { name = matches[1].trim(); } @@ -115,7 +115,7 @@ Zotero.Searches = function() { sql, [libraryID, Zotero.DB.escapeSQLExpression(name) + '%'] ); - return Zotero.Utilities.Internal.getNextName(name, names); + return Zotero.Utilities.Internal.getNextName(name, names, true); }; diff --git a/chrome/content/zotero/xpcom/utilities_internal.js b/chrome/content/zotero/xpcom/utilities_internal.js index 256cb247dc..0e756427b2 100644 --- a/chrome/content/zotero/xpcom/utilities_internal.js +++ b/chrome/content/zotero/xpcom/utilities_internal.js @@ -1194,23 +1194,51 @@ Zotero.Utilities.Internal = { /** * Get the next available numbered name that matches a base name, for use when duplicating * - * - Given 'Foo' and ['Foo'], returns 'Foo (1)'. - * - Given 'Foo (1)' and ['Foo', 'Foo (1)'], returns 'Foo (2)' + * - Given 'Foo' and ['Foo'], returns 'Foo 1'. + * - Given 'Foo' and ['Foo', 'Foo 1'], returns 'Foo 2'. + * - Given 'Foo' and ['Foo', 'Foo 1'], returns 'Foo 2'. + * - Given 'Foo 1', ['Foo', 'Foo 1'], and trim=true, returns 'Foo 2' + * - Given 'Foo' and ['Foo', 'Foo 2'], returns 'Foo 1' */ - getNextName: function (name, existingNames) { - // Trim '(1)', etc. - var matches = name.match(/^(.+) \(\d+\)$/); - if (matches) { - name = matches[1].trim(); - } - var highest = 0; - for (let existingName of existingNames) { - let matches = existingName.match(/ \((\d+)\)$/); - if (matches && matches[1] > highest) { - highest = matches[1]; + getNextName: function (name, existingNames, trim = false) { + // Trim numbers at end of given name + if (trim) { + let matches = name.match(/^(.+) \d+$/); + if (matches) { + name = matches[1].trim(); } } - return name + ' (' + ++highest + ')'; + + if (!existingNames.includes(name)) { + return name; + } + + var suffixes = existingNames + // Get suffix + .map(x => x.substr(name.length)) + // Get "2", "5", etc. + .filter(x => x.match(/^ (\d+)$/)); + + suffixes.sort(function (a, b) { + return parseInt(a) - parseInt(b); + }); + + // If no existing numbered names found, use 1 + if (!suffixes.length) { + return name + ' ' + 1; + } + + // Find first available number + var i = 0; + var num = 1; + while (suffixes[i] == num) { + while (suffixes[i + 1] && suffixes[i] == suffixes[i + 1]) { + i++; + } + i++; + num++; + } + return name + ' ' + num; }, diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 540e8eece3..1b597f2f6d 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -912,17 +912,23 @@ var ZoteroPane = new function() var libraryID = this.getSelectedLibraryID(); - var promptService = Components.classes["@mozilla.org/embedcomp/prompt-service;1"] - .getService(Components.interfaces.nsIPromptService); - var untitled = yield Zotero.DB.getNextName( - libraryID, - 'collections', - 'collectionName', - Zotero.getString('pane.collections.untitled') + // Get a unique "Untitled" name for this level in the collection hierarchy + var collections; + if (parentKey) { + let parent = Zotero.Collections.getIDFromLibraryAndKey(libraryID, parentKey); + collections = Zotero.Collections.getByParent(parent); + } + else { + collections = Zotero.Collections.getByLibrary(libraryID); + } + var prefix = Zotero.getString('pane.collections.untitled'); + var name = Zotero.Utilities.Internal.getNextName( + prefix, + collections.map(c => c.name).filter(n => n.startsWith(prefix)) ); - var newName = { value: untitled }; - var result = promptService.prompt(window, + var newName = { value: name }; + var result = Services.prompt.prompt(window, Zotero.getString('pane.collections.newCollection'), Zotero.getString('pane.collections.name'), newName, "", {}); @@ -991,18 +997,20 @@ var ZoteroPane = new function() yield Zotero.DB.waitForTransaction(); } + var libraryID = this.getSelectedLibraryID(); + var s = new Zotero.Search(); - s.libraryID = this.getSelectedLibraryID(); + s.libraryID = libraryID; s.addCondition('title', 'contains', ''); - var untitled = Zotero.getString('pane.collections.untitled'); - untitled = yield Zotero.DB.getNextName( - s.libraryID, - 'savedSearches', - 'savedSearchName', - Zotero.getString('pane.collections.untitled') + var searches = yield Zotero.Searches.getAll(libraryID) + var prefix = Zotero.getString('pane.collections.untitled'); + var name = Zotero.Utilities.Internal.getNextName( + prefix, + searches.map(s => s.name).filter(n => n.startsWith(prefix)) ); - var io = {dataIn: {search: s, name: untitled}, dataOut: null}; + + var io = { dataIn: { search: s, name }, dataOut: null }; window.openDialog('chrome://zotero/content/searchDialog.xul','','chrome,modal',io); if (!io.dataOut) { return false; diff --git a/test/tests/utilities_internalTest.js b/test/tests/utilities_internalTest.js index 514c3f060c..312160ce64 100644 --- a/test/tests/utilities_internalTest.js +++ b/test/tests/utilities_internalTest.js @@ -203,13 +203,23 @@ describe("Zotero.Utilities.Internal", function () { describe("#getNextName()", function () { it("should get the next available numbered name", function () { - var existing = ['Name (1)', 'Name (3)']; - assert.equal(Zotero.Utilities.Internal.getNextName('Name', existing), 'Name (4)'); + var existing = ['Name', 'Name 1', 'Name 3']; + assert.equal(Zotero.Utilities.Internal.getNextName('Name', existing), 'Name 2'); }); - it("should return 'Name (1)' if no numbered names", function () { + it("should return 'Name 1' if no numbered names", function () { var existing = ['Name']; - assert.equal(Zotero.Utilities.Internal.getNextName('Name', existing), 'Name (1)'); + assert.equal(Zotero.Utilities.Internal.getNextName('Name', existing), 'Name 1'); + }); + + it("should return 'Name' if only numbered names", function () { + var existing = ['Name 1', 'Name 3']; + assert.equal(Zotero.Utilities.Internal.getNextName('Name', existing), 'Name'); + }); + + it("should trim given name if trim=true", function () { + var existing = ['Name', 'Name 1', 'Name 2', 'Name 3']; + assert.equal(Zotero.Utilities.Internal.getNextName('Name 2', existing, true), 'Name 4'); }); }); })