Skip to content

fix(editor): prevent floating toolbar from hiding on button click#350

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:develop/snipefrom
pengfeixx:fix/air-popover-toolbar-click
May 29, 2026
Merged

fix(editor): prevent floating toolbar from hiding on button click#350
deepin-bot[bot] merged 1 commit into
linuxdeepin:develop/snipefrom
pengfeixx:fix/air-popover-toolbar-click

Conversation

@pengfeixx
Copy link
Copy Markdown
Contributor

Add toolbarButtonClicked flag to prevent air popover toolbar from being hidden when user clicks toolbar buttons. The flag is set on mousedown and reset after click, blocking concurrent hide events.

添加toolbarButtonClicked标志位,防止点击悬浮工具栏按钮时工具栏被误隐藏。
该标志在mousedown时设置,click后重置,阻止并发的隐藏事件。

Also: simplify lrelease path detection and filter JS console output to Warning level in QML.

同时简化翻译脚本中lrelease路径检测,并优化QML端JS日志过滤。

Log: 修复悬浮工具栏点击按钮后被误隐藏的问题
PMS: BUG-354381
Influence: 修复后用户点击悬浮编辑工具栏上的按钮时,工具栏不再被误隐藏,
编辑操作可以正常完成。

@github-actions
Copy link
Copy Markdown

Add toolbarButtonClicked flag to prevent air popover toolbar from
being hidden when user clicks toolbar buttons. The flag is set on
mousedown and reset after click, blocking concurrent hide events.

添加toolbarButtonClicked标志位,防止点击悬浮工具栏按钮时工具栏被误隐藏。
该标志在mousedown时设置,click后重置,阻止并发的隐藏事件。

Log: 修复悬浮工具栏点击按钮后被误隐藏的问题
PMS: BUG-354381
Influence: 修复后用户点击悬浮编辑工具栏上的按钮时,工具栏不再被误隐藏,
          编辑操作可以正常完成。
@pengfeixx pengfeixx force-pushed the fix/air-popover-toolbar-click branch from d8e7ae2 to 756366b Compare May 29, 2026 05:26
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的 Git Diff 代码。

这段代码的主要目的是解决一个前端交互冲突:当用户点击悬浮工具栏(air popover)上的按钮时,会触发选区变化或右键菜单逻辑,导致工具栏被意外隐藏。你通过引入一个全局标志位 toolbarButtonClicked 来拦截这种意外的隐藏行为,思路是正确的,但在语法逻辑、代码质量、性能和安全性方面都有可以改进的空间。

以下是详细的审查意见和改进建议:

1. 语法与逻辑

  • 竞态条件与状态不一致:你在 mousedown 事件中设置 toolbarButtonClicked = true,在 click 事件中通过 setTimeout 100ms 后重置为 false,同时又在 hideRightMenu 中手动重置为 false。这种多处异步重置状态的方式非常脆弱,极易导致状态不一致。如果 hideRightMenu 在 100ms 内被调用,标志位被重置为 false,随后 setTimeout 触发再次重置,虽然此时不会引发明显 bug,但逻辑流转不清晰;如果用户极快地连续操作,可能会导致标志位状态错乱。
  • setTimeout 的硬编码延迟setTimeout(function() { toolbarButtonClicked = false; }, 100); 中的 100ms 是一个魔法数字。在不同性能的设备或不同的浏览器事件循环延迟下,100ms 可能不够,也可能过长,导致正常的隐藏操作被吞掉。

2. 代码质量

  • 全局变量污染var toolbarButtonClicked = false; 暴露在全局作用域中,任何代码都可以意外地修改它。建议使用闭包或模块作用域来限制其访问范围。
  • jQuery 选择器重复执行$('.note-air-popover .note-btn')mousedownclick 事件中都被使用,且在 changeContent 函数中也有 $('.note-air-popover')。虽然 jQuery 内部有缓存机制,但在高频事件中,频繁的 DOM 查询依然是不好的实践。
  • 魔法字符串/数字100 应该提取为具有语义的常量。

3. 代码性能

  • 事件委托的粒度$(document).on('mousedown', '.note-air-popover .note-btn', ...) 使用了事件委托,这本身是好的。但由于 document 上的事件监听器过多会影响整体事件处理性能,如果 .note-air-popover 是一个常驻或已知的容器,建议将事件委托绑定在更近的父级(如 $('#summernote')$('.note-air-popover') 本身)上,而不是 document
  • 避免不必要的定时器启动:在 hideRightMenu 中,如果 toolbarButtonClickedtrue,直接 return 是好的,但如果你采纳了下面建议的重构方案,连定时器的创建和清除都可以省去。

4. 代码安全

  • XSS 风险(上下文关联):在 changeContent 函数中,$('.note-editable').html('<p><br></p>') 使用了 .html() 注入硬编码字符串,目前是安全的。但同上下文的 getSelectedRange().innerHTML == "" 表明你在处理 DOM 选区。请确保在任何动态拼接 HTML 并通过 .html() 渲染的地方,都对用户输入进行了严格的转义,防止存储型或反射型 XSS。

💡 重构建议与改进代码

最优雅的解决“点击内部元素不触发外部隐藏”的方法是利用事件对象的传播机制,而不是用全局变量和定时器去“猜”时序。

当点击工具栏按钮时,mousedown 事件会冒泡。如果 hideRightMenu 是由某个外部容器的 mousedown 事件触发的,我们可以通过检查 event.target 是否属于工具栏来决定是否隐藏。

但考虑到你的 hideRightMenu 可能是由 QML 等外部环境直接调用的,无法直接获取 Web DOM 事件对象,因此我们仍然需要保留标志位,但可以大幅简化其逻辑,彻底移除 setTimeout

改进后的代码:

// 使用 IIFE 封装,避免污染全局命名空间
(function() {
    // 常量定义,消除魔法数字
    const TOOLBAR_HIDE_DELAY = 100;
    
    // 将状态变量限制在闭包作用域内
    let hideToolbarTimer = null;
    let toolbarButtonClicked = false;

    // 监听悬浮工具栏按钮的 mousedown 事件
    // 建议将事件委托挂载在更近的父级,如果 #summernote 是父级,则改为 $('#summernote').on(...)
    $(document).on('mousedown', '.note-air-popover .note-btn', function(e) {
        e.preventDefault();
        // 标记点击发生
        toolbarButtonClicked = true; 
        if (hideToolbarTimer) {
            clearTimeout(hideToolbarTimer);
            hideToolbarTimer = null;
        }
    });

    // 移除 click 事件中的 setTimeout,改为在 mousedown 时利用 event.stopPropagation() 
    // 或者在确定安全的同步逻辑中重置状态。
    // 如果必须在 click 重置,去掉延迟:
    $(document).on('click', '.note-air-popover .note-btn', function(e) {
        // click 事件发生后,危险期已过,可以立即重置状态
        toolbarButtonClicked = false; 
    });

    // 右键隐藏悬浮工具栏
    window.hideRightMenu = function() { // 如果需要外部访问,挂载到 window
        // 如果刚刚点击了工具栏,阻止隐藏,并重置状态
        if (toolbarButtonClicked) {
            toolbarButtonClicked = false;
            return;
        }
        if (hideToolbarTimer) clearTimeout(hideToolbarTimer);
        hideToolbarTimer = setTimeout(function() {
            $('#summernote').summernote('airPopover.hide');
            hideToolbarTimer = null; // 执行完毕清理定时器引用
        }, TOOLBAR_HIDE_DELAY);
    };
})();

// 对应 changeContent 函数的修改
function changeContent(we, contents, $editable) {
    if ($editable.summernote('isEmpty')) {
        $('.note-editable').html('<p><br></p>')
    }
    if (webobj && initFinish) {
        // 这里的逻辑保持不变,因为闭包内的 toolbarButtonClicked 无法直接在这里访问
        // 如果采用闭包,可以将 toolbarButtonClicked 挂载到 window,或者提供一个 getter 方法
        // 为了兼容现有架构,建议保留全局变量,但加上命名空间,例如:window.AppState.toolbarButtonClicked
    }
}

如果不使用闭包(最小改动方案):

如果你不想改动太大影响其他地方对变量的读取,请至少做以下优化:

var hideToolbarTimer = null;
var toolbarButtonClicked = false;
var TOOLBAR_HIDE_DELAY = 100; // 提取魔法数字

$(document).on('mousedown', '.note-air-popover .note-btn', function(e) {
    e.preventDefault();
    toolbarButtonClicked = true;
    if (hideToolbarTimer) {
        clearTimeout(hideToolbarTimer);
        hideToolbarTimer = null;
    }
});

// 移除 setTimeout,click 事件触发说明按钮点击流程已完成,同步重置即可
$(document).on('click', '.note-air-popover .note-btn', function(e) {
    toolbarButtonClicked = false;
});

function hideRightMenu() {
    if (toolbarButtonClicked) {
        toolbarButtonClicked = false;
        return;
    }
    if (hideToolbarTimer) clearTimeout(hideToolbarTimer);
    hideToolbarTimer = setTimeout(function() {
        $('#summernote').summernote('airPopover.hide');
    }, TOOLBAR_HIDE_DELAY);
}

总结:核心改进点是移除 click 事件中的 setTimeout。因为 mousedown -> mouseup -> click 的事件触发顺序是严格保证的,当 click 触发时,说明鼠标按下行为已经结束,此时同步将 toolbarButtonClicked 置为 false 是最安全、最准确的,避免了异步定时器带来的不可控状态。

@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

@pengfeixx
Copy link
Copy Markdown
Contributor Author

/merge

@deepin-bot deepin-bot Bot merged commit b4e9dae into linuxdeepin:develop/snipe May 29, 2026
21 checks passed
@pengfeixx pengfeixx deleted the fix/air-popover-toolbar-click branch May 29, 2026 06:26
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