Skip to content

Conversation

@fly602
Copy link
Contributor

@fly602 fly602 commented Jan 14, 2026

处理拖拽窗口的命令参数不正确,导致释放鼠标事件不对.

Log: 修改触控屏鼠标参数
PMS: BUG-304177 311961 321777 346073
Influence: 触控屏拖拽窗口

Summary by Sourcery

Bug Fixes:

  • Correct xdotool mouseup command to properly release the right mouse button during touch-screen window dragging.

@sourcery-ai
Copy link

sourcery-ai bot commented Jan 14, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adjusts xdotool mouse down/up command invocation for touch-screen window dragging, correcting both the command string and how it is executed.

Sequence diagram for updated touch drag mouse down/up handling

sequenceDiagram
    actor User
    participant Touchscreen as Touchscreen
    participant Manager as Manager
    participant Exec as os_exec
    participant Xdotool as xdotool
    participant WM as WindowManager

    User->>Touchscreen: Drag window gesture
    Touchscreen->>Manager: gestureEvent
    Manager->>Exec: exec.Command("xdotool mousedown 3")
    Exec->>Xdotool: run mousedown 3
    Xdotool->>WM: start moving window

    User-->>Touchscreen: Release finger
    Touchscreen-->>Manager: gestureEndEvent
    Manager->>Exec: exec.Command("xdotool mouseup 3")
    Exec->>Xdotool: run mouseup 3
    Xdotool->>WM: stop moving window
Loading

Class diagram for Manager gesture mouse actions

classDiagram
    class Manager {
        +doToggleLaunchpad() error
        +doXdotoolsMouseDown() error
        +doXdotoolsMouseUp() error
    }
Loading

File-Level Changes

Change Details Files
Fix mouse down command invocation for initiating drag via xdotool.
  • Change exec.Command invocation to call the xdotool binary directly rather than via a shell
  • Maintain use of button 3 for the mousedown event
gesture1/gesture_action.go
Correct mouse up command so drag is properly released on touch screens.
  • Fix command from using mousedown to mouseup for button release
  • Align mouseup invocation style with mousedown by calling xdotool directly
gesture1/gesture_action.go

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

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

  • When switching from exec.Command("/bin/sh", "-c", cmd) to direct execution, exec.Command(cmd) will treat the entire string as the binary name; instead, split the command and arguments, e.g., exec.Command("xdotool", "mousedown", "3") and similarly for mouseup.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- When switching from `exec.Command("/bin/sh", "-c", cmd)` to direct execution, `exec.Command(cmd)` will treat the entire string as the binary name; instead, split the command and arguments, e.g., `exec.Command("xdotool", "mousedown", "3")` and similarly for `mouseup`.

## Individual Comments

### Comment 1
<location> `gesture1/gesture_action.go:169` </location>
<code_context>
 func (m *Manager) doXdotoolsMouseDown() error {
 	cmd := "xdotool mousedown 3"
-	out, err := exec.Command("/bin/sh", "-c", cmd).CombinedOutput()
+	out, err := exec.Command(cmd).CombinedOutput()
 	if err != nil {
 		return fmt.Errorf("%s", string(out))
</code_context>

<issue_to_address>
**issue (bug_risk):** exec.Command is given a single string with spaces, which will be treated as the binary name rather than command + args

With `cmd := "xdotool mousedown 3"`, `exec.Command(cmd)` will try to execute a binary literally named `"xdotool mousedown 3"`. To mimic the previous behavior without the shell, pass args separately, e.g. `exec.Command("xdotool", "mousedown", "3").CombinedOutput()`. Do the same for the `mouseup` call.
</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.

处理拖拽窗口的命令参数不正确,导致释放鼠标事件不对.

Log: 修改触控屏鼠标参数
PMS: BUG-304177 311961 321777 346073
Influence: 触控屏拖拽窗口
@deepin-ci-robot
Copy link

deepin pr auto review

这段代码的修改主要涉及对 exec.Command 调用方式的优化。以下是对修改前后的代码进行的详细审查,包括语法逻辑、代码质量、性能和安全性方面的分析:

1. 代码审查总结

修改内容:将 exec.Command("/bin/sh", "-c", cmd) 修改为直接调用 exec.Command("xdotool", "mousedown", "3")(以及类似的 mouseup 调用)。

总体评价:这是一个非常好的改进,显著提升了代码的安全性和健壮性。


2. 详细分析

A. 安全性 —— 显著提升

  • 修改前 (/bin/sh -c)
    • Shell 注入风险:当使用 exec.Command("/bin/sh", "-c", cmd) 时,如果 cmd 字符串中包含用户可控的输入(例如变量),攻击者可以通过构造特殊字符(如 ;, &, |, $() 等)来执行任意系统命令。
    • 即便在这个特定场景中 cmd 是硬编码的,这种写法也是一种不良习惯,容易在后续代码维护中引入漏洞。
  • 修改后 (直接调用)
    • 参数分离:使用 exec.Command("xdotool", "mousedown", "3") 时,参数列表被显式地分隔。Go 的 exec 包会直接将参数传递给 execve 系统调用,中间不经过 Shell 解析
    • 消除注入风险:这种写法天然免疫 Shell 注入攻击,即使未来将 3 替换为变量,也是安全的。

B. 代码质量与可读性 —— 提升

  • 意图更明确
    • 修改后的代码直接展示了要执行的程序及其参数,阅读者一眼就能看出是在调用 xdotool 并传递参数 mousedown3
    • 修改前的代码将命令封装在字符串中,需要额外的脑力去解析字符串内容。
  • 减少字符串操作:去除了中间变量 cmd,使得代码更简洁,减少了不必要的内存分配(虽然微乎其微,但属于良好实践)。

C. 性能 —— 轻微提升

  • 减少进程开销
    • 修改前:系统需要启动 /bin/sh 进程,然后由 sh 解析命令并启动 xdotool 进程。这涉及两个进程的创建和上下文切换。
    • 修改后:系统直接启动 xdotool 进程。省去了 Shell 进程的启动开销和命令解析时间。
  • 解析效率:避免了 Shell 对命令字符串的词法分析和语法分析过程。

D. 语法逻辑 —— 修正了一个潜在的 Bug

  • Bug 修复
    • doXdotoolsMouseUp 函数的修改前代码中,命令字符串写的是 xdotool mousedown 3(按下),但函数名和注释意图是 mouseup(抬起)。这是一个明显的复制粘贴错误。
    • 修改后的代码正确地将命令改为了 xdotool mouseup 3,修复了逻辑错误。

3. 改进建议

虽然当前的修改已经很好,但在错误处理方面还有优化空间:

当前错误处理

if err != nil {
    return fmt.Errorf("%s", string(out))
}

建议改进
建议将原始的 err 信息也包含在返回的错误中,这样不仅能看到标准输出/错误流的内容,还能看到 Go 层面返回的系统错误(例如 "executable file not found")。

if err != nil {
    // 使用 fmt.Errorf 的 %w 动词包装错误,便于调用方使用 errors.Is/As 进行判断
    return fmt.Errorf("xdotool failed: %w, output: %s", err, string(out))
}

4. 结论

这次修改非常正确,主要收益在于:

  1. 消除了 Shell 解析带来的安全隐患
  2. 修复了 doXdotoolsMouseUp 中命令错误的 Bug
  3. 减少了不必要的 Shell 进程启动,略微提升性能
  4. 代码更加清晰、符合 Go 语言惯用写法

建议采纳该修改,并参考上述关于错误处理的建议进一步完善代码。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, fly602, justforlxz

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

@fly602 fly602 merged commit ed70ec0 into linuxdeepin:master Jan 15, 2026
15 of 17 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.

4 participants