Skip to content

fix: collapsed cursor cannot properly toggle underline/strikethrough …#354

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
LiHua000:master
Jun 2, 2026
Merged

fix: collapsed cursor cannot properly toggle underline/strikethrough …#354
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
LiHua000:master

Conversation

@LiHua000
Copy link
Copy Markdown
Contributor

@LiHua000 LiHua000 commented Jun 2, 2026

…in nested formatting

When cursor is collapsed inside nested tags like text|,
clicking underline/strikethrough buttons has no effect because execCommand
relies on browser internal state which is unreliable for nested inline formats.

  • Detect toggle direction via queryCommandState before DOM operations
  • Move cursor out of format tag (or remove empty tag) instead of relying on execCommand
  • Insert zero-width space anchor to prevent cursor being absorbed back into the tag
  • Record both format states before DOM changes, restore if execCommand flips the other
  • Use DOM ancestor check as primary source for button active state detection

log: fix bug

Bug: https://pms.uniontech.com/bug-view-354099.html

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

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

Copy link
Copy Markdown

@max-lvs max-lvs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

优先级 问题 文件
1 严重 零宽空格锚点字符永久残留在 DOM 中
2 严重 空标签移除后光标定位可能越界

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

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

…in nested formatting

When cursor is collapsed inside nested tags like text|,
  clicking underline/strikethrough buttons has no effect because execCommand
  relies on browser internal state which is unreliable for nested inline formats.
  - Detect toggle direction via queryCommandState before DOM operations
  - Move cursor out of format tag (or remove empty tag) instead of relying on execCommand
  - Insert zero-width space anchor to prevent cursor being absorbed back into the tag
  - Record both format states before DOM changes, restore if execCommand flips the other
  - Use DOM ancestor check as primary source for button active state detection

log: fix bug

Bug: https://pms.uniontech.com/bug-view-354099.html
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

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

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff代码。本次修改主要解决了富文本编辑器中两个经典痛点:零宽空格(ZWS)的清理折叠光标下下划线/删除线切换状态异常的问题

整体思路上,针对富文本编辑器的这些底层Bug,你的方向是正确的。不过,在语法逻辑、代码性能和安全性方面,还有不少可以优化和改进的地方。以下是详细的审查意见:


一、 第一个文件:assets/web/index.js (零宽空格清理)

1. 语法与逻辑

  • filter 缺少返回值:jQuery的.filter()函数需要返回一个布尔值来决定是否保留元素。当前代码只在条件成立时处理了节点,但没有return true,这会导致jQuery无法将符合条件的节点传递给后续链式调用(虽然这里没有后续调用,但逻辑不严谨)。如果只是为了遍历,应该使用.each()
  • 正则表达式对象复用:在循环/迭代的闭包中重复创建正则表达式 /​/g 会带来轻微的性能损耗,应该提取到循环外部。

2. 代码性能

  • DOM操作优化$cloneCode.find('*').addBack().contents() 会获取克隆代码片段中的所有节点,包括元素节点和文本节点。对于大型文档,这会创建一个庞大的jQuery对象。
  • 改进建议:可以使用原生的 TreeWalker API,它专门用于高效遍历DOM树,并且可以直接过滤出文本节点,性能远优于jQuery的 find + contents

3. 代码安全

  • XSS防范:由于是对 $cloneCode 进行操作,且最后取的是 innerHTML,只要原始输入经过消毒,这里是安全的。但清理空节点时,this.parentNode.removeChild(this) 需确保 this.parentNode 不为空(在极端DOM结构被破坏的情况下)。

优化后的代码建议:

// 清理格式切换产生的零宽空格锚点
var zwsRegex = //g;
var walker = document.createTreeWalker($cloneCode[0], NodeFilter.SHOW_TEXT, null, false);
var node;
var nodesToRemove = [];

while (node = walker.nextNode()) {
    if (zwsRegex.test(node.textContent)) {
        node.textContent = node.textContent.replace(zwsRegex, '');
        if (node.textContent === '') {
            nodesToRemove.push(node);
        }
    }
}

// 统一移除空节点,避免在遍历中修改DOM引发问题
nodesToRemove.forEach(function(emptyNode) {
    if (emptyNode.parentNode) {
        emptyNode.parentNode.removeChild(emptyNode);
    }
});

二、 第二个文件:assets/web/js/summernote_v9_2.js (格式状态与切换)

这部分修改非常复杂,涉及富文本编辑器最棘手的光标折叠状态下的格式拆分问题。

1. 语法与逻辑

  • 光标移动逻辑存在漏洞:在 wantOff 分支中,当 formatEl.textContent.trim() !== '' 时,你插入了ZWS并将光标移到ZWS之后。但是,你只移动了光标,并没有真正把光标后的文本从 <u><strike> 标签中拆分出来!
    • 现象:用户在 <u>文本|</u> 中按下取消下划线,光标虽然移到了标签外,但后续输入的文字依然在下划线标签内,因为 formatEl 并未被拆分(splitText 或 DOM 拆分)。
    • 正确做法:应该利用 Range.setEndAfter(formatEl) 并执行 range.extractContents(),或者使用 document.execCommand('unlink', false, null) 的类似思路来剥离格式。如果一定要用ZWS占位,必须将原节点拆分。
  • 状态恢复逻辑过于脆弱
    try { var curState = document.queryCommandState(sCmd); if (curState) document.execCommand(sCmd, false); } catch (e) {}
    这种“先查状态再反向执行”的逻辑在富文本中极不可靠。queryCommandState 往往无法准确反映当前光标所在的具体嵌套标签状态,容易导致误杀其他格式。

2. 代码性能

  • 循环向上查找DOM:你在两处使用了 while 循环向上查找 USTRIKE 标签。这部分逻辑是合理的,性能开销也可接受。但可以使用 jQuery 的 .closest() 方法来简化代码(如果当前上下文允许使用 $$$1)。
    var checkNode = node.nodeType === 3 ? node.parentNode : node;
    var $formatEl = $$1(checkNode).closest('u, strike, s');
    var hasUnderline = $formatEl.filter('u').length > 0;
    var hasStrikethrough = $formatEl.filter('strike, s').length > 0;

3. 代码安全

  • 异常捕获过于宽泛:代码中大量使用了空的 catch (e) {}。在富文本操作特别是 Range 操作时,如果发生异常,静默吞掉会导致编辑器状态死锁或光标消失。至少应该 console.warn 输出异常,便于排查。
  • Range 操作越界nr.setStart(parent, parent.childNodes.length) 在某些旧版浏览器中可能抛出 IndexSizeError,虽然你用了 try-catch,但仍需注意。

针对 strikethrough / underline 逻辑的重构建议:

核心问题是如何在折叠光标下正确"跳出"格式标签。更稳健的做法是拆分DOM节点,而不是仅仅移动光标:

if (range.collapsed) {
    var wantOff = false;
    try { wantOff = document.queryCommandState(sCmd); } catch (e) {}

    if (wantOff) {
        var formatTag = sCmd === 'underline' ? 'U' : 'STRIKE';
        var container = range.startContainer;
        var checkNode = container.nodeType === 3 ? container.parentNode : container;
        var formatEl = null;
        
        // 查找格式标签
        while (checkNode && checkNode.nodeType === 1) {
            if (checkNode.tagName === formatTag || (sCmd === 'strikethrough' && (checkNode.tagName === 'STRIKE' || checkNode.tagName === 'S'))) {
                formatEl = checkNode;
                break;
            }
            if ($$1(checkNode).hasClass('note-editable')) break;
            checkNode = checkNode.parentNode;
        }

        if (formatEl) {
            // 关键修复:需要将当前节点拆分,而不是仅移动光标
            // 如果标签内只有光标没有实质文本,直接移除空标签
            if (formatEl.textContent.trim() === '') {
                var parent = formatEl.parentNode;
                // 插入一个占位符(防止光标丢失)
                var placeholder = document.createTextNode('\u200B'); 
                parent.insertBefore(placeholder, formatEl);
                parent.removeChild(formatEl);
                
                // 重置光标到占位符
                var nr = document.createRange();
                nr.setStart(placeholder, 1);
                nr.collapse(true);
                sel.removeAllRanges();
                sel.addRange(nr);
            } else {
                // 标签内有内容:使用原生的 unlink 思路,或者手动拆分节点
                // 方案A(推荐,利用浏览器能力):将光标移到标签外,插入ZWS,并断开DOM连接
                var zws = document.createTextNode('\u200B');
                formatEl.parentNode.insertBefore(zws, formatEl.nextSibling);
                
                var nr = document.createRange();
                nr.setStart(zws, 1);
                nr.collapse(true);
                sel.removeAllRanges();
                sel.addRange(nr);
                
                // 注意:此时虽然光标在外,但如果不拆分formatEl,下次输入可能仍被包裹
                // 可以考虑选中ZWS并执行一次 execCommand(sCmd) 来强制在新位置应用反向格式
            }
            return; // 阻止后续默认的 execCommand 逻辑,因为已经手动处理了DOM
        }
    }
}
// 默认逻辑:非折叠光标,或者不需要 toggle off,直接执行命令
document.execCommand('styleWithCSS', false, false);
document.execCommand(sCmd, false);

总结

  1. 文件1:建议将 jQuery 的 filter 替换为原生的 TreeWalker,提升性能并修复逻辑漏洞。
  2. 文件2:向上遍历检查状态(hasUnderline等)的逻辑很好,但折叠光标下的 Toggle OFF 逻辑存在严重缺陷(未真正拆分DOM节点,导致后续输入仍带格式),且状态恢复逻辑(先查后反向执行)不可靠。建议重构为直接操作DOM拆分节点的方式,并移除脆弱的 queryCommandState 状态恢复代码。
  3. 全局:请勿滥用空的 catch(e) {},这会给线上问题排查带来极大困难,至少保留 console.error

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, max-lvs

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

@LiHua000
Copy link
Copy Markdown
Contributor Author

LiHua000 commented Jun 2, 2026

/merge

@deepin-bot deepin-bot Bot merged commit 8a00dc8 into linuxdeepin:master Jun 2, 2026
18 checks passed
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