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

"Save script" shortcut should match platform (ctrl vs. cmd) #2690

Closed
adaugherity opened this Issue Nov 17, 2017 · 13 comments

Comments

Projects
None yet
7 participants
@adaugherity

adaugherity commented Nov 17, 2017

After upgrading to FF 57 (which upgraded GM to v4), my script no longer works. I tried to fix it via the editor, but appeared to be unable to save my changes as there is no save button and Cmd-s brought up the Firefox Save Page As... dialog.

I eventually discovered https://groups.google.com/forum/#!topic/greasemonkey-users/GLTDE-G9LZI indicating there are not (yet) any buttons, and Ctrl-S is the shortcut to save. That did indeed work, but is unexpected on macOS. Can we please make the shortcut be ⌘S (Cmd-s) on Mac?

@arantius arantius added this to the 4.x milestone Nov 17, 2017

badbrainz added a commit to badbrainz/greasemonkey that referenced this issue Nov 17, 2017

Issue greasemonkey#2690 Define a codemirror "save" command
CodeMirror's default keymap already handles 'ctrl-s' and even runs
a 'save' command when available.

see docs
https://codemirror.net/doc/manual.html#commands

@arantius arantius closed this in e232935 Nov 19, 2017

@arantius arantius modified the milestones: 4.x, 4.1 Nov 23, 2017

Sxderp added a commit to Sxderp/greasemonkey that referenced this issue Nov 23, 2017

Issue greasemonkey#2690 Define a codemirror "save" command
CodeMirror's default keymap already handles 'ctrl-s' and even runs
a 'save' command when available.

see docs
https://codemirror.net/doc/manual.html#commands
@arantius

This comment has been minimized.

Show comment
Hide comment
@arantius

arantius Nov 28, 2017

Collaborator

This is in build 4.1beta5 ( https://addons.mozilla.org/firefox/downloads/file/793791/greasemonkey-4.1beta5-an+fx.xpi?src=devhub ). Testing and confirming would be appreciated!

Collaborator

arantius commented Nov 28, 2017

This is in build 4.1beta5 ( https://addons.mozilla.org/firefox/downloads/file/793791/greasemonkey-4.1beta5-an+fx.xpi?src=devhub ). Testing and confirming would be appreciated!

@adaugherity

This comment has been minimized.

Show comment
Hide comment
@adaugherity

adaugherity Nov 30, 2017

Tested 4.1b5 on Mac, and the save shortcut is now cmd-s. 👍

I do see some warnings logged to the browser console -- are these known issues?

Upon opening the editor:

Loading failed for the <script> with source “moz-extension://e8447a6c-bfa1-8e46-a717-cd76d4efe15c/third-party/codemirror/addon/scroll/annotatesscrollbar.js”.
edit-user-script.html:16

Saving a script:

updater has no added remotes to handle
user-script-obj.js:349:9

adaugherity commented Nov 30, 2017

Tested 4.1b5 on Mac, and the save shortcut is now cmd-s. 👍

I do see some warnings logged to the browser console -- are these known issues?

Upon opening the editor:

Loading failed for the <script> with source “moz-extension://e8447a6c-bfa1-8e46-a717-cd76d4efe15c/third-party/codemirror/addon/scroll/annotatesscrollbar.js”.
edit-user-script.html:16

Saving a script:

updater has no added remotes to handle
user-script-obj.js:349:9
@Sxderp

This comment has been minimized.

Show comment
Hide comment
@Sxderp

Sxderp Nov 30, 2017

Contributor

Upon opening the editor:

Not sure about that. Though I think it may be a bug, as far as I know all the codemirror things should load properly.

Saving a script:

Standard. Just some debug info detailing that there are no @require nor @resource tags.

Contributor

Sxderp commented Nov 30, 2017

Upon opening the editor:

Not sure about that. Though I think it may be a bug, as far as I know all the codemirror things should load properly.

Saving a script:

Standard. Just some debug info detailing that there are no @require nor @resource tags.

@xor10

This comment has been minimized.

Show comment
Hide comment
@xor10

xor10 Nov 30, 2017

Contributor

Not sure about that. Though I think it may be a bug, as far as I know all the codemirror things should load properly.

Typo.

diff --git a/src/content/edit-user-script.html b/src/content/edit-user-script.html
index dc3d16f..91ddbc8 100644
--- a/src/content/edit-user-script.html
+++ b/src/content/edit-user-script.html
@@ -13,7 +13,7 @@
 <script src="/third-party/codemirror/addon/dialog/dialog.js"></script>
 <link rel="stylesheet" href="/third-party/codemirror/addon/dialog/dialog.css">
 
-<script src="/third-party/codemirror/addon/scroll/annotatesscrollbar.js"></script>
+<script src="/third-party/codemirror/addon/scroll/annotatescrollbar.js"></script>
 
 <script src="/third-party/codemirror/addon/search/searchcursor.js"></script>
 <script src="/third-party/codemirror/addon/search/search.js"></script>
Contributor

xor10 commented Nov 30, 2017

Not sure about that. Though I think it may be a bug, as far as I know all the codemirror things should load properly.

Typo.

diff --git a/src/content/edit-user-script.html b/src/content/edit-user-script.html
index dc3d16f..91ddbc8 100644
--- a/src/content/edit-user-script.html
+++ b/src/content/edit-user-script.html
@@ -13,7 +13,7 @@
 <script src="/third-party/codemirror/addon/dialog/dialog.js"></script>
 <link rel="stylesheet" href="/third-party/codemirror/addon/dialog/dialog.css">
 
-<script src="/third-party/codemirror/addon/scroll/annotatesscrollbar.js"></script>
+<script src="/third-party/codemirror/addon/scroll/annotatescrollbar.js"></script>
 
 <script src="/third-party/codemirror/addon/search/searchcursor.js"></script>
 <script src="/third-party/codemirror/addon/search/search.js"></script>

badbrainz added a commit to badbrainz/greasemonkey that referenced this issue Dec 20, 2017

Issue greasemonkey#2690 Define a codemirror "save" command
CodeMirror's default keymap already handles 'ctrl-s' and even runs
a 'save' command when available.

see docs
https://codemirror.net/doc/manual.html#commands
@ramast

This comment has been minimized.

Show comment
Hide comment
@ramast

ramast Dec 22, 2017

I am using Greasemonkey v4.1 with firefox 57.0.1 and I am unable to save any greasemonkey script. Ctrl+S show firefox's "save page as" dialog and there is no save button as mentioned above. wondering if that has something to do with that fix

ramast commented Dec 22, 2017

I am using Greasemonkey v4.1 with firefox 57.0.1 and I am unable to save any greasemonkey script. Ctrl+S show firefox's "save page as" dialog and there is no save button as mentioned above. wondering if that has something to do with that fix

@Eselce

This comment has been minimized.

Show comment
Hide comment
@Eselce

Eselce Dec 22, 2017

Contributor

Which OS? (Confirmed: There is no button yet!)

Contributor

Eselce commented Dec 22, 2017

Which OS? (Confirmed: There is no button yet!)

@ramast

This comment has been minimized.

Show comment
Hide comment
@ramast

ramast Dec 22, 2017

Sorry I forgot to mention that, I am running Linux 64bit. Firefox installed directly from mozilla not through any kind of package manager

ramast commented Dec 22, 2017

Sorry I forgot to mention that, I am running Linux 64bit. Firefox installed directly from mozilla not through any kind of package manager

@Eselce

This comment has been minimized.

Show comment
Hide comment
@Eselce

Eselce Dec 22, 2017

Contributor

I'm using Linux since 1991, but sorry, no experience with Firefox/Linux here. I assume, that you may use Ctrl-S shortcut. Any hotkey manager active?

Contributor

Eselce commented Dec 22, 2017

I'm using Linux since 1991, but sorry, no experience with Firefox/Linux here. I assume, that you may use Ctrl-S shortcut. Any hotkey manager active?

@ramast

This comment has been minimized.

Show comment
Hide comment
@ramast

ramast Dec 22, 2017

Ctrl+S is hotkey for Firefox's "Save Page As" menu item.
Firefox receives the hot key and handle it by showing the "Save Page As" dialog.
I have no knowladge of greasemonkey's coding but I suppose you ask firefox not to handle Ctrl+S and instead notify greasemoneky? if so then probably that part is not working.

ramast commented Dec 22, 2017

Ctrl+S is hotkey for Firefox's "Save Page As" menu item.
Firefox receives the hot key and handle it by showing the "Save Page As" dialog.
I have no knowladge of greasemonkey's coding but I suppose you ask firefox not to handle Ctrl+S and instead notify greasemoneky? if so then probably that part is not working.

@ramast

This comment has been minimized.

Show comment
Hide comment
@ramast

ramast Dec 22, 2017

I tried on a fresh install (without any other addons) and it worked perfectly fine. So maybe this is caused by addons or because of migration from older version of greasemonkey.

Please disregard my comment, If I figured out the cause and it was a bug I'd raise the issue.

Cheers

ramast commented Dec 22, 2017

I tried on a fresh install (without any other addons) and it worked perfectly fine. So maybe this is caused by addons or because of migration from older version of greasemonkey.

Please disregard my comment, If I figured out the cause and it was a bug I'd raise the issue.

Cheers

@Eselce

This comment has been minimized.

Show comment
Hide comment
@Eselce

Eselce Dec 22, 2017

Contributor

As for me, it works perfectly fine as well. You're welcome.

Contributor

Eselce commented Dec 22, 2017

As for me, it works perfectly fine as well. You're welcome.

@Sxderp

This comment has been minimized.

Show comment
Hide comment
@Sxderp

Sxderp Dec 23, 2017

Contributor

The problem is usually due to some error with the scripts loading into the editor. I've had it happen before but I usually debug the problem and move on. I'm 90% sure that it can be prevented with some better error handling, but I don't recall where. Unfortunately I haven't run into the problem in a while.

Contributor

Sxderp commented Dec 23, 2017

The problem is usually due to some error with the scripts loading into the editor. I've had it happen before but I usually debug the problem and move on. I'm 90% sure that it can be prevented with some better error handling, but I don't recall where. Unfortunately I haven't run into the problem in a while.

@mrf345

This comment has been minimized.

Show comment
Hide comment
@mrf345

mrf345 Mar 31, 2018

Same issue on Linux, finally solved it by resorting to onSave() after looking into the editor sourcecode.

Buttons are unified across all browsers and operating systems, shortcuts are not !

mrf345 commented Mar 31, 2018

Same issue on Linux, finally solved it by resorting to onSave() after looking into the editor sourcecode.

Buttons are unified across all browsers and operating systems, shortcuts are not !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment