Skip to content

fix(remotemanage): group dialog obscured by terminal window#528

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:master
Apr 30, 2026
Merged

fix(remotemanage): group dialog obscured by terminal window#528
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
pengfeixx:master

Conversation

@pengfeixx
Copy link
Copy Markdown
Contributor

GroupConfigOptDlg was created without parent and window modality, causing it to be hidden behind terminal. Now passes parent and sets Qt::WindowModal, matching ServerConfigOptDlg behavior.

添加分组对话框未设置父窗口和窗口模态,导致被终端窗口遮挡。
现在传入父窗口并设置Qt::WindowModal,与添加服务器对话框一致。

Log: 修复添加分组对话框被终端窗口遮挡的问题
PMS: BUG-358805
Influence: 修复后添加分组对话框始终保持在终端窗口之上,不会被遮挡。

GroupConfigOptDlg was created without parent and window modality,
causing it to be hidden behind terminal. Now passes parent and sets
Qt::WindowModal, matching ServerConfigOptDlg behavior.

添加分组对话框未设置父窗口和窗口模态,导致被终端窗口遮挡。
现在传入父窗口并设置Qt::WindowModal,与添加服务器对话框一致。

Log: 修复添加分组对话框被终端窗口遮挡的问题
PMS: BUG-358805
Influence: 修复后添加分组对话框始终保持在终端窗口之上,不会被遮挡。
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码修改主要是在创建 GroupConfigOptDlg 对象时,给构造函数增加了一个 this 指针作为参数。这意味着将对话框的父对象设置为了当前的 RemoteManagementPanel

以下是针对这段修改的详细审查意见:

1. 语法逻辑

  • 修改点new GroupConfigOptDlg(groupName) -> new GroupConfigOptDlg(groupName, this)
  • 分析
    • 修改前:对话框没有父对象(parentnullptr)。在 Qt 中,没有父对象的 QWidget 默认是顶层窗口。
    • 修改后:对话框的父对象被设置为 this(即 RemoteManagementPanel)。
  • 逻辑影响
    • 对象树管理:这是 Qt 内存管理的关键改进。设置 parent 后,当 RemoteManagementPanel 析构时,dlg 也会被自动删除,防止了潜在的内存泄漏。
    • 窗口层级:对话框将不再是独立的顶层窗口,而是依附于父窗口。这通常会导致对话框显示在父窗口的中心位置,并且在任务栏中可能不会单独显示图标。
    • 生命周期风险:如果 RemoteManagementPanel 在对话框打开期间被销毁,对话框也会随之销毁。需要确认这是否符合业务逻辑预期(通常这是预期的行为,防止面板销毁后对话框还在操作已失效的数据)。

2. 代码质量

  • 改进意见非常好
  • 理由:遵循了 Qt 的对象所有权机制,利用对象树自动管理内存,减少了手动 delete 的必要性,提高了代码的健壮性。

3. 代码性能

  • 影响无显著影响
  • 理由:设置父对象只是在对象内部增加了一个指针引用,对性能开销可以忽略不计。

4. 代码安全

  • 改进意见提升了安全性
  • 理由
    • 内存安全:如前所述,避免了内存泄漏。
    • 悬空指针风险:如果 RemoteManagementPanel 被销毁,而对话框依然存在且持有对 RemoteManagementPanel 的引用(通过 lambda 捕获 this),那么对话框关闭时的操作可能会导致悬空指针访问。通过设置父对象,确保了面板销毁时对话框也被销毁,从而切断了这种风险路径。

5. 进一步优化建议

虽然这段修改本身是正确的,但结合上下文,建议关注以下几点:

  1. Lambda 捕获
    代码中使用了 [ = ] 进行值捕获。如果 GroupConfigOptDlg 继承自 QDialog,通常建议使用 Qt::UniqueConnection 连接信号,或者确保在对话框关闭时断开连接(虽然对话框被销毁时会自动断开,但显式管理更清晰)。
    此外,如果 lambda 中不需要捕获外部变量(除了 dlg 指针本身,这里 result 是参数),可以考虑使用 [=] 或者明确捕获 dlg

  2. 内存管理确认
    请确认 GroupConfigOptDlg 的构造函数签名是否支持 QWidget* parent 参数。

    // 假设构造函数如下:
    explicit GroupConfigOptDlg(const QString &groupName, QWidget *parent = nullptr);

    如果构造函数没有默认参数,或者第二个参数不是 QWidget*,则此修改会导致编译错误。根据 diff 看来编译是通过的,所以这一点应该是满足的。

  3. 窗口属性
    如果希望对话框虽然设置了父对象,但依然保持顶层窗口的行为(例如模态对话框),请确保在 GroupConfigOptDlg 的构造函数中调用了 setWindowFlagssetWindowModality

    • 如果是模态对话框,通常需要 dlg->setModal(true) 或者在构造时使用 Qt::Dialog 标志。
    • 如果不希望它作为子窗口嵌入到父窗口中(即保持独立窗口外观),构造函数中应确保设置了 Qt::Window 标志。

总结
这是一个高质量的修改,正确利用了 Qt 的父子对象机制来管理内存和生命周期,解决了潜在的内存泄漏问题,并增强了代码的安全性。建议合并。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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 a24bf0b into linuxdeepin:master Apr 30, 2026
16 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