Skip to content

fix: remove password visibility button key event handling#723

Merged
mhduiy merged 1 commit intolinuxdeepin:masterfrom
mhduiy:button
Jan 31, 2026
Merged

fix: remove password visibility button key event handling#723
mhduiy merged 1 commit intolinuxdeepin:masterfrom
mhduiy:button

Conversation

@mhduiy
Copy link
Contributor

@mhduiy mhduiy commented Jan 29, 2026

Removed the event filter that was intercepting Enter/Return key presses on the password visibility toggle button. The button now uses default Qt button behavior for key events instead of custom handling. This change simplifies the code and ensures consistent behavior with standard Qt button interactions.

The previous implementation specifically handled Enter/Return key presses to trigger button clicks when the button was not set as default. This custom logic has been removed to allow the button to use Qt's built-in key event handling mechanisms.

Influence:

  1. Test password visibility toggle functionality with mouse clicks
  2. Verify Enter/Return key behavior in password fields
  3. Check that button focus and activation work as expected
  4. Test overall password edit widget behavior in forms
  5. Verify no regression in password visibility toggling

fix: 移除密码可见性按钮的按键事件处理

移除了拦截密码可见性切换按钮上 Enter/Return 按键的事件过滤器。现在按钮
使用 Qt 默认的按钮行为来处理按键事件,而不是自定义处理。这一更改简化了代
码,并确保与标准 Qt 按钮交互行为的一致性。

之前的实现专门处理了 Enter/Return 按键,以便在按钮未设置为默认按钮时触
发按钮点击。这一自定义逻辑已被移除,以允许按钮使用 Qt 内置的按键事件处理
机制。

Influence:

  1. 测试通过鼠标点击切换密码可见性的功能
  2. 验证密码字段中 Enter/Return 键的行为
  3. 检查按钮焦点和激活功能是否按预期工作
  4. 测试表单中密码编辑控件的整体行为
  5. 验证密码可见性切换功能没有回归问题

PMS: BUG-341493

Removed the event filter that was intercepting Enter/Return key presses
on the password visibility toggle button. The button now uses default Qt
button behavior for key events instead of custom handling. This change
simplifies the code and ensures consistent behavior with standard Qt
button interactions.

The previous implementation specifically handled Enter/Return key
presses to trigger button clicks when the button was not set as default.
This custom logic has been removed to allow the button to use Qt's
built-in key event handling mechanisms.

Influence:
1. Test password visibility toggle functionality with mouse clicks
2. Verify Enter/Return key behavior in password fields
3. Check that button focus and activation work as expected
4. Test overall password edit widget behavior in forms
5. Verify no regression in password visibility toggling

fix: 移除密码可见性按钮的按键事件处理

移除了拦截密码可见性切换按钮上 Enter/Return 按键的事件过滤器。现在按钮
使用 Qt 默认的按钮行为来处理按键事件,而不是自定义处理。这一更改简化了代
码,并确保与标准 Qt 按钮交互行为的一致性。

之前的实现专门处理了 Enter/Return 按键,以便在按钮未设置为默认按钮时触
发按钮点击。这一自定义逻辑已被移除,以允许按钮使用 Qt 内置的按键事件处理
机制。

Influence:
1. 测试通过鼠标点击切换密码可见性的功能
2. 验证密码字段中 Enter/Return 键的行为
3. 检查按钮焦点和激活功能是否按预期工作
4. 测试表单中密码编辑控件的整体行为
5. 验证密码可见性切换功能没有回归问题

PMS: BUG-341493
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

这段代码是一个删除操作的 Git diff,从 DPasswordEdit::eventFilter 函数中移除了一段处理 QEvent::KeyPress 的逻辑。以下是对这段代码变更的详细审查意见,包括语法逻辑、代码质量、性能和安全方面的分析:

1. 语法逻辑

  • 变更分析:代码移除了对 QEvent::KeyPress 的监听以及后续的按键判断逻辑。
  • 逻辑影响:原逻辑的作用是:当用户在"切换密码可见性"按钮(togglePasswordVisibleButton)上按下回车键(Qt::Key_ReturnQt::Key_Enter)时,触发该按钮的点击事件(click()),从而切换密码的显示/隐藏状态。
  • 结论:从语法上看,删除操作本身没有问题。但从逻辑上看,这移除了一个辅助功能。删除后,用户无法通过键盘(回车键)操作该按钮,必须使用鼠标点击。这降低了键盘导航的便利性。

2. 代码质量

  • 可维护性:删除这段代码使得 eventFilter 函数更加简洁,直接将事件传递给基类 DLineEdit 处理。如果原逻辑是为了修复某个特定问题或临时方案,那么删除它是合理的。
  • 一致性:需要检查项目中其他类似的控件或按钮是否支持回车键激活。如果其他自定义按钮都支持回车键操作,那么这里的删除可能会导致交互体验不一致。
  • 注释缺失:Diff 中没有看到关于为什么要删除这段代码的注释。如果是因为该逻辑已过时、有更好的替代方案(如 Qt 的默认行为已包含此功能),建议在提交信息中说明原因。

3. 代码性能

  • 正面影响:删除这段代码对性能有微小的正面影响。每次 eventFilter 被调用时,都会减少一次 event->type() 的判断、一次指针比较(watcher == ...)、一次 isDefault() 检查以及一次 dynamic_cast 操作。虽然在现代计算机上这种开销微乎其微,但在高频触发的场景下,减少不必要的逻辑总是有益的。

4. 代码安全

  • 类型转换:原代码使用了 dynamic_cast<QKeyEvent*>(event)。这是一个安全的做法,因为它会在转换失败时返回 nullptr,代码中也正确地检查了 if (keyEvent)。虽然这段代码被删除了,但在审查遗留代码时,这种写法是值得肯定的。
  • 空指针风险:原代码中使用了 D_D(DPasswordEdit) 宏来获取 d 指针(D-Pointer 模式)。假设该宏实现正确,d 指针应该是有效的。原代码访问了 d->togglePasswordVisibleButton,如果该指针未被正确初始化,可能会导致崩溃。删除这段代码消除了这一潜在的(尽管可能很小)崩溃风险点。

改进建议

  1. 确认交互需求

    • 如果该功能是刻意移除的(例如,产品需求变更,不再允许键盘切换密码),那么此变更是正确的。建议在 Git Commit Message 中明确说明:"Remove keyboard shortcut (Enter key) for toggling password visibility to align with new UX guidelines"。
    • 如果该功能是被误删的(例如,合并冲突时误操作),那么这是一个回归 Bug。建议恢复这段代码,或者寻找替代方案。
  2. 替代方案(如果需要保留键盘交互功能)
    如果删除这段代码是因为 isDefault() 的判断逻辑有问题,或者 dynamic_cast 开销较大,但又想保留回车键触发的功能,可以考虑以下改进:

    • 使用 QShortcut:在构造函数中为该按钮设置快捷键,而不是在 eventFilter 中全局拦截。

      // 示例伪代码
      auto *shortcut = new QShortcut(QKeySequence(Qt::Key_Return), this);
      connect(shortcut, &QShortcut::activated, d->togglePasswordVisibleButton, &QAbstractButton::click);

      优点:逻辑解耦,不污染 eventFilter,Qt 内部处理更高效。

    • 重写 keyPressEvent:如果是在输入框内按回车想触发按钮点击,可以在 DPasswordEdit 类中重写 keyPressEvent,而不是在 eventFilter 中处理。

  3. 无障碍访问

    • 移除回车键支持可能会影响屏幕阅读器或仅使用键盘的用户。请确保该按钮仍然可以通过 Tab 键聚焦,并且有明确的焦点状态。

总结

这段代码变更在语法和性能上是安全的,但在功能逻辑上可能存在倒退(移除了键盘交互支持)。

最终建议:请确认移除该功能是否符合产品设计和无障碍访问规范。如果必须移除,请补充相应的文档说明或注释;如果是误删,请恢复代码并考虑使用 QShortcut 进行重构以提高代码质量。

@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, mhduiy

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

@mhduiy mhduiy merged commit 7446242 into linuxdeepin:master Jan 31, 2026
21 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