From 7d74344b7d1e73ba990921ac3f0a69db706ebf2c Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 20 Jul 2014 02:03:02 -0400 Subject: [PATCH 1/7] Restore highlighting of collection row during drag on Windows --- chrome/content/zotero-platform/win/overlay.css | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/chrome/content/zotero-platform/win/overlay.css b/chrome/content/zotero-platform/win/overlay.css index 464090cbc..b90ec7c58 100644 --- a/chrome/content/zotero-platform/win/overlay.css +++ b/chrome/content/zotero-platform/win/overlay.css @@ -78,6 +78,11 @@ border-width: 0 1px 1px 0; } +/* Restore row highlighting on drag over, though I'm not sure how we're losing it to begin with. */ +#zotero-collections-tree treechildren::-moz-tree-row(dropOn) { + background-color: Highlight; +} + #zotero-items-tree { border-width: 0 1px; } From b8f69d628167b31f851da8b88e303d2c60d824d5 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 20 Jul 2014 02:11:11 -0400 Subject: [PATCH 2/7] Fix Quick Copy drag to Chrome textareas on Windows Possibly to other places as well (but not Notepad, which doesn't work from Firefox or Chrome either) Unfortunately this requires going back to 'copy' cursor feedback when dragging, even when Shift is used. We can only choose one on Windows (as far as I can tell), and we were previously using the unadorned 'move'. --- chrome/content/zotero/xpcom/itemTreeView.js | 2 +- chrome/content/zotero/xpcom/libraryTreeView.js | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js index 23e1ac159..7f7b3faf7 100644 --- a/chrome/content/zotero/xpcom/itemTreeView.js +++ b/chrome/content/zotero/xpcom/itemTreeView.js @@ -2558,7 +2558,7 @@ Zotero.ItemTreeCommandController.prototype.onEvent = function(evt) Zotero.ItemTreeView.prototype.onDragStart = function (event) { // See note in LibraryTreeView::_setDropEffect() if (Zotero.isWin) { - event.dataTransfer.effectAllowed = 'move'; + event.dataTransfer.effectAllowed = 'copy'; } var itemIDs = this.saveSelection(); diff --git a/chrome/content/zotero/xpcom/libraryTreeView.js b/chrome/content/zotero/xpcom/libraryTreeView.js index a6cd76b35..90f25e055 100644 --- a/chrome/content/zotero/xpcom/libraryTreeView.js +++ b/chrome/content/zotero/xpcom/libraryTreeView.js @@ -208,6 +208,11 @@ Zotero.LibraryTreeView.prototype = { // as the dropEffect. This allows the dropEffect setting (which we use // in the tree's canDrop() and drop() to determine the desired action) // to be changed, even if the cursor doesn't reflect the new setting. + // + // Update (07/2014): We now use 'copy' for effectAllowed on Windows in + // onDragStart, since with 'move' text can't be dragged to Chrome + // textareas (and probably other places). This locks the cursor at copy, + // even when Shift is used, but that seems less bad. if (Zotero.isWin) { event.dataTransfer.effectAllowed = effect; } From 717a440156fa117d6eb0db0a31487a010f73c0ec Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 20 Jul 2014 02:50:00 -0400 Subject: [PATCH 3/7] Clarify comment for Windows drag-and-drop mess --- .../content/zotero/xpcom/libraryTreeView.js | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/chrome/content/zotero/xpcom/libraryTreeView.js b/chrome/content/zotero/xpcom/libraryTreeView.js index 90f25e055..3ccef0b32 100644 --- a/chrome/content/zotero/xpcom/libraryTreeView.js +++ b/chrome/content/zotero/xpcom/libraryTreeView.js @@ -198,21 +198,21 @@ Zotero.LibraryTreeView.prototype = { // and 'copy'/'link' for drags with system-default modifier keys, // as long as the actions are allowed by the initial effectAllowed set // in onDragStart, regardless of the effectAllowed or dropEffect set - // in onDragOver. To prevent inaccurate 'copy'/'link' cursors, we set - // effectAllowed to 'move' in onDragStart, which locks the cursor at - // 'move'. ('none' still changes the cursor, but 'copy'/'link' do not.) + // in onDragOver. It doesn't seem to be possible to use 'copy' for + // the default and 'move' for modified, as we need to in the collections + // tree. To prevent inaccurate 'copy'/'link' cursors, we set effectAllowed + // to 'copy' in onDragStart, which locks the cursor at 'copy'. ('none' still + // changes the cursor, but 'move'/'link' do not.) We use 'copy' instead of + // 'move' because with 'move' text can't be dragged to Chrome textareas + // (and probably other places), and that seems worse than always showing + // 'copy' feedback. // - // However, since effectAllowed is enforced, leaving it at 'move' - // would prevent our default 'copy' from working, so we also have to + // However, since effectAllowed is enforced, leaving it at 'copy' + // would prevent our modified 'move' from working, so we also have to // set effectAllowed here (called from onDragOver) to the same action // as the dropEffect. This allows the dropEffect setting (which we use // in the tree's canDrop() and drop() to determine the desired action) // to be changed, even if the cursor doesn't reflect the new setting. - // - // Update (07/2014): We now use 'copy' for effectAllowed on Windows in - // onDragStart, since with 'move' text can't be dragged to Chrome - // textareas (and probably other places). This locks the cursor at copy, - // even when Shift is used, but that seems less bad. if (Zotero.isWin) { event.dataTransfer.effectAllowed = effect; } From 3b5bd454336c64ac5878620a7fac33578161822e Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 20 Jul 2014 13:16:56 -0400 Subject: [PATCH 4/7] Further Windows drag-and-drop comment rewriting --- .../content/zotero/xpcom/libraryTreeView.js | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/chrome/content/zotero/xpcom/libraryTreeView.js b/chrome/content/zotero/xpcom/libraryTreeView.js index 3ccef0b32..548ded974 100644 --- a/chrome/content/zotero/xpcom/libraryTreeView.js +++ b/chrome/content/zotero/xpcom/libraryTreeView.js @@ -194,25 +194,25 @@ Zotero.LibraryTreeView.prototype = { _setDropEffect: function (event, effect) { - // On Windows (in Fx26), Firefox uses 'move' for unmodified drags, - // and 'copy'/'link' for drags with system-default modifier keys, + // On Windows (in Fx26), Firefox uses 'move' for unmodified drags + // and 'copy'/'link' for drags with system-default modifier keys // as long as the actions are allowed by the initial effectAllowed set // in onDragStart, regardless of the effectAllowed or dropEffect set // in onDragOver. It doesn't seem to be possible to use 'copy' for // the default and 'move' for modified, as we need to in the collections - // tree. To prevent inaccurate 'copy'/'link' cursors, we set effectAllowed - // to 'copy' in onDragStart, which locks the cursor at 'copy'. ('none' still - // changes the cursor, but 'move'/'link' do not.) We use 'copy' instead of - // 'move' because with 'move' text can't be dragged to Chrome textareas - // (and probably other places), and that seems worse than always showing - // 'copy' feedback. + // tree. To prevent inaccurate cursor feedback, we set effectAllowed to + // 'copy' in onDragStart, which locks the cursor at 'copy'. ('none' still + // changes the cursor, but 'move'/'link' do not.) It'd be better to use + // the unadorned 'move', but we use 'copy' instead because with 'move' text + // can't be dragged to some external programs (e.g., Chrome, Notepad++), + // which seems worse than always showing 'copy' feedback. // // However, since effectAllowed is enforced, leaving it at 'copy' - // would prevent our modified 'move' from working, so we also have to - // set effectAllowed here (called from onDragOver) to the same action - // as the dropEffect. This allows the dropEffect setting (which we use - // in the tree's canDrop() and drop() to determine the desired action) - // to be changed, even if the cursor doesn't reflect the new setting. + // would prevent our modified 'move' in the collections tree from working, + // so we also have to set effectAllowed here (called from onDragOver) to + // the same action as the dropEffect. This allows the dropEffect setting + // (which we use in the tree's canDrop() and drop() to determine the desired + // action) to be changed, even if the cursor doesn't reflect the new setting. if (Zotero.isWin) { event.dataTransfer.effectAllowed = effect; } From 15108eea3f099a5a39a145ed55043d1ed9889e6a Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Sun, 20 Jul 2014 15:56:55 -0400 Subject: [PATCH 5/7] Fix intermittent source file deletion when dragging files in on Windows (at least, if this is caused by what I think it was caused by) https://forums.zotero.org/discussion/38486/ --- chrome/content/zotero/xpcom/itemTreeView.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/chrome/content/zotero/xpcom/itemTreeView.js b/chrome/content/zotero/xpcom/itemTreeView.js index 7f7b3faf7..6b0c8de16 100644 --- a/chrome/content/zotero/xpcom/itemTreeView.js +++ b/chrome/content/zotero/xpcom/itemTreeView.js @@ -2896,7 +2896,7 @@ Zotero.ItemTreeView.prototype.canDropCheck = function (row, orient, dataTransfer var itemGroup = this._itemGroup; - if (orient == 0) { + if (row != -1 && orient == 0) { var rowItem = this._getItemAtRow(row).ref; // the item we are dragging over } @@ -2904,7 +2904,7 @@ Zotero.ItemTreeView.prototype.canDropCheck = function (row, orient, dataTransfer var items = Zotero.Items.get(ids); // Directly on a row - if (orient == 0) { + if (rowItem) { var canDrop = false; for each(var item in items) { @@ -2981,7 +2981,7 @@ Zotero.ItemTreeView.prototype.canDropCheck = function (row, orient, dataTransfer } else if (dataType == "text/x-moz-url" || dataType == 'application/x-moz-file') { // Disallow direct drop on a non-regular item (e.g. note) - if (orient == 0) { + if (rowItem) { if (!rowItem.isRegularItem()) { return false; } From 638d29972c71452c7af216e9167a0684b5e055c4 Mon Sep 17 00:00:00 2001 From: Aurimas Vinckevicius Date: Fri, 18 Jul 2014 00:03:55 -0500 Subject: [PATCH 6/7] Don't ignore initial punctuation when sorting Also, change sorting sensitivity to "base" Closes #514 --- chrome/content/zotero/xpcom/zotero.js | 39 +++++++++++++++++++++++++-- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/chrome/content/zotero/xpcom/zotero.js b/chrome/content/zotero/xpcom/zotero.js index 28e12c0bc..82c20cd75 100644 --- a/chrome/content/zotero/xpcom/zotero.js +++ b/chrome/content/zotero/xpcom/zotero.js @@ -1490,7 +1490,8 @@ Components.utils.import("resource://gre/modules/Services.jsm"); locale = locale.match(/^[a-z]{2}(\-[A-Z]{2})?/)[0]; var collator = new Intl.Collator(locale, { ignorePunctuation: true, - numeric: true + numeric: true, + sensitivity: 'base' }); } catch (e) { @@ -1504,10 +1505,44 @@ Components.utils.import("resource://gre/modules/Services.jsm"); }; } + // Grab all ASCII punctuation and space at the begining of string + var initPunctuationRE = /^[\x20-\x2F\x3A-\x40\x5B-\x60\x7B-\x7E]+/; + // Punctuation that should be ignored when sorting + var ignoreInitRE = /["'[{(]+$/; + // Until old code is updated, pretend we're returning an nsICollation return this.collation = { compareString: function (_, a, b) { - return collator.compare(a, b); + if (!a && !b) return 0; + if (!a || !b) return b ? -1 : 1; + + // Compare initial punctuation + var aInitP = initPunctuationRE.exec(a) || ''; + var bInitP = initPunctuationRE.exec(b) || ''; + + var aWordStart = 0, bWordStart = 0; + if (aInitP) { + aWordStart = aInitP[0].length; + aInitP = aInitP[0].replace(ignoreInitRE, ''); + } + if (bInitP) { + bWordStart = bInitP.length; + bInitP = bInitP[0].replace(ignoreInitRE, ''); + } + + // If initial punctuation is equivalent, use collator comparison + // that ignores all punctuation + if (aInitP == bInitP || !aInitP && !bInitP) return collator.compare(a, b); + + // Otherwise consider "attached" words as well, e.g. the order should be + // "__ n", "__z", "_a" + // We don't actually care what the attached word is, just whether it's + // there, since at this point we're guaranteed to have non-equivalent + // initial punctuation + if (aWordStart < a.length) aInitP += 'a'; + if (bWordStart < b.length) bInitP += 'a'; + + return aInitP.localeCompare(bInitP); } }; } From 921910090154ca58aafab3fcfd868f3572844977 Mon Sep 17 00:00:00 2001 From: Simon Kornblith Date: Wed, 23 Jul 2014 18:04:41 -0400 Subject: [PATCH 7/7] Another Fx 32 translation fix --- chrome/content/zotero/xpcom/translation/translate.js | 1 + 1 file changed, 1 insertion(+) diff --git a/chrome/content/zotero/xpcom/translation/translate.js b/chrome/content/zotero/xpcom/translation/translate.js index 959a2a57c..a098624ff 100644 --- a/chrome/content/zotero/xpcom/translation/translate.js +++ b/chrome/content/zotero/xpcom/translation/translate.js @@ -346,6 +346,7 @@ Zotero.Translate.Sandbox = { if(!Zotero.Utilities.isEmpty(sandbox.exports)) { sandbox.exports.Zotero = sandbox.Zotero; sandbox = sandbox.exports; + if(Zotero.isFx && sandbox.wrappedJSObject) sandbox = sandbox.wrappedJSObject; } else { translate._debug("COMPAT WARNING: "+translation.translator[0].label+" does "+ "not export any properties. Only detect"+translation._entryFunctionSuffix+