Skip to content

Conversation

@wjyrich
Copy link
Contributor

@wjyrich wjyrich commented Sep 25, 2025

Added validation to prevent drag and drop operations from targeting fixed plugin sections in the system tray. The changes include:

  1. Implemented isForbiddenDropTarget function to check if drop target is a fixed section
  2. Added drag event rejection when attempting to drop on fixed sections
  3. Marked quick settings toggle as fixed section in the model
  4. This prevents users from accidentally moving or reordering critical system components

fix: 防止拖拽项目到固定插件区域

添加验证以防止拖放操作目标为系统托盘中的固定插件区域。更改包括:

  1. 实现 isForbiddenDropTarget 函数检查拖放目标是否为固定区域
  2. 在尝试拖放到固定区域时添加拖放事件拒绝
  3. 在模型中标记快速设置切换为固定区域
  4. 防止用户意外移动或重新排列关键系统组件

Pms: BUG-289447 BUG-289445

Summary by Sourcery

Disallow dragging tray items onto fixed plugin sections to prevent accidental reordering of critical system components.

Bug Fixes:

  • Prevent drag and drop operations from targeting fixed plugin sections in the system tray

Enhancements:

  • Introduce isForbiddenDropTarget function to detect fixed sections and reject invalid drop events
  • Mark the quick settings toggle action as a fixed section in the TraySortOrderModel

@sourcery-ai
Copy link

sourcery-ai bot commented Sep 25, 2025

Reviewer's Guide

Adds drag-and-drop validation in the tray UI to block moves onto fixed plugin sections by detecting forbidden targets in QML and marking the quick settings toggle as a fixed section in the sort order model.

File-Level Changes

Change Details Files
Add forbidden-drop-target detection in TrayContainer QML
  • Implement isForbiddenDropTarget to lookup sectionType at cursor
  • In onEntered handler, reject dragEvent when target is fixed
panels/dock/tray/package/TrayContainer.qml
Label quick settings toggle as fixed in C++ model
  • Set SectionTypeRole to SECTION_FIXED for the quick settings action in updateVisualIndexes
panels/dock/tray/traysortordermodel.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@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.

Hey there - I've reviewed your changes - here's some feedback:

  • Consider moving the linear search in isForbiddenDropTarget into the C++ model or caching a mapping from visual index to section type to avoid iterating the entire model on every drag event.
  • Add the forbidden-target check in onDragMove (in addition to onEntered) to prevent users from dragging into fixed sections mid-drag.
  • Define sectionType values as exported constants or enums in QML instead of using the hardcoded string literal 'fixed' to reduce brittleness.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider moving the linear search in isForbiddenDropTarget into the C++ model or caching a mapping from visual index to section type to avoid iterating the entire model on every drag event.
- Add the forbidden-target check in onDragMove (in addition to onEntered) to prevent users from dragging into fixed sections mid-drag.
- Define sectionType values as exported constants or enums in QML instead of using the hardcoded string literal 'fixed' to reduce brittleness.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@wjyrich wjyrich force-pushed the fix-bug-289445 branch 2 times, most recently from acfbd97 to 0e23b25 Compare September 25, 2025 07:06
@18202781743 18202781743 requested a review from Copilot September 25, 2025 07:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR prevents users from dragging items to fixed plugin sections in the system tray by implementing validation logic to reject drop operations on protected areas. The changes prevent accidental reordering of critical system components like the quick settings toggle.

  • Added validation to detect and reject drag operations targeting fixed plugin sections
  • Implemented getSectionTypeByVisualIndex function to identify section types during drag operations
  • Marked the quick settings toggle as a fixed section in the data model

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
traysortordermodel.h Adds Q_INVOKABLE method declaration for getting section type by visual index
traysortordermodel.cpp Implements section type lookup function and marks quick settings as fixed section
TrayContainer.qml Adds drag validation logic to reject drops on fixed sections

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

}
}

return QString(); // 未找到对应项目
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

The comment is in Chinese. Consider using English for consistency with the rest of the codebase: '// Item not found'

Suggested change
return QString(); // 未找到对应项目
return QString(); // Item not found

Copilot uses AI. Check for mistakes.
Comment on lines 171 to 177
// 检查当前悬停位置是否是禁止拖拽的插件
let dropIdx = DDT.TrayItemPositionManager.itemIndexByPoint(Qt.point(drag.x, drag.y))
let visualIndex = dropIdx.index
let sectionType = DDT.TraySortOrderModel.getSectionTypeByVisualIndex(visualIndex)
if (sectionType === "fixed") {
dragEvent.accepted = false
return
}
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

The comment on line 171 is in Chinese. Consider using English for consistency: '// Check if current hover position is a forbidden drag target plugin'

Copilot uses AI. Check for mistakes.
return
}

// 检查 ActionShowStashDelegate 是否显示
Copy link

Copilot AI Sep 25, 2025

Choose a reason for hiding this comment

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

The comment is in Chinese. Consider using English for consistency: '// Check if ActionShowStashDelegate is visible'

Suggested change
// 检查 ActionShowStashDelegate 是否显示
// Check if ActionShowStashDelegate is visible

Copilot uses AI. Check for mistakes.
Added validation to prevent drag and drop operations from targeting
fixed plugin sections in the system tray. The changes include:
1. Implemented isForbiddenDropTarget function to check if drop target is
a fixed section
2. Added drag event rejection when attempting to drop on fixed sections
3. Marked quick settings toggle as fixed section in the model
4. This prevents users from accidentally moving or reordering critical
system components

fix: 防止拖拽项目到固定插件区域

添加验证以防止拖放操作目标为系统托盘中的固定插件区域。更改包括:
1. 实现 isForbiddenDropTarget 函数检查拖放目标是否为固定区域
2. 在尝试拖放到固定区域时添加拖放事件拒绝
3. 在模型中标记快速设置切换为固定区域
4. 防止用户意外移动或重新排列关键系统组件

Pms: BUG-289447 BUG-289445
@deepin-ci-robot
Copy link

deepin pr auto review

我对这个代码审查如下:

  1. 代码逻辑:
  • 代码在拖放功能中增加了对固定类型(fixed)插件的检查,防止它们被拖动
  • 新增了getModelIndexByVisualIndex函数用于通过可视索引获取模型索引
  • 在updateVisualIndexes函数中将快速设置按钮标记为固定类型
  1. 语法问题:
  • 代码语法基本正确,没有明显的语法错误
  • QML和C++之间的交互正确使用了Qt的信号槽机制
  1. 代码质量:
  • 代码结构清晰,功能划分合理
  • 注释充分,逻辑可读性良好
  • 使用了适当的Qt框架特性
  1. 性能考虑:
  • getModelIndexByVisualIndex函数使用线性搜索,当项目数量较多时可能影响性能
  • 建议考虑使用哈希表或其他数据结构来缓存可视索引到模型索引的映射关系,以提高查找效率
  1. 代码安全:
  • 对拖放事件进行了适当的类型检查
  • 对固定类型插件的拖放进行了限制,提高了系统的稳定性
  • 使用了Qt的类型安全特性进行数据传递

改进建议:

  1. 性能优化:
QModelIndex TraySortOrderModel::getModelIndexByVisualIndex(int visualIndex) const
{
    // 考虑添加一个缓存映射,避免每次都线性搜索
    static QHash<int, QModelIndex> visualIndexToModelIndexCache;
    
    // 如果缓存中没有,则进行查找并更新缓存
    if (!visualIndexToModelIndexCache.contains(visualIndex)) {
        for (int i = 0; i < rowCount(); i++) {
            QModelIndex index = this->index(i, 0);
            int itemVisualIndex = data(index, VisualIndexRole).toInt();
            bool visibility = data(index, VisibilityRole).toBool();
            
            if (visibility && itemVisualIndex == visualIndex) {
                visualIndexToModelIndexCache[visualIndex] = index;
                return index;
            }
        }
        visualIndexToModelIndexCache[visualIndex] = QModelIndex();
    }
    
    return visualIndexToModelIndexCache.value(visualIndex);
}
  1. 代码健壮性:
  • 在TrayContainer.qml中,建议对DDT.TrayItemPositionManager.itemIndexByPoint的返回值进行空值检查
  • 考虑添加对visualIndex参数的有效性检查
  1. 代码组织:
  • 可以考虑将拖放相关的逻辑封装到一个单独的类中,以提高代码的可维护性
  • 建议将固定的插件类型定义为一个枚举或常量,而不是硬编码字符串
  1. 功能扩展:
  • 可以考虑添加对拖放操作的撤销/重做功能
  • 可以考虑添加对拖放动画的支持,以提升用户体验

总体而言,这段代码实现了拖放功能的基本需求,并且对固定类型插件进行了适当的保护。通过上述改进,可以进一步提高代码的性能、健壮性和可维护性。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@wjyrich wjyrich merged commit f491d05 into linuxdeepin:master Sep 25, 2025
9 of 11 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.

4 participants