[高优先] 重新修订 GM_registerMenuCommand 相关代码设计#790
Conversation
There was a problem hiding this comment.
Pull Request Overview
这个 PR 重新设计了 GM_registerMenuCommand 相关代码的架构,以提高与 Tampermonkey (TM) 的兼容性。重点解决了菜单项唯一性、显示更新以及跨页面/框架的同步问题。
主要变更:
- 重新设计菜单项的键值系统,使用稳定的 groupKey 来合并相同语义的菜单项
- 实现实时菜单更新机制,通过消息传递即时同步菜单状态变化
- 修正 Chrome contextMenu API 的稳定性问题,使用固定 UUID 池避免菜单 ID 冲突
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/store/AppContext.tsx | 改进消息订阅的类型安全性,添加泛型支持与清理优化 |
| src/pages/popup/App.tsx | 添加菜单实时更新监听机制,优化脚本列表排序逻辑 |
| src/pages/options/routes/Setting.tsx | 统一添加消息订阅清理逻辑,保持代码风格一致 |
| src/pages/options/routes/ScriptList.tsx | 为消息订阅添加类型注解,确保类型安全 |
| src/pages/components/ScriptMenuList/index.tsx | 重构菜单项分组逻辑,支持 groupKey 聚合与实时更新 |
| src/app/service/service_worker/types.ts | 定义新的菜单项类型结构,支持 groupKey 和唯一键系统 |
| src/app/service/service_worker/popup.ts | 核心菜单管理逻辑重写,实现稳定的 contextMenu 管理 |
| src/app/service/service_worker/gm_api.ts | 更新 GM API 参数类型,适配新的菜单键值系统 |
| src/app/service/service_worker/client.ts | 修改客户端接口以使用新的菜单键值参数 |
| src/app/service/queue.ts | 更新消息队列类型定义,支持新的菜单键值结构 |
| src/app/service/content/gm_api.ts | 重写内容脚本的菜单注册逻辑,支持环境隔离与唯一键 |
| src/app/cache.ts | 添加注释说明返回值的边界情况处理 |
Comments suppressed due to low confidence (1)
src/app/service/service_worker/popup.ts:1
- 这个注释实际上属于
src/app/cache.ts文件,而不是 popup.ts。注释位置错误可能会误导开发者。
import { type IMessageQueue } from "@Packages/message/message_queue";
| //@ts-ignore | ||
| b.enable - a.enable || |
There was a problem hiding this comment.
使用 //@ts-ignore 注释不是好的做法。应该明确处理类型问题,比如将 enable 转换为数字或使用条件表达式。
| //@ts-ignore | |
| b.enable - a.enable || | |
| Number(b.enable) - Number(a.enable) || |
| url = new URL(currentUrl); | ||
| } catch (_: any) { | ||
| // ignore error | ||
| // 容错:URL 解析失败时忽略错误(不影响后续 UI) |
There was a problem hiding this comment.
[nitpick] 注释中的"容错"应为"容错处理"或"错误处理",以更准确地表达意图。
| // 容错:URL 解析失败时忽略错误(不影响后续 UI) | |
| // 容错处理:URL 解析失败时忽略错误(不影响后续 UI) |
There was a problem hiding this comment.
Copilot跟ChatGPT有不一样的看法呢
| delete options.id; // id不直接储存在options (id 影响 groupKey 操作) | ||
| if (providedId === undefined) providedId = this.menuIdCounter! += 1; // 如无指定,使用累计器id | ||
| const ret = providedId; | ||
| // providedId = (typeof providedId === "number" ? `n${providedId}` : `t${providedId}`) as string; |
There was a problem hiding this comment.
有重复的注释掉的代码和相同的TODO注释。应该删除注释掉的代码,并考虑将TODO注释提取为统一的文档或issue。
| // providedId = (typeof providedId === "number" ? `n${providedId}` : `t${providedId}`) as string; | |
| this.sendMessage("GM_unregisterMenuCommand", [id]); | ||
| GM_unregisterMenuCommand(menuId: TScriptMenuItemID): void { | ||
| execEnvInit(this); | ||
| // let menuKey = (typeof menuId === "number" ? `n${menuId}` : `t${menuId}`) as string; |
There was a problem hiding this comment.
有重复的注释掉的代码和相同的TODO注释。应该删除注释掉的代码,并考虑将TODO注释提取为统一的文档或issue。
| // let menuKey = (typeof menuId === "number" ? `n${menuId}` : `t${menuId}`) as string; |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2b8cf7c to
f7750a1
Compare
GM_registerMenuCommand 相关代码设计GM_registerMenuCommand 相关代码设计
|
先记一下:未有考虑 ServiceWorker 热重启问题。 contextMenuIds 要储 chrome.storage.session 或固定ID? 再看看 |
64291a7 to
4f676f8
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
src/app/service/content/gm_api.ts:1
- 应该避免使用 @ts-ignore,建议使用适当的类型转换或修正类型定义以消除类型错误。
import type { Message, MessageConnect } from "@Packages/message/types";
src/app/service/content/gm_api.ts:1
- 应该避免使用 @ts-ignore,建议使用适当的类型转换或修正类型定义以消除类型错误。
import type { Message, MessageConnect } from "@Packages/message/types";
|
这一段注释是不是有问题?这个pr的结果依旧是 3&4,不过这个逻辑也可以 const p40 = GM_registerMenuCommand("MenuReg D-40", () => {
console.log(601);
});
const p50 = GM_registerMenuCommand("MenuReg D-50", () => {
console.log(602);
});
console.log("p40, p50", [p40, p50]); // TM gives 3&4, but SC should skip 3 and give 4 and 5 (or skip both 3&4)checkSubFrameIdSequence 我理解,其实就是不同id,但是同名的菜单的触发逻辑,都会触发一次 |
| // 内容脚本执行环境识别符,用于区分 mainframe / subframe 等环境并作为 menu key 的命名空间。 | ||
| // 由 execEnvInit() 以 randomMessageFlag() 生成,避免跨 frame 的 ID 碰撞。 | ||
| // (同一环境跨脚本也不一样) | ||
| contentEnvKey: string | undefined; |
There was a problem hiding this comment.
可以使用 runFlag ,不过另外再生成一个也没问题,语义上不同,只是提一下有 runFlag 也可以用于区分不同的环境/脚本
| }); | ||
| unlock(); | ||
| return newValue!; | ||
| return newValue!; // 必须注意当 value 为 「undefined, null, "", 0」 时,newValue 是 undefined |
|
|
||
| // 标记需要同步后,若成功写回快取,再触发实际菜单重建(避免多次小变更重复重建)。 | ||
| async onMenuCommandsChanged(tabId: number, uuid: string) { | ||
| menuRegisterNeedUpdate = true; |
There was a problem hiding this comment.
menuRegisterNeedUpdate 的设计感觉意义不大, onMenuCommandsChanged -> syncMenuCommandsToSessionStore 是顺序的,也没有另外的地方有调用
| let menus = menuRegister.get(mrKey) as ScriptMenuItem[]; | ||
| if (!menus) { | ||
| menus = [] as ScriptMenuItem[]; | ||
| menuRegister.set(mrKey, menus); |
There was a problem hiding this comment.
menuRegister的设计也有一些多余,没有地方有效用到,可读性不佳,而且如果service_worker重启,会丢失信息
There was a problem hiding this comment.
我明白了,如果并发的话,会有问题,会推送多次菜单到popup,导致闪动
There was a problem hiding this comment.
// GM_registerMenuCommand 是同步函数。
// 所以流程是:先在 popup.ts 即时更新「无锁的记录」,再回写到「有锁记录」交给 Popup/App.tsx。
// 这样可以避免新增/删除操作的次序冲突。
// 外部(popup.tsx)只会读取 menu items,不会直接修改(若要修改,必须透过 popup.ts)。
這裡有寫
我最初想法是,应该要跳过id, 但之后再想了一下,跟TM就好,简单。实际使用问题是开发人员的问题
TM的做法就是把相同按钮,不论是同一frame 还是不同frame, 都合并在一个 |
…1/scriptcat into pr/cyfung1031/790
|
79ac078 的 popup.ts 改動要退回 |
你这个写法用了 unregisterMenuCommandMessages 和 registerMenuCommandMessages
我不坚持原案了 |
次序(顺序?)应该是没问题的 性能可以考虑在添加队列后使用 await Promise / setTimeout 将线程所有权抛出,然后再判断一下长度是否为0,来减少后续的储存操作 |
|
发现了一个问题,我使用 沉浸式翻译 ,他会注册两个同样的菜单,然后点击的时候,重复触发了事件,导致点击菜单没有翻译效果,一个简单的示例: // ==UserScript==
// @name test menu
// @namespace https://docs.scriptcat.org/
// @version 0.1.0
// @description try to take over the world!
// @author You
// @match https://github.com/CodFrm
// @icon https://www.google.com/s2/favicons?sz=64&domain=github.com
// @grant GM_registerMenuCommand
// ==/UserScript==
for (let i = 0; i < 2; i++) {
GM_registerMenuCommand("菜单", () => {
console.log("应该只输出一次");
})
}TM也是如此,但是直觉感觉不应该如此,同名菜单只触发一次似乎更合理一点,如果多iframe,就多iframe中触发,每个iframe中触发一次 |
你是指 吧? TM沒有錯 |

概述
close #787 關連 #580
现有的设计有问题。在以下脚本可验证。
PR后,脚本的执行会跟TM完全一致。
问题有好几个,难以一一说明。你跑下面脚本看看就知道。
脚本中有三个可改动设定
checkSubFrameIdSequence:网页有iframe的话,改为true会改为检查子母frame菜单显示问题。intervalChanging:预设false. 改成true后会进行每秒修改选单项目。skipClickCheck: 预设中,脚本会一步一步引导用户执行及检查脚本运作是否跟TM一致。true的话不进行。全false的话是检查最基本的项目。请在没有 iframe 的简单页面进行。打开 DevTools 即可理解要怎么测试。
然后改 checkSubFrameIdSequence 为 true, 查看一下 top window 和 iframe window 同时执行的情况。点一下菜单项目吧。
然后改 intervalChanging 为 true. 先在 没有 iframe 的简单页面 看一下。旧版本的话,popup菜单显示没有即时更新,要关一下开一下才行。这跟TM不一样。TM是即时更新。
然后右键点一下看看。我发现TM的右键菜单很正常但SC的右键菜单总是不显示 interval-n-xxxx
最终找到的解决方法是,浏览器contextMenu的项目id需要穏定。穏定后就解决问题了
最后是在有 iframe 的以 intervalChanging true 跑一下。 因为top window 跟iframe window的时间有差异,所以菜单会一时四个一时两个 ( interval-m-1001 跟 interval-m-1002 不合并)。这个在PR后也是跟TM一致了
这个PR把这些复杂的问题都全部解决了,完全跟TM一致
代码部份尽量加注释避免你看不懂。
变更内容
重新修订
GM_registerMenuCommand相关代码设计截图