Skip to content

fix: save dialog resets filename and format when changing save path#202

Merged
lzwind merged 1 commit into
linuxdeepin:masterfrom
JWWTSL:master
May 28, 2026
Merged

fix: save dialog resets filename and format when changing save path#202
lzwind merged 1 commit into
linuxdeepin:masterfrom
JWWTSL:master

Conversation

@JWWTSL

@JWWTSL JWWTSL commented May 28, 2026

Copy link
Copy Markdown
Contributor

Log: setDirectory() was called after selectFile and selectNameFilter, causing the native GTK dialog to clear the previously set filename and format on directory change. Fixed by moving setDirectory() before filter and file selection so selections persist.

Bug: https://pms.uniontech.com/bug-view-347163.html

Summary by Sourcery

Bug Fixes:

  • Prevent the GTK native save dialog from resetting the selected filename and format when the directory is changed.

@sourcery-ai

sourcery-ai Bot commented May 28, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Reorders and consolidates file dialog initialization so the directory is set before filters and selected file, preventing GTK from clearing the filename and selected format when the save path changes.

Sequence diagram for updated file save dialog initialization order

sequenceDiagram
    participant DrawBoard_private
    participant FileSelectDialog

    DrawBoard_private->>FileSelectDialog: FileSelectDialog(_borad)
    alt file not empty
        DrawBoard_private->>FileSelectDialog: setDirectory(QFileInfo(file).dir().absolutePath())
        DrawBoard_private->>FileSelectDialog: setNameFilters(drawApp->writableFormatNameFilters())
        DrawBoard_private->>FileSelectDialog: selectNameFilter(drawApp->defaultFileDialogNameFilter())
        DrawBoard_private->>FileSelectDialog: selectFile(file)
    else file empty
        DrawBoard_private->>FileSelectDialog: setDirectory(drawApp->defaultFileDialogPath())
        DrawBoard_private->>FileSelectDialog: setNameFilters(drawApp->writableFormatNameFilters())
        DrawBoard_private->>FileSelectDialog: selectNameFilter(drawApp->defaultFileDialogNameFilter())
        DrawBoard_private->>FileSelectDialog: selectFile(fullFileName)
    end
    DrawBoard_private->>FileSelectDialog: exec()
Loading

File-Level Changes

Change Details Files
Ensure the file save dialog sets its initial directory before configuring filters and selected filename so state is preserved on GTK when changing directories.
  • Move setDirectory() calls before setNameFilters(), selectNameFilter(), and selectFile() when initializing the FileSelectDialog.
  • When an existing file path is provided, set the dialog directory to that file’s directory before applying filters and selecting the file.
  • When no file path is provided, set the dialog directory to the application’s default file dialog path before constructing and selecting the default filename with a format suffix.
src/frame/ccentralwidget.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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

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:

  • The directory selection logic is now split between the initial if (!file.isEmpty()) block and later branches; consider computing the target directory once (e.g., into a local QString directory) before constructing/configuring the dialog so that future changes to directory selection are centralized and less error-prone.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The directory selection logic is now split between the initial `if (!file.isEmpty())` block and later branches; consider computing the target directory once (e.g., into a local `QString directory`) before constructing/configuring the dialog so that future changes to directory selection are centralized and less error-prone.

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.

@JWWTSL JWWTSL force-pushed the master branch 2 times, most recently from e50c474 to ba05af4 Compare May 28, 2026 07:42
lzwind
lzwind previously approved these changes May 28, 2026
@JWWTSL

JWWTSL commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

/forcemerge

Log: setDirectory() after selectFile/selectNameFilter caused native GTK dialog to clear selections on directory change; fixed by reordering setDirectory() first and enabling DontUseNativeDialog.

Pms: bug-3471613
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已经仔细审查了你提供的Git Diff。本次修改主要解决了文件对话框在特定桌面环境(如GTK)下,由于原生对话框行为导致的文件名和过滤器被重置的Bug,并对版权信息进行了更新。

以下是我对本次代码变更在语法逻辑、代码质量、代码性能和代码安全方面的详细审查意见:

1. 语法与逻辑

  • ccentralwidget.cpp 逻辑调整(优秀)
    setDirectory 提前到 setNameFiltersselectFile 之前是非常正确的做法。在Linux的GTK原生文件选择对话框中,后调用 setDirectory 会触发路径刷新,从而导致之前设置的文件名和过滤器丢失。本次修改从逻辑上完美解决了这个时序问题。
  • cmultiptabbarwidget.cpp 选项修改(存在隐患)
    启用了 QFileDialog::DontUseNativeDialog 选项,这确实能规避原生对话框(如GTK)的各种怪异行为,保证跨平台行为的一致性。但是,Qt的内置对话框(非原生)在处理大量文件或网络路径时性能较差,且外观上无法完美融入Deepin/UOS的桌面风格。
    • 建议:如果必须使用非原生对话框来保证功能正确性,建议增加编译期或运行期的条件宏控制,例如仅在Linux/GTK环境下禁用原生对话框,而在其他平台保持原生:
    #ifdef Q_OS_LINUX
        this->setOptions(QFileDialog::DontResolveSymlinks | QFileDialog::DontUseNativeDialog);
    #else
        this->setOptions(QFileDialog::DontResolveSymlinks);
    #endif

2. 代码质量

  • 注释清晰(优秀)
    新增的注释 // Set directory first to avoid GTK native dialog resetting filename and name filter when setDirectory is called later 非常好,清楚地解释了为什么要调整调用顺序。这对于后续维护者理解这段看似“多此一举”的代码至关重要。
  • 版权信息更新(需注意)
    将版权时间从 2023 修改为 2026。通常版权声明的时间应该是作品首次发布或最近实质性修改的年份,修改为未来的年份(2026)可能是为了省去未来几年的频繁修改,但严格从法律合规角度来说,版权年份应反映真实的发表/修改年份。
    • 建议:如果公司/项目规范允许这种前瞻性写法则无妨,否则建议改为当前实际年份(如2024)或范围式写法(如 2020-present)。

3. 代码性能

  • QFileInfo 的重复构造(可优化)
    ccentralwidget.cpp 中,QFileInfo(file)setDirectoryselectFile 的逻辑中被分别构造了一次。虽然 QFileInfo 构造开销极小,但在同一逻辑块中,建议复用对象以提升代码整洁度和微小的性能。
    • 改进建议
    if (!file.isEmpty()) {
        QFileInfo fileInfo(file); // 只构造一次
        dialog.setDirectory(fileInfo.dir().absolutePath());
        dialog.setNameFilters(drawApp->writableFormatNameFilters());
        dialog.selectNameFilter(drawApp->defaultFileDialogNameFilter());
        dialog.selectFile(file); // 或者使用 fileInfo.absoluteFilePath()
    } else {
        // ...
    }
  • 非原生对话框的性能开销(需评估)
    如前所述,DontUseNativeDialog 会使用Qt纯绘制的对话框,在文件列表极多(如上万个文件)的目录下首次打开时,渲染速度明显慢于系统原生对话框。如果该软件经常需要浏览超大目录,请务必进行性能测试。

4. 代码安全

  • 路径遍历与符号链接(安全提醒)
    代码中保留了 QFileDialog::DontResolveSymlinks 选项。这意味着如果用户选择了一个符号链接,后续保存文件时获取的路径可能是一个包含软链接的路径。
    • 风险:如果后续的保存逻辑对文件路径有严格的权限或目录归属校验,未解析的软链接可能导致路径校验被绕过(Path Traversal的一种场景)。
    • 建议:确认后续保存文件的业务逻辑中,是否需要调用 QFileInfo::canonicalFilePath() 来获取解析后的真实绝对路径进行校验和写入,以防止潜在的安全风险。
  • 默认文件名后缀逻辑(逻辑自洽性)
    else 分支中,代码通过循环 drawApp->writableFormatNameFilters() 来为默认文件名添加后缀。由于 setNameFilters 已经执行,这里的逻辑是自洽的。但需注意,如果过滤器列表为空,fullFileName 将没有后缀,这可能导致用户保存出一个无后缀的文件,在某些严格依赖后缀的系统中可能引发打开困难。建议增加对过滤器为空时的兜底处理。

总结

本次修改核心逻辑正确,有效修复了GTK环境下文件对话框状态重置的问题。主要的改进点在于评估是否必须全局禁用原生对话框以及减少 QFileInfo 的冗余构造。代码安全性方面需确认后续文件写入时对软链接路径的处理。

希望这些审查意见对你有所帮助!如果有任何疑问,欢迎随时讨论。

@JWWTSL JWWTSL requested a review from lzwind May 28, 2026 09:11
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JWWTSL, lzwind

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

@lzwind lzwind merged commit 52d6166 into linuxdeepin:master May 28, 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