Skip to content

fix(dock): delay plugin resident status until drag release from quick…#1607

Open
Ivy233 wants to merge 1 commit into
linuxdeepin:masterfrom
Ivy233:fix/tray-plugin-drag-resident-timing
Open

fix(dock): delay plugin resident status until drag release from quick…#1607
Ivy233 wants to merge 1 commit into
linuxdeepin:masterfrom
Ivy233:fix/tray-plugin-drag-resident-timing

Conversation

@Ivy233
Copy link
Copy Markdown
Contributor

@Ivy233 Ivy233 commented May 26, 2026

… panel

  1. Use staging mechanism for plugins dragged from quick panel to tray
  2. Only commit visibility change when mouse is released, not during drag
  3. Preserve original behavior for plugins dragged within tray itself
  4. Remove unnecessary setSurfaceVisible call in onExited handler

Log: Fix plugins being marked as resident during drag instead of on drop

Influence:

  1. Verify plugins from quick panel are not marked resident until drop
  2. Verify tray-internal plugin dragging still works correctly

fix(dock): 延迟插件驻留状态直到从快捷中心拖拽松手

  1. 对从快捷中心拖拽到托盘的插件使用暂存机制
  2. 仅在松手时提交可见性变更,拖拽过程中不触发
  3. 保留托盘内部插件拖拽的原有行为
  4. 移除 onExited 处理器中不必要的 setSurfaceVisible 调用

Log: 修复插件在拖拽过程中被标记为驻留而非松手时标记的问题

Influence:

  1. 验证从快捷中心拖拽的插件在松手前不会被标记为驻留
  2. 验证托盘内部插件拖拽仍然正常工作

PMS: BUG-361185

@Ivy233 Ivy233 requested review from 18202781743 and BLumia May 26, 2026 05:27
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Sorry @Ivy233, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@Ivy233 Ivy233 force-pushed the fix/tray-plugin-drag-resident-timing branch 2 times, most recently from 8a42367 to 02bc92c Compare May 26, 2026 12:54
Comment thread panels/dock/tray/package/TrayContainer.qml Outdated
@Ivy233 Ivy233 force-pushed the fix/tray-plugin-drag-resident-timing branch 2 times, most recently from e420b79 to 7b9e530 Compare May 26, 2026 13:31
Comment thread panels/dock/tray/package/TrayContainer.qml Outdated
… panel

1. Use staging mechanism for plugins dragged from quick panel to tray
2. Only commit visibility change when mouse is released, not during drag
3. Preserve original behavior for plugins dragged within tray itself
4. Remove unnecessary setSurfaceVisible call in onExited handler

Log: Fix plugins being marked as resident during drag instead of on drop

Influence:
1. Verify plugins from quick panel are not marked resident until drop
2. Verify tray-internal plugin dragging still works correctly

fix(dock): 延迟插件驻留状态直到从快捷中心拖拽松手

1. 对从快捷中心拖拽到托盘的插件使用暂存机制
2. 仅在松手时提交可见性变更,拖拽过程中不触发
3. 保留托盘内部插件拖拽的原有行为
4. 移除 onExited 处理器中不必要的 setSurfaceVisible 调用

Log: 修复插件在拖拽过程中被标记为驻留而非松手时标记的问题

Influence:
1. 验证从快捷中心拖拽的插件在松手前不会被标记为驻留
2. 验证托盘内部插件拖拽仍然正常工作

PMS: BUG-361185
@Ivy233 Ivy233 force-pushed the fix/tray-plugin-drag-resident-timing branch from 7b9e530 to 62aacec Compare May 27, 2026 01:38
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次修改主要是对托盘拖放逻辑的重构,将快捷面板(quickPanel)的拖放行为与收纳区(isStash)统一,采用“暂存机制”,并移除了拖出时针对快捷面板的特定清理逻辑。

以下是针对语法逻辑、代码质量、代码性能和代码安全方面的审查意见与改进建议:

1. 语法与逻辑

  • 潜在逻辑漏洞:source 变量未定义或未传递
    onDropped 事件中,新增了 source === "quickPanel" 的判断,但在该函数的上下文中,并没有看到 source 的定义或从 dropEvent 中提取该值的代码。如果 source 未定义,该条件将始终为 false,导致快捷面板的拖放无法走暂存提交逻辑。
    • 建议:确保 source 已正确声明并赋值。如果它是从拖放事件中传递的,应该类似于 isStash 的获取方式,例如:let source = dropEvent.getDataAsString("text/x-dde-shell-tray-dnd-source")
  • 逻辑一致性:onEnteredonDropped 的条件匹配
    onEntered 中,条件为 shouldAllowDrop && (isStash || source === "quickPanel");而在 onDropped 中,条件仅为 isStash || source === "quickPanel"。虽然 onDropped 触发时通常意味着已经允许放置,但为了代码的严谨性和对称性,建议保持逻辑判断的一致性,或在 onDropped 中也加上对 shouldAllowDrop 的检查(如果业务上存在不允许放置但仍触发 dropped 的情况)。
  • 移除 onExited 逻辑的副作用
    原代码在 onExited 中有这段逻辑:
    if (source !== "" && !isDropped) {
        dropTrayTimer.stop()
        DDT.TraySortOrderModel.setSurfaceVisible(surfaceId, false)
    }
    作用是:当从快捷面板拖入托盘但未放下就退出时,停止定时器并将该图标隐藏。现在这段逻辑被删除了。
    • 风险:如果用户从快捷面板拖拽图标进入托盘区域(此时可能触发了某些预览或中间状态),然后又拖出托盘区域取消操作,原代码会负责清理状态(停止计时器、恢复不可见)。删除后,需确认是否有其他机制(如 onCanceled 或 QML 的 drag 组件自身)接管了这部分清理工作,否则可能导致图标状态异常(如:拖拽取消后图标在托盘区幽灵显示,或定时器未停止导致意外插入)。

2. 代码质量

  • 硬编码的魔术字符串
    "quickPanel""text/x-dde-shell-tray-dnd-sectionType" 等属于硬编码的字符串。在 QML 和 JS 混合开发中,拼写错误很难在编译期被发现。
    • 建议:将这些字符串提取为常量,或者统一放在一个配置对象中。例如:
      const DND_SOURCE_QUICK_PANEL = "quickPanel"
      const DND_SECTION_TYPE_STASHED = "text/x-dde-shell-tray-dnd-sectionType"
      // 使用时:
      let isStash = dropEvent.getDataAsString(DND_SECTION_TYPE_STASHED) === "stashed"
  • 遗留的调试代码
    onDropped 中存在 console.log("dropped", currentItemIndex, isBefore, isStash)。这属于调试代码,不应该合入主分支。
    • 建议:移除该 console.log,或者替换为更规范的日志系统(如 console.info / console.debug),并在发布构建中屏蔽底层调试日志。

3. 代码性能

  • Math.floor 的重复调用
    onEnteredonDropped 中,Math.floor(currentItemIndex) 被多次调用。虽然 Math.floor 性能开销极小,但在高频事件(如拖拽)中,减少不必要的函数调用是好习惯。
    • 建议:如果 currentItemIndex 在整个拖拽过程中不会变化,可以在 onEntered 开始时计算一次并缓存;如果必须实时计算,当前写法也可接受,但可以注意保持代码风格统一。

4. 代码安全

  • XSS 与注入风险(低风险)
    QML/JS 环境下 getDataAsString 获取的数据如果直接用于显示,存在理论上的注入风险,但此处仅用于逻辑判断(=== "stashed"),因此是安全的。
  • 边界条件防御
    currentItemIndexsurfaceId 是外部传入/计算得来的,建议在调用底层 C++ Model (DDT.TraySortOrderModel) 之前,对关键参数进行防御性校验,防止底层 C++ 侧因接收到非法值(如 undefinednull,负数索引等)而发生崩溃。
    • 建议
      if (shouldAllowDrop && (isStash || source === "quickPanel")) {
          if (surfaceId && currentItemIndex >= 0) {
              DDT.TraySortOrderModel.stageDropPosition(surfaceId, Math.floor(currentItemIndex))
          }
      }

综合改进后的代码示例

结合以上建议,改进后的代码片段如下:

// 建议在文件顶部或相关上下文定义常量
// const DND_SECTION_TYPE = "text/x-dde-shell-tray-dnd-sectionType"
// const DND_SOURCE_QUICK_PANEL = "quickPanel"

// onEntered 内部:
let shouldAllowDrop = showStashActionVisible ? (dropHoverIndex !== 0) : (dropHoverIndex !== -1)
// 注意:确保 source 在此处已正确获取
let isQuickPanel = source === "quickPanel" 

if (shouldAllowDrop && (isStash || isQuickPanel)) {
    // 收纳区或快捷面板拖拽:使用暂存机制
    if (surfaceId && currentItemIndex >= 0) { // 防御性编程
        DDT.TraySortOrderModel.stageDropPosition(surfaceId, Math.floor(currentItemIndex))
    }
} else if (shouldAllowDrop) {
    // 托盘内部拖拽:使用定时器机制
    dropTrayTimer.handleDrop = function() {
        if (isDropped || dragExited) return
        DDT.TraySortOrderModel.dropToDockTray(surfaceId, Math.floor(currentItemIndex), isBefore)
    }
    dropTrayTimer.restart()
}

// onDropped 内部:
let isBefore = dropIdx.isBefore
let isStash = dropEvent.getDataAsString("text/x-dde-shell-tray-dnd-sectionType") === "stashed"
let isQuickPanel = source === "quickPanel" // 确保此处 source 已定义
// console.log("dropped", currentItemIndex, isBefore, isStash) // 移除或降级为 debug 日志

// 从收纳区或快捷中心拖拽的使用暂存机制
if (isStash || isQuickPanel) {
    DDT.TraySortOrderModel.commitStagedDrop()
} else {
    // 托盘内部拖拽直接提交
    DDT.TraySortOrderModel.dropToDockTray(surfaceId, Math.floor(currentItemIndex), isBefore);
}
DDT.TraySortOrderModel.actionsAlwaysVisible = false

// onExited 内部:
onExited: function () {
    dragExited = true
    DDT.TraySortOrderModel.clearStagedDrop()
    
    // 警告:请确认以下逻辑是否已被其他生命周期钩子(如 onCanceled)接管!
    // 如果没有,从快捷面板拖入后退出可能无法正确清理状态
    /*
    if (source !== "" && !isDropped) {
        dropTrayTimer.stop()
        DDT.TraySortOrderModel.setSurfaceVisible(surfaceId, false)
    }
    */
    
    // Hide action icons when drag leaves tray without dropping
    if (!isDropped) {
        DDT.TraySortOrderModel.actionsAlwaysVisible = false
    }
}

重点确认事项:请务必核实 source 变量的来源以及 onExited 中被删除的快捷面板状态重置逻辑是否有替代方案,这两点最容易引发线上Bug。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, Ivy233

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

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