userscripts icon indicating copy to clipboard operation
userscripts copied to clipboard

Feature request: GM.registerMenuCommand

Open iFelix18 opened this issue 3 years ago • 13 comments

API to register a command to be displayed in the Userscripts pop-up menu.

Reference: Greasemonkey, Violentmonkey, and Tampermonkey.

iFelix18 avatar Feb 16 '22 23:02 iFelix18

Thanks for the feature request @iFelix18 - I'll consider this, but there's a long list of updates already in queue.

For the time being you might want to consider @run-at content-menu - I don't think this is documented in the readme, but that will add a new context menu (right click) item. Sometimes you need to right click twice to get it show.

Obviously, this is desktop only.

// ==UserScript==
// @name        Context Test
// @description This is your new file, start writing code
// @include        *
// @run-at context-menu
// @inject-into auto
// ==/UserScript==

alert("context test 1");

quoid avatar Feb 17 '22 00:02 quoid

The run-at context menu option works awesome. Thanks.

m-thomson avatar Feb 13 '23 23:02 m-thomson

Is this being worked on actively? I've got a draft implementation here, if you're interested: https://github.com/gormster/userscripts/tree/issue/230

gormster avatar Jan 03 '24 07:01 gormster

@gormster Yes, the issue is on the task list, I think there's a refactoring there regarding scripts uuid and injection process and popup loading process as a prerequisite. So I haven't started this work yet.

Thanks for trying to implement this feat, I did a cursory review and as I mentioned in the comment, this was the first issue I noticed. My previous thought was to store that temporary data in the content script, as well as other related data, such as @resource, I think an updated integrated design would be a lot neater.

We could discuss more if you're interested.

ACTCD avatar Jan 08 '24 18:01 ACTCD

Ah, didn't realise that, but I did think about it when I noticed there were no other JS variables being used in the background script…

Storing the data entirely in the content script would definitely be neater… I'm trying to remember why I didn't implement it that way. I definitely tried! I think it was mainly because originally, the popup page only talks to the background script, which only talks to the native extension process. I don't think it ever talks directly to the content script?

Except, of course, now it does, to actually execute the registered commands… so yeah, can definitely simplify this by just asking the tab for the list of commands instead of the background page. That would be a lot cleaner!

(Aside: do we care about refreshing the popup page when a new command is registered? It can, hypothetically, happen at any time, though I doubt many user scripts are adding commands at any time besides their initial run.)

gormster avatar Jan 09 '24 00:01 gormster

done! that was actually fairly straightforward, and got rid of a ton of complexity. hooray!

gormster avatar Jan 09 '24 01:01 gormster

@gormster Good quick work! I will leave a new comment when I review again.

But I can answer couple questions first (that you probably already know)

originally, the popup page only talks to the background script, which only talks to the native extension process. I don't think it ever talks directly to the content script?

Well, popup page have no different from background pages. They all are privileged pages that can communicate with native layer or content scripts.

do we care about refreshing the popup page when a new command is registered?

I think popup should only load the currently registered items, not update them anytime after opening, that brings unnecessary re-rendering.

ACTCD avatar Jan 09 '24 10:01 ACTCD

@gormster I reviewed your implementation again and it looks mostly fine.

But at the same time I also see some of minor issues, and I'm sorry I don't have time to point them all out at the moment. I just briefly tried to build your branch, looks like it's working as expected. but it looks like the UI could use some improvement, but that may need to be combined with another popup refactoring effort (#447).

I'm working full steam ahead on another refactoring effort regarding the settings UI. So I can't think too much about it at the moment. I might prefer to consider these issues carefully when doing related refactoring designs later.

Again, I'm sorry I didn't provide much useful new information. And thank you for your efforts on this issue.

ACTCD avatar Jan 10 '24 00:01 ACTCD

Aside: do we care about refreshing the popup page when a new command is registered? It can, hypothetically, happen at any time, though I doubt many user scripts are adding commands at any time besides their initial run.

Some scripts might be adding commands asynchronously or, if they are used on a SPA, might add them on demand depending on the current page. (Violentmonkey even has GM.unregisterMenuCommand)

HimbeersaftLP avatar Jan 10 '24 08:01 HimbeersaftLP

@HimbeersaftLP I think any changes (register or unregister) should take effect the next time the user opens the popup, rather than dynamically re-rendering the current popup view, which would cause unnecessary flickering or accidental clicks by the user.

ACTCD avatar Jan 10 '24 08:01 ACTCD

@ACTCD Ohh, I misunderstood that part, yeah, that is probably fine, unless some weird Userscript tries to make a menu navigation with menucommands lol

HimbeersaftLP avatar Jan 10 '24 13:01 HimbeersaftLP