wshanks / Zutilo

Zotero plugin providing some additional editing features
Other
1.53k stars 72 forks source link

Keyboard shortcut does not work #71

Closed yippiekahee closed 7 years ago

yippiekahee commented 7 years ago

In Zutilo in Zotero 4 I dedicated "shift+T" to open a tag, so I can immediately type my tag text in there. After upgrading to Zotero 5 my "shift+T" only jumps to the tag tab. So I still need my mouse for tag entries. That does not work for me since I hate my mouse.

wshanks commented 7 years ago

Confirmed. Some of the Zotero internals must have changed.

yippiekahee commented 7 years ago

Tnx. Does your finding make it a Zotero or a Zutilo issue?

wshanks commented 7 years ago

Zutilo. There isn't API for Zotero plugins. It just runs in the same context as Zotero's internals and hopes they don't change much. It looks like the new tag method was renamed from new() to newTag():

https://github.com/zotero/zotero/commit/a3eea03a38da31783342da8a1fea3fed70c796b4

wshanks commented 7 years ago

Okay, this was fixed by 61c7d818697649474f8680d60354756f8caf164a. I am attaching a fixed version as a zip file if you want to try it before I can make a release. Rename .zip to .xpi and then install it normally.

zutilo.zip

yippiekahee commented 7 years ago

indeed fixed! thank you

yippiekahee commented 7 years ago

Sorry, same issue returns: Paste Tags (shift+V in my settings) does not function anymore in Zotero 5

yippiekahee commented 7 years ago

ps. Paste Tags does work if it's only one item. If multiple items separated by a hard return are copied, none of them get pasted as tags using Paste Tags. This used to work in Zutillo/Zotero4

wshanks commented 7 years ago

Hmm, sorry, maybe I don't undestand. I just tried using Zutilo's copy tags function on an item with three tags and then using Zutilo's paste tags function on a second item. All three tags were added to the second item. Is that different from the case you are describing?

yippiekahee commented 7 years ago

That's strange: using Zutilo's copy (Zc) /paste (Zp) function everything is fine. But using copy (ctrl+c) in Word (Wc) - which is my standard procedure - Zp fails (if > 1 item).

And, if I do this on >1 item: Zc, than Wp, the items appear in Word. If I use Zp, the items appear in Zotero. But if I subsequently Wc these same items, Zp fails...

So they both use the Windows clipboard memory (Zc + Wp is ok), but in a different manner (Wc + Zp is not ok)

Does this clarify?

wshanks commented 7 years ago

Yes, that does. Could you start Zotero with the -jsconsole option and see an error message appears when you try to paste tags? Here are instructions for starting Zotero with -jsconsole:

https://www.zotero.org/support/reporting_problems#reporting_startup_errors

Best practice would be to select the item you want, clear the console, try to paste, and see if anything new appears in the error console.

My guess is that it is having trouble splitting the lists of tags. Windows uses different line endings than macOS/Linux. Zutilo should still split the list for any type of line ending but maybe something changed that affects the splitting. The copy tags function uses the macOS/Linux line ending to separate the items.

yippiekahee commented 7 years ago

I tried adding the -jsconsole. But system says 'Windows cannot find' followed by path. I used "C:\Program Files (x86)\Zotero\zotero.exe -jsconsole" Updating screendump in png or jpg does noet function

removing "-jsconsole" does run Zotero, so the path is correct

wshanks commented 7 years ago

I think you don't want -jsconsole in the same quotation marks as zotero.exe (see a similar discussion here: https://forums.zotero.org/discussion/67609/trouble-launching-zotero-5-0).

yippiekahee commented 7 years ago

1&2 directly after starting Zotero, 3&4 after trying to paste 3 items

Timestamp: 26-09-2017 19:29:16 Warning: Use of nsIDOMWindowInternal is deprecated. Use nsIDOMWindow instead. Source File: chrome://zutilo/content/zutilo.jsm Line: 110

Timestamp: 26-09-2017 19:29:16 Warning: unreachable code after return statement Source File: resource://gre/modules/commonjs/toolkit/loader.js -> resource://zotero/bluebird/util.js Line: 201, Column: 4 Source Code: eval(obj);

Timestamp: 26-09-2017 19:29:57 Error: Error(s) encountered during statement execution: Tag cannot be blank [QUERY: INSERT INTO tags (tagID, name) VALUES (?, ?)] [PARAMS: 14020, ] [ERROR: Tag cannot be blank] Source File: chrome://zotero/content/xpcom/db.js Line: 703

Timestamp: 26-09-2017 19:29:57 Error: Error(s) encountered during statement execution: Tag cannot be blank [QUERY: INSERT INTO tags (tagID, name) VALUES (?, ?)] [PARAMS: 14020, ] [ERROR: Tag cannot be blank] Source File: chrome://zotero/content/xpcom/db.js Line: 703

wshanks commented 7 years ago

It seems like it is not splitting on the newlines properly and is ending up with empty entries in the list of tags to add. The code is modeled on the code used for handling lists of tags pasted into a tag entry textbox. The Zotero code for that was updated since Zutilo's paste function was updated. I tried adopting those changes in the attached zip (rename to xpi as before) zutilo.zip . You can see if it works better.

yippiekahee commented 7 years ago

I'm sorry, it works worse: 1 item tags as well as >1 item tags do not past anymore into my tag list. Thank you though

wshanks commented 7 years ago

Yeah, sorry, there was a syntax error in that version. I had a chance to test it on a Windows system today and figured out what the issue was. I should be able to fix it soon.

wshanks commented 7 years ago

Here is a new xpi. I think it should fix the problem now.

zutilo.zip

yippiekahee commented 7 years ago

Issue seems to be fixed. thank you

bjohas commented 6 years ago

Hello! I seems that "rename attachments" (under shortcuts) doesn't work... maybe another internal has changed there?

wshanks commented 6 years ago

Hmm, it works for me when I have an attachment item selected. If I have a regular item or a comment selected, then I get an error. Is that what you see? I suppose Zutilo could filter out the non-attachment items or at least stop rather than generate an error.

bjohas commented 6 years ago

AH! My keyboard shortcut conflicted with "toggleAllRead". Though weirdly (on OS X) not only shift-cmd R, but also shift-alt-cmd R, shif-alt-ctrl-cmd R all trigger toggleAllRead. But avoiding R altogether made it work. Do you have an explanation for this weird capture of R, or should I post to the forum?

wshanks commented 6 years ago

Zutilo uses the method built-in to Firefox for registering global keyboard shortcuts/commands (var key = document.createElement('key'); key.addEventListener('command', function() {...})). This binds the key with its modifiers to a single function. For the most part, Zotero uses onkeypress handlers and processes the key events itself. This gives it more granular control over keyboard events (I'm not sure but it might also have been more important when Zotero was a Firefox add-on and had to work around Firefox commands). In the case of the extra modifiers being ignored, it might be that Zotero is only checking the key event for the presence of shift, cmd, and r and not checking for alt or ctrl. Maybe @dstillman could comment on whether what you see is a bug.

One downside of Zotero using key press handlers is that Zutilo's code to check for conflicting key commands does not check for conflicts with Zotero's shortcuts, as you discovered. Zutilo should do a separate check against them.

bjohas commented 6 years ago

Thanks for investigating, that's really helpful!