Skip to content

fix(editor): record undo snapshot on mouseup to preserve cursor position#351

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:develop/snipefrom
pengfeixx:fix/undo-cursor-position
Jun 1, 2026
Merged

fix(editor): record undo snapshot on mouseup to preserve cursor position#351
deepin-bot[bot] merged 1 commit into
linuxdeepin:develop/snipefrom
pengfeixx:fix/undo-cursor-position

Conversation

@pengfeixx
Copy link
Copy Markdown
Contributor

The undo system's lazy snapshot mechanism did not capture cursor position when user clicked to move it, causing cursor to jump to a stale position after Ctrl+Z.

修复编辑器撤销时光标跳转到错误位置的问题,在mouseup时记录快照
以保存正确的光标位置。

Log: 修复撤销时光标位置跳转错误
PMS: BUG-332099
Influence: 用户点击移动光标后输入内容再撤销,光标现在会正确地
保持在点击位置,而不是跳转到之前的旧位置。

The undo system's lazy snapshot mechanism did not capture cursor position
when user clicked to move it, causing cursor to jump to a stale position
after Ctrl+Z.

修复编辑器撤销时光标跳转到错误位置的问题,在mouseup时记录快照
以保存正确的光标位置。

Log: 修复撤销时光标位置跳转错误
PMS: BUG-332099
Influence: 用户点击移动光标后输入内容再撤销,光标现在会正确地
保持在点击位置,而不是跳转到之前的旧位置。
@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lzwind, pengfeixx

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 1, 2026

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "src/web/js/summernote_v9_2.js": [
        {
            "line": "    var KEY_BOGUS = 'bogus';",
            "line_number": 3933,
            "rule": "S106",
            "reason": "Var naming | 6d7db7168d"
        },
        {
            "line": "                    .attr('src', 'https://instagram.com/p/' + igMatch[1] + '/embed/')",
            "line_number": 6881,
            "rule": "S35",
            "reason": "Url link | dcb31e9ddf"
        },
        {
            "line": "                    .attr('src', 'http://v.qq.com/iframe/player.html?vid=' + vid + '&auto=0');",
            "line_number": 6918,
            "rule": "S35",
            "reason": "Url link | ce631f8419"
        }
    ]
}

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX,你的智能编程助手。我已仔细审查了你提供的 Git Diff 输入。这段代码主要是在 mouseup 事件触发且非右键时,增加了一次撤销历史记录(recordUndo)。

以下是我从语法逻辑、代码质量、代码性能和代码安全四个维度提出的详细审查意见与改进建议:

1. 语法与逻辑

  • 撤销记录过于频繁(核心逻辑问题)
    mouseup 事件在用户每次松开鼠标时都会触发,这包括但不限于:点击空白处、拖拽文本、点击按钮、双击选中文字等。如果在每次 mouseup 时都调用 recordUndo(),会导致撤销历史栈迅速膨胀。用户在进行一次正常的文本输入(keydown -> input -> keyup -> mouseup)时,可能会产生多条冗余的撤销记录,导致用户按 Ctrl+Z 时体验异常(可能需要按多次才能撤销一次完整的输入)。
    建议:通常富文本编辑器(如 Summernote)内部已经对输入事件(input)、按键事件(keydown/keyup)或内容变化做了快照记录。如果是为了记录“拖拽移动文本”或“鼠标选中后修改”等操作,建议在确认内容确实发生改变时再记录,或者利用编辑器自带的 change 事件去触发 recordUndo,而不是在 mouseup 时无差别记录。
  • 右键判断的局限性
    代码 if (event.which !== 3) 用于排除右键。但在现代浏览器中,event.which 已被废弃(Deprecated)。虽然目前仍然可用,但不够严谨。
    建议:使用 event.button 代替。鼠标右键的 event.button 值为 2。判断条件应改为 if (event.button !== 2)

2. 代码质量

  • 缺乏注释
    在事件监听中插入核心逻辑(记录撤销栈)是一个敏感操作,但缺乏注释说明为什么要在 mouseup 时记录。其他开发者后续维护时可能会疑惑此处是否冗余。
    建议:添加清晰的注释,说明此处补录 recordUndo 的业务意图(例如:“为了捕获鼠标拖拽文本产生的DOM变化,在mouseup时记录快照”)。
  • 上下文依赖
    _this.history.recordUndo() 依赖了外部的 _this 上下文。确保在 mouseup 触发时,_this.history 对象已经初始化且未被销毁,否则可能会抛出 Cannot read property 'recordUndo' of undefined 的异常。

3. 代码性能

  • 历史栈内存溢出风险
    如前所述,频繁的 recordUndo 会生成大量的历史状态快照。富文本的快照通常包含完整的 DOM 结构或复杂的 Delta 数据,内存占用较高。如果不加限制,长时间编辑会导致浏览器内存占用持续攀升,甚至页面卡顿。
    建议
    1. 确认 recordUndo 内部是否有历史栈长度限制(如最多保存 100 步)。如果没有,需要在外部或内部加上限制机制。
    2. 考虑防抖或脏检查机制:只有当编辑器内容与上一次 recordUndo 时的内容确实不同时,才执行记录。

4. 代码安全

  • XSS 与数据注入
    此处代码本身不直接涉及 XSS 漏洞,但 recordUndo 会保存当前 DOM 状态。如果攻击者通过 F12 控制台或浏览器插件在 DOM 中注入了恶意脚本节点,随后用户触发了 mouseup,该恶意节点就会被记录到撤销栈中。虽然这不是这段代码引入的直接漏洞,但在设计历史记录机制时需确保恢复快照时对 HTML 进行安全过滤(这通常由 Summernote 内核处理,此处仅作提醒)。
  • 原型链污染
    如果 history 对象的属性来自不可信的输入并被动态修改,调用 recordUndo 可能会引发意外行为。确保 history 对象的完整性。

💡 改进建议代码

基于以上分析,如果你确实需要在 mouseup 时记录撤销(例如为了捕获拖拽移动文本的操作),建议增加内容变化检查,并使用现代 DOM 事件标准:

}).on('mouseup', function (event) {
    // 使用现代标准 event.button 替代已废弃的 event.which (2 代表右键)
    if (event.button !== 2) {
        _this.context.triggerEvent('mouseup', event);
        
        // 记录撤销前保存当前内容快照,用于比对
        var currentContent = _this.editable.html();
        
        // 建议增加脏检查:只有内容确实发生变化时才记录撤销
        // 假设 _this.lastUndoSnapshot 是你维护的上一次记录时的内容
        if (currentContent !== _this.lastUndoSnapshot) {
            _this.history.recordUndo();
            _this.lastUndoSnapshot = currentContent; // 更新快照
        }
    }
}).on('scroll', function (event) {
    _this.context.triggerEvent('scroll', event);

最后确认:Summernote 原生代码中通常已经在 keydown 或通过 MutationObserver 对内容变更做了 recordUndo 处理。强烈建议你在合并这段代码前,测试原生的撤销功能是否已经能满足需求,避免重复记录导致用户体验下降。

@pengfeixx
Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot Bot merged commit 4fcdc89 into linuxdeepin:develop/snipe Jun 1, 2026
21 checks passed
@pengfeixx pengfeixx deleted the fix/undo-cursor-position branch June 1, 2026 01:47
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.

3 participants