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

Add command to activate extension for Vivaldi #84

Merged
merged 1 commit into from Aug 16, 2023

Conversation

hdykokd
Copy link
Contributor

@hdykokd hdykokd commented Aug 15, 2023

In Vivaldi, there's an issue where the _execute_action of Manifest V3 cannot be executed.
See: https://forum.vivaldi.net/topic/75247/extensions-keyboard-shortcuts-don-t-work/22

This pull request offers a solution to that problem. Specifically, it defines a new shortcut command that performs actions equivalent to _execute_action.

This change is intended for Vivaldi users, and ideally, we wouldn't want to merge such browser-specific modifications into the main codebase. However, repeatedly building locally to keep up with the updates of Mouse Dictionary is cumbersome...

What are your thoughts on this?

- Modified manifest-chrome.json to include a new command `activate_extension`.
- Updated background.js to listen for the command and execute the main.js script.
@wtetsu
Copy link
Owner

wtetsu commented Aug 16, 2023

Thank you for your contribution!

I know this problem as it was reported in the past that shortcut keys do not work in Vivaldi, but I was surprised that this was discovered a long time ago and has not yet been fixed.

Maybe you are a Vivaldi user, what is your goal in this matter?

  • Is it enough to have a build option where activate_extension is added? (e.g. Prepare manifest-vivaldi.json. This is not published in Chrome Store)
  • Would you like it to be distributed in the Chrome Store with the activate_extension?

I'm a bit worried about the latter, if there are any strange side-effects.

@hdykokd
Copy link
Contributor Author

hdykokd commented Aug 16, 2023

The latter is ideal. However, as you mentioned, there might be potential side effects. I'm not familiar with the specifications of Chrome Extensions, so I don't have a concrete opinion on this.

While the former might be useful for other Vivaldi users, it's questionable whether it has enough merit to be merged into the master. It might be a good idea to fork the repository and make changes like the former.

@wtetsu wtetsu merged commit 9244ea8 into wtetsu:master Aug 16, 2023
10 of 12 checks passed
@wtetsu
Copy link
Owner

wtetsu commented Aug 16, 2023

I have merged it anyway, but...

Considering the user ratio, I think it's not worth the risk of potential side effects by doing something that is not relevant to the majority of users.
In the first place, since only Vivaldi's behavior is strange among Chromium-based browsers, I think Vivaldi side should fix it.


For that reason, I added build commands as a workaround.
Could you please try npm run build-vivaldi and it works in Vivaldi as expected?

I'm not confident about this code.
(Where does tab.id come from?)

@hdykokd
Copy link
Contributor Author

hdykokd commented Aug 16, 2023

In the first place, since only Vivaldi's behavior is strange among Chromium-based browsers, I think Vivaldi side should fix it.

I agree. This is an issue that Vivaldi should address.

Thank you for considering a solution.

(Where does tab.id come from?)

https://developer.chrome.com/docs/extensions/reference/commands/#event-onCommand
It seems to be passed as the second argument of the callback.
As far as I've tested it on Vivaldi, it seems to be working fine.

@wtetsu
Copy link
Owner

wtetsu commented Aug 17, 2023

@hdykokd

I tried in the env below, but it didn't work as expected.
Could you please try again in master branch with a build by npm run build-vivaldi?

  • 6.1.3035.302 (Stable channel) (64-bit)
  • Windows 10 Version 22H2 (Build 19045.3324)

@hdykokd
Copy link
Contributor Author

hdykokd commented Aug 17, 2023

@wtetsu
I apologize for the confusion. There was a missing change in my pull request. This line needs to be modified as follows:

chrome.commands.onCommand.addListener((command, tab) => {

@wtetsu
Copy link
Owner

wtetsu commented Aug 17, 2023

Thank you.
I made sure this code works (2986987)

@hdykokd
Copy link
Contributor Author

hdykokd commented Aug 17, 2023

I confirmed it works!. Thank you very much.

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

Successfully merging this pull request may close these issues.

None yet

2 participants