Skip to content

fix(compressor): fix cursor jumping to end when editing in rename dialog#420

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:fix-4-30
Jun 3, 2026
Merged

fix(compressor): fix cursor jumping to end when editing in rename dialog#420
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
dengzhongyuan365-dev:fix-4-30

Conversation

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor

@dengzhongyuan365-dev dengzhongyuan365-dev commented Jun 3, 2026

  • Replace textChanged+setText filtering with QRegularExpressionValidator to reject illegal characters at input time in RenameDialog

    • Use QValidator to prevent / \ : * " ' ? < > | etc. from being entered, instead of removing them after textChanged fires
    • Avoid setText() in textChanged slot which resets cursor position

    在重命名对话框中使用 QValidator 在输入阶段直接拒绝非法字符,
    替代在 textChanged 信号中使用 setText 移除字符的方式,
    解决输入时光标会跳转到末尾的问题。

    PMS: BUG-363723

    Influence: 重命名对话框输入时光标不再跳转到末尾

Summary by Sourcery

Prevent cursor position from jumping to the end when editing text in the rename dialog by validating input at entry time.

Bug Fixes:

  • Reject illegal characters in the rename dialog via a line edit validator instead of post-processing text, preventing cursor jumps during editing.

Enhancements:

  • Simplify rename dialog text handling by delegating character validation to Qt validators while keeping OK button enablement logic based on non-empty input.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Jun 3, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Refactors the RenameDialog input handling to use a validator-based filename filter instead of post-processing text changes, preventing the cursor from jumping to the end while still enforcing disallowed-character rules and keeping the OK button enablement logic intact.

Sequence diagram for RenameDialog validator-based input handling

sequenceDiagram
    actor User
    participant RenameDialog
    participant DLineEdit
    participant QValidator
    participant OkButton

    User->>DLineEdit: type character
    DLineEdit->>QValidator: validate(input)
    alt [invalid input]
        QValidator-->>DLineEdit: reject change
    else [valid input]
        QValidator-->>DLineEdit: accept change
        DLineEdit-->>RenameDialog: textChanged
        RenameDialog->>DLineEdit: text
        RenameDialog->>OkButton: setEnabled(bool)
    end
Loading

File-Level Changes

Change Details Files
Switch RenameDialog filename filtering from textChanged+setText logic to a QValidator-based approach to reject invalid characters at input time and avoid cursor jumps.
  • Add conditional includes for QRegularExpressionValidator (Qt 6+) and QRegExpValidator (pre-Qt 6).
  • Instantiate and attach a regular-expression-based validator on the inner QLineEdit of DLineEdit to disallow leading spaces/special characters and disallow special characters elsewhere.
  • Remove the text sanitization logic in the textChanged handler that used setText with a QRegularExpression filter.
  • Retain and adjust the textChanged handler to only control the OK button enabled state based on whether the input text is empty.
src/source/dialog/popupdialog.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 found 1 issue, and left some high level feedback:

  • The Qt5 and Qt6 regular expressions diverge slightly (\s vs explicit space/tab and different ordering of escape sequences); consider defining a single shared pattern (e.g., as a QString or raw string literal) and using it for both validators to ensure identical behavior and easier maintenance.
  • The inline regex strings are fairly dense and heavily escaped; using raw string literals and/or a short comment describing the allowed character set (especially for the first vs subsequent characters) would make the validation rules easier to understand and update later.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The Qt5 and Qt6 regular expressions diverge slightly (`\s` vs explicit space/tab and different ordering of escape sequences); consider defining a single shared pattern (e.g., as a QString or raw string literal) and using it for both validators to ensure identical behavior and easier maintenance.
- The inline regex strings are fairly dense and heavily escaped; using raw string literals and/or a short comment describing the allowed character set (especially for the first vs subsequent characters) would make the validation rules easier to understand and update later.

## Individual Comments

### Comment 1
<location path="src/source/dialog/popupdialog.cpp" line_range="20-27" />
<code_context>
 #include <QDebug>
 #include <DPasswordEdit>
 #include <QFileInfo>
+#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
+#include <QRegularExpressionValidator>
+#else
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Align the Qt5/Qt6 regex semantics around whitespace handling to avoid subtle behavioral differences.

In Qt6, the `QRegularExpression` pattern excludes all `\s` (including Unicode whitespace) in the first character, while the Qt5 `QRegExp` version only excludes space, `\t`, and `\r\n`. As a result, filenames starting with less-common whitespace are rejected in Qt6 but allowed in Qt5. If this divergence isn’t intentional, consider making the patterns equivalent (either both use an explicit minimal whitespace set or both approximate `\s`) so Qt5/Qt6 builds behave consistently.
</issue_to_address>

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.

Comment thread src/source/dialog/popupdialog.cpp
    - Replace textChanged+setText filtering with QRegularExpressionValidator
      to reject illegal characters at input time in RenameDialog
    - Use QValidator to prevent / \ : * " ' ? < > | etc. from being
      entered, instead of removing them after textChanged fires
    - Avoid setText() in textChanged slot which resets cursor position

    在重命名对话框中使用 QValidator 在输入阶段直接拒绝非法字符,
    替代在 textChanged 信号中使用 setText 移除字符的方式,
    解决输入时光标会跳转到末尾的问题。

    PMS: BUG-363723

    Influence: 重命名对话框输入时光标不再跳转到末尾
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeex,很高兴为你审查这段代码。

这段代码的主要改进是将输入校验从被动的 textChanged 信号中修改文本(setText),改为主动使用 QValidator 进行拦截。这是一个非常棒的改进方向!使用 Validator 可以在输入法组合字符时就拒绝非法字符,从根本上避免了 setText 导致的光标跳动和输入法卡顿问题,同时也去掉了原先代码中正则表达式未正确转义 C 字符串的隐患(原代码 \r\n\t 在 C 字符串中未被转义为 \\r\\n\\t)。

不过,在语法逻辑、代码质量和安全性方面,还有一些需要优化和注意的地方。以下是详细的审查意见:

一、 语法与逻辑问题

  1. Lambda 捕获与对象生命周期(潜在的内存泄漏/悬空指针)

    • 问题connect(m_lineEdit, &DLineEdit::textChanged,[=](){...}) 使用了值捕获 [=],这会隐式捕获 this 指针。如果 RenameDialog 被销毁但 m_lineEdit 因为某种原因仍然存在(虽然此处 parent 是 this,可能性较小,但在复杂 Qt 逻辑中需注意),或者信号触发时访问了已销毁的成员,会导致崩溃。
    • 建议:明确捕获 [this](),或者更推荐使用 C++14 的初始化捕获将需要的变量传入。如果只是访问成员变量,显式写出 [this] 能让代码意图更清晰。
  2. isNull() 判断冗余且存在逻辑漏洞

    • 问题if(text.isNull()||text.isEmpty()) 中,QString::isNull() 对于普通的文本框输入几乎永远不会返回 true(除非显式调用了 QString() 构造)。更重要的是,当前的 QValidator 允许输入为纯空格的字符串(例如输入 " "),这种情况下 text.isEmpty()false,导致确认按钮被错误地启用了。
    • 建议:移除 isNull(),并使用 text.trimmed().isEmpty() 来判断,防止用户只输入空格就通过校验。
  3. QValidator 的父子对象设置

    • 问题new QRegularExpressionValidator(re, m_lineEdit) 将 validator 的 parent 设置为了 m_lineEdit。虽然 Qt 的对象树会管理它的内存,但更规范的做法是将 validator 的 parent 设置为 m_lineEdit->lineEdit()(即真正应用该 validator 的 QWidget),或者直接设为 this(Dialog 本身)。

二、 代码性能

  1. textChanged 中的重复字符串拷贝
    • 问题QString text = m_lineEdit->text(); 每次文本改变都会产生一次深拷贝。
    • 建议:可以使用 const QString &text = m_lineEdit->text(); 使用常量引用避免不必要的内存分配。

三、 代码安全与健壮性

  1. 正则表达式的边界安全(超长字符串 DoS 攻击)

    • 问题:正则表达式 ^[^\\s/\\\\:*\"'?<>|\\r\\n\\t][^/\\\\:*\"'?<>|\\r\\n\\t]*$ 中使用了 *,这意味着可以输入无限长的合法字符。虽然 Qt 的 QLineEditmaxLength 属性,但当前代码并未看到对其进行了设置(全局变量 g_nTextLength = 255 似乎没被用上)。如果用户粘贴了一段超长文本,正则引擎的回溯可能会造成界面卡顿(ReDoS 风险)。
    • 建议:在代码中显式调用 m_lineEdit->lineEdit()->setMaxLength(g_nTextLength);,或者将正则表达式中的 * 改为限制长度的表达式(但设置 maxLength 是最高效且安全的做法)。
  2. 输入法(IME)与 Validator 的兼容性

    • 问题:在 Qt 中,QValidator 会拦截输入法的组合状态。例如,当用户使用拼音输入法时,输入 "zhong" 还在候选词阶段,如果首字母 'z' 被判定为合法,而后续拼音不符合正则,可能会导致输入法无法正常提交。
    • 建议:目前的正则只限制了首字符不能为空格和特殊字符,对英文字母是放行的,因此中文拼音输入法通常不会受影响。但需要重点测试中文输入法(如搜狗、Fcitx)的打字体验,确保不会出现无法上屏中文的问题。

四、 代码质量与可读性

  1. 冗余的宏定义分支

    • 问题:Qt 5.4 及以上版本就已经引入了 QRegularExpressionValidator。如果你的 Qt5 最低版本支持高于 5.4,完全可以直接使用 QRegularExpressionValidator,去掉 #if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0) 的分支,减少代码冗余。
    • 建议:如果必须兼容极老的 Qt5(5.4以下),保留分支是可以的;否则建议统一使用 QRegularExpressionValidator
  2. 魔法字符串

    • 问题:正则表达式字符串 ^[^\\s/\\\\:*\"'?<>|\\r\\n\\t][^/\\\\:*\"'?<>|\\r\\n\\t]*$ 较长且包含大量转义,阅读和维护成本较高。
    • 建议:可以将其提取为常量,并加好注释说明规则。

改进后的代码建议

综合以上意见,我为你修改了代码,你可以参考:

 // 引入头文件部分(如果 Qt5 版本高于 5.4,建议直接使用 QRegularExpressionValidator,去掉分支)
#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
#include <QRegularExpressionValidator>
#else
#include <QRegExpValidator>
#endif

// ... 中间代码不变 ...

int RenameDialog::showDialog(const QString &strReName, const QString &strAlias,
                             bool isDirectory, bool isRepeat)
{
    qDebug() << "Showing RenameDialog for:" << strReName << "isDirectory:" << isDirectory << "isRepeat:" << isRepeat;
    if(m_lineEdit == NULL) {
        m_lineEdit = new DLineEdit(this);
        
        // 限制最大输入长度,防止超长字符串导致正则回溯性能问题或越界
        m_lineEdit->lineEdit()->setMaxLength(g_nTextLength);

        // 规则:第一位不允许空格和特殊字符,后续位不允许特殊字符(中间空格允许)
        const QString validationPattern = "^[^\\s/\\\\:*\"'?<>|\\r\\n\\t][^/\\\\:*\"'?<>|\\r\\n\\t]*$";
        
#if QT_VERSION >= QT_VERSION_CHECK(5, 4, 0)
        QRegularExpression re(validationPattern);
        // 将 validator 的 parent 设为真正挂载的 widget 或 this
        m_lineEdit->lineEdit()->setValidator(new QRegularExpressionValidator(re, m_lineEdit->lineEdit()));
#else
        QRegExp re(validationPattern);
        m_lineEdit->lineEdit()->setValidator(new QRegExpValidator(re, m_lineEdit->lineEdit()));
#endif

        // 输入字符格式与文管保持一致
        connect(m_lineEdit, &DLineEdit::textChanged, this, [this](){
            // 使用引用避免 QString 拷贝开销
            const QString &text = m_lineEdit->text();
            if(m_nOkBtnIndex >= 0) { 
                // 移除 isNull() 判断,使用 trimmed() 防止纯空格通过校验
                if(text.trimmed().isEmpty()){
                    getButton(m_nOkBtnIndex)->setEnabled(false);
                } else {
                    getButton(m_nOkBtnIndex)->setEnabled(true);
                }
            }
        });
        // ... 其他逻辑
    }
    // ... 其他逻辑
}

总结:你原本的改进思路非常正确,用 Validator 替换 setText 是解决光标跳动和输入法问题的标准做法。通过补充上述的边界条件(长度限制、纯空格拦截)和性能优化(引用传递),这段代码将变得更加健壮和安全。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

@dengzhongyuan365-dev
Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot deepin-bot Bot merged commit edcd09e into linuxdeepin:master Jun 3, 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