Skip to content

fix: touchpad FN key only shows OSD, no double toggle - #179

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
fly602:master
Sep 20, 2026
Merged

deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
fly602:master

Conversation

@fly602

@fly602 fly602 commented Sep 20, 2026

Copy link
Copy Markdown
Contributor
  1. Remove direct DBus TPadEnable toggling from dde-shortcut-tool
  2. Keep OSD display (TouchpadToggle/TouchpadOn/TouchpadOff) only
  3. Actual touchpad switch is handled by the system keyevent service, avoiding double-toggle that inverted the CC switch state

Log: FN touchpad shortcut no longer toggles state twice; control center switch stays in sync

Influence:

  1. On a laptop with touchpad, press FN touchpad hotkey, verify touchpad toggles exactly once and OSD shows correct icon
  2. Check control center touchpad switch matches actual touchpad state after each FN press
  3. Verify touchpad enable/disable still works from control center itself

fix: 触控板FN快捷键仅显示OSD,不再重复切换状态

  1. 移除 dde-shortcut-tool 中直接通过 DBus 切换 TPadEnable 的逻辑
  2. 保留 OSD 显示(TouchpadToggle/TouchpadOn/TouchpadOff)
  3. 实际开关由系统 keyevent 服务处理,避免双重切换导致 控制中心开关状态与实际相反

Log: 触控板FN快捷键不再重复切换状态,控制中心开关保持同步

Influence:

  1. 在带触控板的笔记本上按 FN 触控板快捷键,验证触控板仅切换 一次且 OSD 图标正确
  2. 每次按 FN 后检查控制中心触控板开关与实际状态一致
  3. 验证从控制中心直接开关触控板仍正常工作

PMS: BUG-377717

Summary by Sourcery

Prevent duplicate touchpad state changes by limiting shortcut handling to OSD notifications and delegating actual toggling to the system keyevent service.

Bug Fixes:

  • Prevent touchpad FN shortcuts from toggling the touchpad twice and desynchronizing the Control Center state.

Enhancements:

  • Delegate touchpad state changes to the system keyevent service while retaining shortcut OSD notifications.
  • Simplify the touchpad shortcut controller by removing its direct DBus touchpad control dependency.

1. Remove direct DBus TPadEnable toggling from dde-shortcut-tool
2. Keep OSD display (TouchpadToggle/TouchpadOn/TouchpadOff) only
3. Actual touchpad switch is handled by the system keyevent service,
   avoiding double-toggle that inverted the CC switch state

Log: FN touchpad shortcut no longer toggles state twice; control
center switch stays in sync

Influence:
1. On a laptop with touchpad, press FN touchpad hotkey, verify
   touchpad toggles exactly once and OSD shows correct icon
2. Check control center touchpad switch matches actual touchpad
   state after each FN press
3. Verify touchpad enable/disable still works from control center
   itself

fix: 触控板FN快捷键仅显示OSD,不再重复切换状态

1. 移除 dde-shortcut-tool 中直接通过 DBus 切换 TPadEnable 的逻辑
2. 保留 OSD 显示(TouchpadToggle/TouchpadOn/TouchpadOff)
3. 实际开关由系统 keyevent 服务处理,避免双重切换导致
   控制中心开关状态与实际相反

Log: 触控板FN快捷键不再重复切换状态,控制中心开关保持同步

Influence:
1. 在带触控板的笔记本上按 FN 触控板快捷键,验证触控板仅切换
   一次且 OSD 图标正确
2. 每次按 FN 后检查控制中心触控板开关与实际状态一致
3. 验证从控制中心直接开关触控板仍正常工作

PMS: BUG-377717
@sourcery-ai

sourcery-ai Bot commented Sep 20, 2026

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

Reviewer's Guide

The touchpad shortcut controller no longer changes the touchpad state itself; it only emits the appropriate OSD, preventing the FN key path from toggling twice while leaving actual state changes to the system keyevent service and preserving Control Center synchronization.

Sequence diagram for touchpad FN shortcut handling

sequenceDiagram
    actor User
    participant KeyEventService
    participant TouchPadController
    participant Touchpad
    participant OSD
    participant ControlCenter

    User->>KeyEventService: FN touchpad shortcut
    KeyEventService->>Touchpad: Toggle touchpad state
    KeyEventService->>TouchPadController: toggle()
    TouchPadController->>OSD: showOSD(TouchpadToggle)
    Touchpad-->>ControlCenter: Updated touchpad state
    ControlCenter-->>User: Synchronized switch state
Loading

File-Level Changes

Change Details Files
Make shortcut actions presentation-only and delegate touchpad state changes to the system keyevent service.
  • Remove the session D-Bus interface and direct TPadEnable reads/writes.
  • Keep toggle, enable, and disable actions mapped to the corresponding OSD signals.
  • Simplify lifecycle management now that the controller owns no D-Bus object.
src/plugin-qt/shortcut/tools/dde-shortcut-tool/touchpadcontroller.cpp
src/plugin-qt/shortcut/tools/dde-shortcut-tool/touchpadcontroller.h

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 found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="src/plugin-qt/shortcut/tools/dde-shortcut-tool/touchpadcontroller.cpp" line_range="62-64" />
<code_context>
-    }
-    
-    // Toggle always shows TouchpadToggle OSD (Go version logic)
+    // The shortcut only shows the OSD; the actual touchpad switch is handled
+    // elsewhere (e.g. the system keyevent service).
     showOSD("TouchpadToggle");
-    
     return true;
</code_context>
<issue_to_address>
**issue (bug_risk):** The `touchpad toggle`, `touchpad on`, and `touchpad off` commands now return success and display an OSD without changing the touchpad state. Any caller that invokes `dde-shortcut-tool` directly rather than going through the system keyevent service therefore receives a successful no-op, despite the command names and help text promising to toggle or enable/disable the touchpad.

**Triggers:** When a user, script, control-center integration, or custom shortcut invokes these dde-shortcut-tool actions directly.

**Suggested fix:** Keep the state-changing DBus implementation for explicit tool actions, or clearly separate the OSD-only keyevent path from the public `on`, `off`, and `toggle` commands and update their contract/help text.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment on lines +62 to 64
// The shortcut only shows the OSD; the actual touchpad switch is handled
// elsewhere (e.g. the system keyevent service).
showOSD("TouchpadToggle");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): The touchpad toggle, touchpad on, and touchpad off commands now return success and display an OSD without changing the touchpad state. Any caller that invokes dde-shortcut-tool directly rather than going through the system keyevent service therefore receives a successful no-op, despite the command names and help text promising to toggle or enable/disable the touchpad.

Triggers: When a user, script, control-center integration, or custom shortcut invokes these dde-shortcut-tool actions directly.

Suggested fix: Keep the state-changing DBus implementation for explicit tool actions, or clearly separate the OSD-only keyevent path from the public on, off, and toggle commands and update their contract/help text.

@fly602

fly602 commented Sep 20, 2026

Copy link
Copy Markdown
Contributor Author

/test all

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: fly602, yixinshark

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

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

AI 代码审查报告

提交信息

  • PR: linuxdeepin/dde-services#179
  • 标题: fix: touchpad FN key only shows OSD, no double toggle
  • 作者: fly602
  • 分支: master -> master
  • 修改文件: 2 个文件(+6 行, -101 行)
  • PMS: BUG-377717
  • 扫描时间: 2026-09-20 10:23:46

需求描述

修复触控板 FN 快捷键重复切换状态的问题:

  1. 移除 dde-shortcut-tool 中直接通过 DBus 切换 TPadEnable 的逻辑
  2. 保留 OSD 显示(TouchpadToggle/TouchpadOn/TouchpadOff)
  3. 实际开关由系统 keyevent 服务处理,避免双重切换导致控制中心开关状态与实际相反

总体评分

维度 得分 状态
语法逻辑 25/25 ✓ 语法正确,逻辑清晰
代码质量 25/25 ✓ 代码结构清晰,注释完整
代码性能 20/20 ✓ 性能良好,资源使用合理
代码安全 30/30 ✓ 存在0个安全漏洞
总分 100/100 优秀

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个


维度1: 语法逻辑(25分) - 得分: 25 ✓

语法正确,逻辑清晰

审查内容

文件: touchpadcontroller.cpp / touchpadcontroller.h

  1. 构造函数正确调用基类构造函数 BaseController(parent),函数体为空,语法正确
  2. 析构函数声明为 ~TouchPadController() override = default;,由于移除了 m_touchpadInterface 成员变量,使用 = default 是正确的做法
  3. toggle() 方法仅调用 showOSD("TouchpadToggle") 并返回 true,逻辑简洁正确
  4. setEnabled(bool enabled) 方法根据参数选择 OSD 信号名称并调用 showOSD(),逻辑正确
  5. 移除了 #include <QDBusReply>(不再需要),保留了 #include <QDBusInterface>#include <QDBusConnection>showOSD() 方法仍需使用)
  6. 移除了 class QDBusInterface; 前向声明和 QDBusInterface *m_touchpadInterface; 成员变量

结论

  • 无编译错误,无链接错误
  • 无空指针解引用风险(移除了指针成员)
  • 无内存泄漏风险(移除了动态分配的成员)
  • 边界条件处理完善(showOSD() 中检查了 DBus 接口有效性)
  • 代码变更与 commit message 目的一致:移除直接 DBus 切换逻辑,仅保留 OSD 显示

维度2: 代码质量(25分) - 得分: 25 ✓

代码结构清晰,注释完整

审查内容

  1. 注释完整性: toggle()setEnabled() 方法中添加了清晰的注释说明设计意图
  2. 代码重复: 无重复代码,toggle()setEnabled() 共用 showOSD() 方法
  3. 结构合理性: 移除了不必要的成员变量和构造/析构逻辑,代码更加简洁。函数长度适中,模块耦合度降低
  4. 调试信息清理: 无残留调试代码。移除了原有的 qDebug() 调试输出,符合代码清理目的
  5. 头文件清洁: 移除了不再需要的前向声明和成员变量声明

结论

  • 代码结构清晰,职责单一(仅负责 OSD 显示)
  • 注释准确描述了设计意图和原因
  • 无残留调试代码
  • 代码变更符合 commit message 描述的清理目的

维度3: 代码性能(20分) - 得分: 20 ✓

性能良好,资源使用合理

审查内容

  1. 资源管理优化: 移除了构造函数中创建持久 DBus 接口的操作,减少了对象创建时的资源消耗
  2. 无内存泄漏: 移除了 m_touchpadInterface 成员变量及其 delete 操作,消除了潜在的内存泄漏风险
  3. 无频繁系统调用: showOSD() 仅在用户按下 FN 键时调用,频率低,性能影响可忽略
  4. 栈上对象管理: showOSD() 中使用栈上 QDBusInterface 对象,函数结束时自动析构,无资源泄漏

结论

  • 移除持久 DBus 接口是性能优化
  • 无资源泄漏风险
  • 无性能瓶颈

维度4: 代码安全(30分) - 得分: 30 ✓

存在0个安全漏洞

审查内容

  1. 无命令注入风险: 代码不执行任何外部命令
  2. 无 SQL 注入风险: 代码不操作数据库
  3. 无路径遍历风险: 代码不处理文件路径
  4. 无硬编码密钥: 代码中无敏感信息
  5. 无用户输入未校验: execute() 方法中的 action 参数仅与硬编码字符串比较,无安全风险
  6. DBus 调用安全: DBus 服务路径和接口名称均为硬编码常量,不接受外部输入
  7. 减少了攻击面: 移除了对 org.deepin.dde.InputDevices1 的写操作(setProperty("TPadEnable", ...)),降低了 DBus 攻击面

结论

  • 存在0个安全漏洞
  • 无安全风险
  • 安全合规
  • 代码变更减少了 DBus 写操作,降低了安全风险

审查结论

本次 PR 的目的是修复触控板 FN 快捷键重复切换状态的问题。代码变更完全符合这一目的:

  1. 移除了 TouchPadController 中直接通过 DBus 切换 TPadEnable 属性的逻辑
  2. 保留了 OSD 显示功能(TouchpadToggle/TouchpadOn/TouchpadOff
  3. 实际触控板开关交由系统 keyevent 服务处理,避免双重切换
  4. 代码结构清晰,注释完整,无安全漏洞,无性能问题

审查通过,建议合并。

@fly602

fly602 commented Sep 20, 2026

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Sep 20, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot
deepin-bot Bot merged commit 157f500 into linuxdeepin:master Sep 20, 2026
5 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