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 shortcuts that are deleted reappear (and are in conflict) #233

Closed
grahamperrin opened this issue Apr 28, 2021 · 7 comments
Closed

Comments

@grahamperrin
Copy link

Steps

  1. With Firefox 88, enable Swift Selection Search 3.47.3 and Tab Manager Plus for Firefox 5.2.0
  2. observe the conflicting shortcut
  3. delete the two shortcuts for Swift Selection Search
  4. quit
  5. start Firefox

Expected

  1. no conflict

Actual result

  1. conflict; reappearance of the two shortcuts that were deleted.

This is a major issue only in that Swift Selection Search must be disabled to prevent repetitive shortcut conflicts.

For me personally, not a major issue; I can live without the extension.

https://github.com/CanisLupus/swift-selection-search/blame/master/README.md#L6 noted and understood. Thanks for all the fish.

@CanisLupus
Copy link
Owner

Hi Graham,

I think I know what you are seeing. Are you using Firefox's "Manage Extension Shortcuts" to disable the shortcuts?

image

If so, SSS's shortcuts should be disabled from its own settings menu, by deleting the shortcut (the text), here:

image

I realize this is not intuitive for users that expect the other menu to work, and I fully agree. There a reason it's like this, but not a terribly good one. :) SSS had shortcuts (at least one) when it was first created in 2016, even before being a WebExtension. When WebExtensions first got support for shortcuts, I re-added it as a setting with its own menu, as you see above. At that time I think that the "Manage Extension Shortcuts" menu didn't exist and I'm not aware when it was added.

I remember being told by a user later (possibly here on GitHub?) that the menu existed and there was this problem, but it wasn't a big priority to fix things in Firefox's shortcuts menu, so it was left like that.

So yes, I consider this a bug, but it should be possible to solve on the user side (not ideal!) by deleting the shortcut text in SSS's settings menu ("Popup/icons behaviour" section).

I hope this helps! :) Cheers!

@CanisLupus
Copy link
Owner

By the way, the reason it reappears on restart is that SSS stores the shortcut as text in its settings, and it never gets the memo that the user deleted it in Firefox's menu, so it simply registers the shortcut again when it starts.

@grahamperrin

This comment has been minimized.

@CanisLupus
Copy link
Owner

CanisLupus commented Apr 30, 2021

No problem!

Thanks for understanding, Graham. :) I think we can leave this issue open. It's an actual bug and it's not summarized elsewhere (that I remember). If development resumes at some point, this will be a good reminder.

@CanisLupus
Copy link
Owner

CanisLupus commented Jun 21, 2022

Hey! Here's an update after I made some preliminary research on what it would take to fix this in SSS (it looks like a lot but it's not too bad, but it was more than I anticipated since the browser shortcuts menu only exists after Firefox 66). This mostly serves as a summary and also as a reminder for me, so no need to reply. :) Development hasn't resumed, but I'm okay with dealing with the bugs that show up. 😁

Research:

  • The menu was added in Firefox 66.
  • The menu allows changing and deleting a shortcut, but does not allow resetting it to its default value.
  • Whenever a setting is changed or when the browser starts, SSS will overwrite the shortcuts with the ones it has saved, regardless of what changes the user might have made in the menu.
  • SSS has no way of knowing of any changes in that menu, which means that it's not possible to make it play well with it. It's possible to not override the shortcuts on start/save, but it will never display the correct values if they were changed in the menu.
  • The only solution is to disable the shortcut options in the SSS settings page and direct the user to the new browser menu.
  • Since the menu was added in Firefox 66 but SSS supports Firefox 63, SSS needs to keep all the code and functionality it has currently. The settings will just have to work/display differently only after Firefox 66.

Required changes (if running on Firefox 66+):

  • Remove text boxes from the settings menu for options "Shortcut to open popup" and "Shortcut to toggle auto popup opening".
  • Change description text for the options to explain where the shortcuts can be changed now, and what defaults they have (since there's no option to reset to defaults).
  • Disable "Default" buttons on both settings. They could stay and actually reset the shortcuts, but they would need a confirmation message since the user wouldn't see the difference on the page. Most importantly, it would complicate the code significantly because saving the settings won't touch the shortcuts, but resetting causes a save.
  • Improve shortcut names (descriptions) in the manifest, since they are important to the user now (this must affect versions before 63, but it's not like better names will hurt users on old versions :p).
  • Change code in setup_Commands() to skip updating the shortcuts on start/save.
  • Delete the shortcut settings from the saved data on read (they're not needed anymore).
  • Add info to the "Reset options" button saying it doesn't affect the shortcuts. Resetting all settings or loading a backup (from a file or from Firefox Sync) shouldn't change the shortcuts. This keeps it consistent with not having "Default" buttons for the options avoids possible confusion with the settings page changing things outside itself.

Some of these are a pain if SSS is to still support Firefox versions older than 66. It's either this or up the minimum required Firefox version again just for this fix. :) I'll think about it.

Cheers!

@CanisLupus
Copy link
Owner

CanisLupus commented Aug 31, 2022

Hey! I've updated SSS to version 3.48.0 (actually a few days ago). The shortcut options now direct you to use Firefox's menu to change shortcuts, which is the new way since Firefox 66 (aaaaages ago) :)

Now, when SSS loads, it won't override the shortcuts that you had set in that menu. SSS won't change shortcuts anymore, just respond to them. As a consequence, resetting settings and loading backups also won't touch the shortcuts (which is probably for the best).

Follow up from the previous post:

  • I bumped the minimum Firefox version to 66 from 63. Essentially, looking at the extremely tiny version usage stats I concluded it was not worth it to support versions 63 to 65.

Delete the shortcut settings from the saved data on read (they're not needed anymore).

  • This was not done in the end, just in case the option needs to be reverted for some reason.

Add info to the "Reset options" button saying it doesn't affect the shortcuts.

  • Also felt unnecessary since they are no longer "options" in any case.

Cheers!

@grahamperrin
Copy link
Author

Thank you!

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

2 participants