Skip to content

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

Closed
JWWTSL wants to merge 2 commits into
linuxdeepin:masterfrom
JWWTSL:master
Closed

fix: save dialog resets filename and format when changing save path#204
JWWTSL wants to merge 2 commits into
linuxdeepin:masterfrom
JWWTSL:master

Conversation

@JWWTSL

@JWWTSL JWWTSL commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

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

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

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

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

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX,你的智能编程助手。我已仔细审查了你提供的Git Diff代码。这次修改主要解决了文件对话框在特定环境(特别是Linux + GTK原生对话框)下的交互Bug,并对版权信息进行了更新。

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

1. 语法与逻辑

亮点:

  • 修复了关键的逻辑Bug:在原代码中,setDirectoryselectFile 之后调用。在使用GTK原生文件对话框时,这会导致已选定的文件名和文件类型过滤器被重置。将 setDirectory 提前到 setNameFiltersselectFile 之前,符合Qt文件对话框的初始化逻辑,完美解决了该问题。

改进建议:

  • setOptions 的潜在逻辑覆盖风险:在 cmultiptabbarwidget.cpp 中,将 setOptions(QFileDialog::DontResolveSymlinks | QFileDialog::DontUseNativeDialog) 替换了原来的单选项。需要注意的是,setOptions覆盖原有的所有 Options,而不是追加。如果未来有其他地方调用了 setOptions,可能会产生覆盖冲突。如果只是想追加选项,建议使用 setOption(QFileDialog::DontUseNativeDialog, true)

2. 代码质量

亮点:

  • 添加了清晰的注释解释了为什么要将 setDirectory 提前,这对后续维护非常有帮助。

改进建议:

  • 魔法字符串/重复逻辑:在 ccentralwidget.cpp 中,QFileInfo(file).dir().absolutePath()drawApp->defaultFileDialogPath() 的逻辑分支处理得很好,但 dialog.setDirectory 被调用了两次。虽然当前逻辑清晰,但可以稍微优化以减少重复代码:

    // 建议优化为:
    const QString dirPath = file.isEmpty() ? drawApp->defaultFileDialogPath() : QFileInfo(file).dir().absolutePath();
    dialog.setDirectory(dirPath);
    
    dialog.setNameFilters(drawApp->writableFormatNameFilters());
    dialog.selectNameFilter(drawApp->defaultFileDialogNameFilter());
    
    if (!file.isEmpty()) {
        dialog.selectFile(file);
    } else {
        // ... 处理默认文件名逻辑
    }

    这样 setDirectory 只需调用一次,代码更加简洁。

  • 强制使用非原生对话框的体验折衷:启用 DontUseNativeDialog 会使用Qt自绘的对话框。在Linux Deepin/UOS等系统下,自绘对话框的风格可能与系统原生GTK主题不一致,且在某些情况下文件加载速度可能略逊于原生对话框。建议在注释中补充说明为什么必须禁用原生对话框(例如:为了避免GTK对话框重置文件名和过滤器的Bug),以便未来Qt或GTK修复该问题时,可以及时回退。

3. 代码性能

改进建议:

  • 避免重复构造 QFileInfo 对象:在 ccentralwidget.cpp 中,如果采用上述代码质量的优化建议,QFileInfo(file) 只需构造一次。如果保持现有逻辑,当 !file.isEmpty() 时,QFileInfo(file) 会被构造一次;虽然性能影响微乎其微,但遵循最佳实践,对于多次使用的相同计算,应存入局部变量。

  • DontUseNativeDialog 的性能影响:Qt的Non-Native Dialog是纯Qt Widget绘制的,在遍历含有大量文件的目录时,其响应速度和内存占用通常不如调用系统底层API的原生对话框。这是为了修复逻辑Bug而做的必要妥协,但需要在性能层面有所知晓。

4. 代码安全

改进建议:

  • 符号链接解析安全:代码中保留了 QFileDialog::DontResolveSymlinks 选项。在保存文件的场景下(AcceptSave),不解析符号链接通常是安全的,甚至可以避免用户在复杂的软链接目录中迷失。但是,如果后续业务逻辑需要对保存的路径进行权限校验或安全审计,请务必注意:取得的路径可能是包含软链接的路径,而非真实路径。如果需要获取真实路径进行安全校验,建议在 dialog.exec() 返回后,对获取到的文件路径使用 QFileInfo::canonicalFilePath() 进行一次解析,以防止通过软链接进行目录穿越等安全风险。

  • 版权年份的合规性:将版权年份从 2023 更新到 2026。请确保这种未来年份的写法符合贵司的法务合规要求。通常开源协议(如GPL-3.0-or-later)的版权声明有两种常见写法:一是写创建到当前的年份范围(如 2020-2023),二是写创建年份加当前年份(如 2020, 2023)。直接写未来年份(2026)在开源社区中相对少见,除非是预先设定的版本发布年份,建议与法务确认。


综合修改建议代码示例

针对 ccentralwidget.cpp 中的逻辑,建议优化如下:

if (toddf) {
    FileSelectDialog dialog(_borad);

    // 优化:提前计算目录路径,只调用一次setDirectory
    QString dirPath = file.isEmpty() ? drawApp->defaultFileDialogPath() 
                                     : QFileInfo(file).dir().absolutePath();
    dialog.setDirectory(dirPath);

    dialog.setNameFilters(drawApp->writableFormatNameFilters());
    dialog.selectNameFilter(drawApp->defaultFileDialogNameFilter());

    if (!file.isEmpty()) {
        dialog.selectFile(file);
    } else {
        // Add a format suffix to the default filename
        QString fullFileName = defaultFileName;
        // ... 原有的后缀处理逻辑 ...
        dialog.selectFile(fullFileName);
    }
    dialog.exec();

针对 cmultiptabbarwidget.cpp,建议增加注释说明:

// 使用 DontUseNativeDialog 以避免 GTK 原生对话框在调用 setDirectory 时重置文件名和过滤器
this->setOptions(QFileDialog::DontResolveSymlinks | QFileDialog::DontUseNativeDialog);

整体而言,这是一次非常针对痛点的好修复,主要逻辑正确,只需在代码细节和长期维护性上稍加打磨即可。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JWWTSL

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

@JWWTSL JWWTSL closed this Jun 3, 2026
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.

2 participants