Skip to content

fix(tab): use Window as message parent when lacking file permission#457

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
GongHeng2017:202604291712-eagle-fix
Apr 29, 2026
Merged

fix(tab): use Window as message parent when lacking file permission#457
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
GongHeng2017:202604291712-eagle-fix

Conversation

@GongHeng2017
Copy link
Copy Markdown

@GongHeng2017 GongHeng2017 commented Apr 29, 2026

Use Window instance instead of currentWidget as the parent for permission denied messages to prevent potential null pointer issues.

当文件无权限时,使用 Window 实例替代 currentWidget 作为消息父控件,
避免潜在的空指针问题。

Log: 修复无权限文件打开时的消息提示目标
Bug: https://pms.uniontech.com/bug-view-353539.html
Influence: 修复后打开无权限文件时,权限不足提示能稳定显示,不会因 currentWidget 为空而失败。

Summary by Sourcery

Bug Fixes:

  • Ensure permission-denied messages are reliably shown even when there is no current editor widget for the tab.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 29, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Replaces the tab editor’s current widget as the parent of the permission-denied warning message with the Window instance itself to avoid null-pointer issues when opening files without sufficient permissions.

File-Level Changes

Change Details Files
Ensure permission-denied message is always attached to a valid parent widget when opening a file without read permission.
  • Replace m_editorWidget->currentWidget() with the Window instance as the parent widget passed to DMessageManager::sendMessage when file permissions are insufficient
  • Keep existing warning icon and translated message text unchanged while adjusting only the message parent
src/widgets/window.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
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.

Hey - I've left some high level feedback:

  • In the permissions check, you handle ReadUser, ReadOwner, and ReadOther but not ReadGroup; consider including QFile::ReadGroup so group-based read permissions are respected when deciding whether to show the permission error.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the permissions check, you handle `ReadUser`, `ReadOwner`, and `ReadOther` but not `ReadGroup`; consider including `QFile::ReadGroup` so group-based read permissions are respected when deciding whether to show the permission error.

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.

Use Window instance instead of currentWidget as the parent for
permission denied messages to prevent potential null pointer issues.

当文件无权限时,使用 Window 实例替代 currentWidget 作为消息父控件,
避免潜在的空指针问题。

Log: 修复无权限文件打开时的消息提示目标
Bug: https://pms.uniontech.com/bug-view-353539.html
Influence: 修复后打开无权限文件时,权限不足提示能稳定显示,不会因 currentWidget 为空而失败。
@GongHeng2017 GongHeng2017 force-pushed the 202604291712-eagle-fix branch from 56862b4 to 0a611cc Compare April 29, 2026 09:56
@github-actions
Copy link
Copy Markdown

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "src/widgets/window.cpp": [
        {
            "line": "    QString key = \"base/enable\";",
            "line_number": 390,
            "rule": "S106",
            "reason": "Var naming | 64f28539d9"
        }
    ]
}

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

代码审查报告

1. 语法逻辑

  • 版权年份更新SPDX-FileCopyrightText 从 2011-2023 更新至 2011-2026。这是一个合法的修改,用于延长版权声明的时间范围。逻辑上没有问题。

2. 代码质量

  • 消息接收者修改
    • 原代码m_editorWidget->currentWidget()
    • 修改后this
    • 分析:将消息的接收者从当前编辑器小部件(currentWidget())更改为窗口本身(this)。这可能是为了确保消息显示在更合适的位置(例如窗口顶部),而不是可能被遮挡或不可见的编辑器小部件上。这种修改在UI交互上可能是合理的,但需要确保 DMessageManager::instance()->sendMessage() 的实现支持这种用法。

3. 代码性能

  • 性能影响:修改对性能的影响可以忽略不计。this 是一个简单的指针传递,而 m_editorWidget->currentWidget() 可能涉及函数调用和额外的查找操作。因此,修改后可能略有性能提升,但实际影响极小。

4. 代码安全

  • 权限检查逻辑
    • 原代码和修改后的代码在权限检查逻辑上完全一致,没有变化。
    • 潜在问题:权限检查使用了 QFile::ReadUser || QFile::ReadOwner || QFile::ReadOther,这可能过于宽松。例如,ReadOther 允许任何用户读取文件,这在某些安全场景下可能是不合适的。建议根据实际需求调整权限检查逻辑,例如仅检查 ReadUserReadOwner
  • 文件存在性检查fileInfo.exists() 确保文件存在,这是良好的实践。

5. 其他建议

  • 国际化字符串tr("You do not have permission to open %1") 使用了Qt的国际化机制,这是正确的做法。
  • 错误处理:当前代码在权限不足时直接返回,但可能需要额外的日志记录或错误处理(例如记录到日志文件),以便于调试和审计。
  • 图标资源QIcon(":/images/warning.svg") 的路径是硬编码的,建议使用资源管理系统或宏定义来管理路径,以便于维护和国际化。

6. 改进建议

  • 权限检查优化

    bool bIsRead = (permissions & QFile::ReadUser) || (permissions & QFile::ReadOwner);

    如果不需要允许其他用户读取文件,可以移除 ReadOther 的检查。

  • 消息接收者明确化
    如果 DMessageManager::instance()->sendMessage() 的行为对消息接收者有特定要求,建议添加注释说明为什么选择 this 而不是 m_editorWidget->currentWidget()

  • 日志记录

    qWarning() << "Permission denied for file:" << filepath;

    在权限不足时添加日志记录。

总结

修改后的代码在语法逻辑和安全性上与原代码基本一致,主要变化是消息接收者的调整,这在UI交互上可能是合理的。建议进一步优化权限检查逻辑并添加日志记录以提高代码的健壮性和可维护性。

@github-actions
Copy link
Copy Markdown

  • 敏感词检查失败, 检测到1个文件存在敏感词
详情
{
    "src/widgets/window.cpp": [
        {
            "line": "    QString key = \"base/enable\";",
            "line_number": 390,
            "rule": "S106",
            "reason": "Var naming | 64f28539d9"
        }
    ]
}

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: GongHeng2017, 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

@GongHeng2017
Copy link
Copy Markdown
Author

/merge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented Apr 29, 2026

This pr cannot be merged! (status: unstable)

@GongHeng2017
Copy link
Copy Markdown
Author

/forcemerge

@deepin-bot
Copy link
Copy Markdown
Contributor

deepin-bot Bot commented Apr 29, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot Bot merged commit 49b149e into linuxdeepin:release/eagle Apr 29, 2026
20 of 22 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