Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keyboard shortcut does not work #71

Closed
yippiekahee opened this issue Sep 15, 2017 · 23 comments
Closed

Keyboard shortcut does not work #71

yippiekahee opened this issue Sep 15, 2017 · 23 comments

Comments

@yippiekahee
Copy link

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
Copy link
Owner

wshanks commented Sep 16, 2017

Confirmed. Some of the Zotero internals must have changed.

@yippiekahee
Copy link
Author

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

@wshanks
Copy link
Owner

wshanks commented Sep 19, 2017

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():

zotero/zotero@a3eea03

@wshanks
Copy link
Owner

wshanks commented Sep 21, 2017

Okay, this was fixed by 61c7d81. 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

@wshanks wshanks closed this as completed Sep 21, 2017
@yippiekahee
Copy link
Author

indeed fixed! thank you

@yippiekahee
Copy link
Author

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

@yippiekahee
Copy link
Author

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
Copy link
Owner

wshanks commented Sep 26, 2017

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
Copy link
Author

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
Copy link
Owner

wshanks commented Sep 26, 2017

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
Copy link
Author

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
Copy link
Owner

wshanks commented Sep 26, 2017

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
Copy link
Author

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
Copy link
Owner

wshanks commented Sep 27, 2017

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
Copy link
Author

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
Copy link
Owner

wshanks commented Sep 27, 2017

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
Copy link
Owner

wshanks commented Sep 28, 2017

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

zutilo.zip

@yippiekahee
Copy link
Author

Issue seems to be fixed. thank you

@bjohas
Copy link
Contributor

bjohas commented Jan 26, 2018

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

@wshanks
Copy link
Owner

wshanks commented Jan 27, 2018

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
Copy link
Contributor

bjohas commented Jan 27, 2018

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
Copy link
Owner

wshanks commented Jan 27, 2018

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
Copy link
Contributor

bjohas commented Jan 29, 2018

Thanks for investigating, that's really helpful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants