Skip to content

feat: add one-shot shortcut capture interface#63

Merged
zccrs merged 1 commit into
linuxdeepin:masterfrom
wineee:keybind-1
May 27, 2026
Merged

feat: add one-shot shortcut capture interface#63
zccrs merged 1 commit into
linuxdeepin:masterfrom
wineee:keybind-1

Conversation

@wineee
Copy link
Copy Markdown
Member

@wineee wineee commented May 25, 2026

Added the treeland_shortcut_capture_v1 interface and a new
capture_next_key request to treeland_shortcut_manager_v2. The protocol
version is bumped to 2. The new request allows clients to capture the
next shortcut input once, bound to a specific wl_surface. The compositor
validates surface activity/focus and only captures valid shortcut key
releases; otherwise, it cancels with a detailed reason. This change
enables more interactive and user-friendly shortcut configuration
workflows, while ensuring protocol safety and clear error signaling.

增加了 treeland_shortcut_capture_v1 接口,并为
treeland_shortcut_manager_v2 添加了新的 capture_next_key 请求,协议版本
提升到 2。新请求允许客户端绑定特定 wl_surface 后一次性捕获下一个快捷键输
入。合成器会验证 surface 的活动或焦点状态,仅当捕获到有效的快捷键释放时
才发送 captured 事件,否则会以详细原因取消。这一改动提升了快捷键配置的交
互性和用户友好性,同时确保协议安全和清晰的错误信号。

@deepin-ci-robot
Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends the treeland_shortcut_manager_v2 Wayland protocol by adding a one-shot shortcut capture flow, enabling clients to request capture of the next shortcut input once (scoped to a specific wl_surface) with explicit cancellation reasons.

Changes:

  • Bumped treeland_shortcut_manager_v2 interface version from 1 to 2 and added a new protocol error invalid_surface (since v2).
  • Added capture_next_key request (since v2) to create a one-shot capture object bound to a provided surface.
  • Introduced new treeland_shortcut_capture_v1 interface with captured / cancelled terminal events and cancellation reasons.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread xml/treeland-shortcut-manager-v2.xml Outdated
Comment thread xml/treeland-shortcut-manager-v2.xml Outdated
Comment thread xml/treeland-shortcut-manager-v2.xml Outdated
Comment thread xml/treeland-shortcut-manager-v2.xml Outdated
@wineee wineee force-pushed the keybind-1 branch 2 times, most recently from ed785b2 to e67313a Compare May 25, 2026 08:19
@wineee wineee marked this pull request as ready for review May 25, 2026 08:33
@wineee wineee requested review from zccrs and zzxyb May 25, 2026 08:34
Comment thread xml/treeland-shortcut-manager-v2.xml Outdated
@wineee wineee force-pushed the keybind-1 branch 3 times, most recently from 15508dd to 94ab58d Compare May 26, 2026 10:51
Added the treeland_shortcut_capture_v1 interface and a new
capture_next_key request to treeland_shortcut_manager_v2. The protocol
version is bumped to 2. The new request allows clients to capture the
next shortcut input once, bound to a specific wl_surface. The compositor
validates surface activity/focus and only captures valid shortcut key
releases; otherwise, it cancels with a detailed reason. This change
enables more interactive and user-friendly shortcut configuration
workflows, while ensuring protocol safety and clear error signaling.

增加了 treeland_shortcut_capture_v1 接口,并为
treeland_shortcut_manager_v2 添加了新的 capture_next_key 请求,协议版本
提升到 2。新请求允许客户端绑定特定 wl_surface 后一次性捕获下一个快捷键输
入。合成器会验证 surface 的活动或焦点状态,仅当捕获到有效的快捷键释放时
才发送 captured 事件,否则会以详细原因取消。这一改动提升了快捷键配置的交
互性和用户友好性,同时确保协议安全和清晰的错误信号。
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的 Wayland 协议 XML 文件的 git diff。本次修改主要是将 treeland_shortcut_manager_v2 协议升级到 version 2,并新增了 capture_next_shortcut 请求以及配套的 treeland_shortcut_capture_v1 接口,用于实现单次快捷键捕获(通常用于快捷键设置界面)。

整体来看,协议的设计逻辑比较清晰,状态机定义也较为明确。但在语法规范、逻辑严谨性、代码质量和安全性方面,我有一些改进建议,请参考:

1. 语法与规范

  • 接口版本不一致
    treeland_shortcut_manager_v2 的版本从 1 升级到了 2,但新增的 treeland_shortcut_capture_v1 接口版本是 1。虽然 Wayland 协议允许不同接口有独立版本,但在同一个 XML 文件中,且由同一个 since="2" 的请求创建的接口,建议仔细确认:该捕获接口是否在未来有可能扩展?如果会,命名为 _v1 是合理的;如果这是一个与 manager v2 强绑定的独立协议,建议保持命名和版本的一致性,以免实现者混淆。
  • allow-null="true" 的约束缺失
    capture_next_shortcut 请求中,seat 参数设置了 allow-null="true",但描述中仅简单说明了“null to capture from any seat”。在 Wayland 协议中,允许为 null 通常需要更严格的语义约束。如果传入了一个不属于该客户端的 seat,或者当前处于不可用状态的 seat,合成器应该如何处理?建议在描述中明确补充非法 seat 对象的校验规则,或者声明合成器将忽略不相关的 seat。

2. 逻辑严谨性

  • 捕获流程的时序与竞态
    描述中提到:“The compositor must verify that the provided surface currently has keyboard focus or is active... If this check fails... must emit failed with reason not_active”。
    然而,Wayland 协议是异步的。从客户端发出请求,到合成器处理请求之间,焦点可能已经发生变化。建议明确说明:焦点检查应基于合成器处理该请求时的实时状态,而不是客户端发送请求时的状态。目前的描述容易让开发者产生歧义。
  • 修饰键的静默消费与状态不同步风险
    描述中提到:“Pure modifier-only key presses are silently consumed while waiting for a non-modifier key.”
    这是一个高风险逻辑。如果用户在捕获过程中按下了 Ctrl 键(被静默消费),然后放弃了捕获(比如点击了其他窗口导致焦点移出),此时 Ctrl 键的释放事件将不会被合成器捕获,也不会转发给客户端。这会导致客户端认为 Ctrl 键一直处于按下状态,造成修饰键状态卡死
    改进建议:明确要求合成器在捕获被中断、取消或对象被销毁时,必须合成伪造的 Key Release 事件给客户端,或者在此状态下恢复修饰键事件的正常转发,以防止客户端键盘状态机陷入死锁。

3. 代码质量与可维护性

  • 枚举值的语义重叠
    treeland_shortcut_capture_v1failed_reason 枚举中,定义了 aborted(value 2)和 interrupted(value 4)。
    根据描述,interrupted 是指“被非快捷键输入(如单独的字母键)打断”,而 aborted 是“被合成器策略中止”。在实际实现中,这两者的界限可能比较模糊。例如:用户按下了 Esc 键,这既可以说是“非快捷键输入”,也可以说是“合成器定义的中止策略”。
    改进建议:进一步细化描述,明确区分这两种失败场景的边界,或者考虑如果语义高度重合,是否可以合并,以减轻实现端的判断负担。

4. 安全性

  • Surface 归属校验与协议错误
    你在描述中正确地指出了:“The surface argument must be a wl_surface owned by the requesting client. Otherwise, the compositor will raise the invalid_surface protocol error.” 这是一个很好的安全措施,防止了跨进程的焦点劫持。
    但在 Wayland 中,更常见的安全隐患是使用已销毁的对象。如果客户端传入了一个已经通过 wl_surface.destroy 销毁的 surface ID,Wayland 核心协议通常会抛出全局致命错误。建议在描述中补充说明:如果传入的 surface 是一个无效(已销毁)的对象,合成器应直接遵循 Wayland 核心协议处理,而无需再抛出 invalid_surface 业务错误,避免与核心协议冲突。
  • 拒绝服务攻击风险
    客户端可以无限次调用 capture_next_shortcut 创建 capture 对象,如果合成器为每个对象都分配了较重的资源(如注册全局键盘拦截器),恶意客户端可能会耗尽合成器内存。
    改进建议:在 capture_next_shortcut 的描述中增加一条限制,例如:“The compositor may raise a busy failed event if the client requests a capture while another capture from the same client is still pending”,或者限制单个客户端允许同时存在的 pending capture 数量。目前的 busy 枚举仅在 failed_reason 中,且描述为“another capture is in progress”,建议明确这是全局限制还是单客户端限制。

改进后的 XML 片段建议参考:

针对上述部分问题,我为你修改了 capture_next_shortcut 的描述部分,供你参考:

        <request name="capture_next_shortcut" since="2">
            <description summary="capture the next shortcut once">
                Request the compositor to capture the next shortcut input once,
                bound to the provided surface.

                The capture object is created by this request via new_id.
                The compositor performs validation after receiving the request,
                and may immediately emit a terminal event on the newly created object.
                Clients must be prepared to handle a capture object that is
                failed immediately after creation.

                The surface argument must be a wl_surface owned by the requesting client.
                Otherwise, the compositor will raise the `invalid_surface` protocol error.
                If the surface ID is invalid or already destroyed, the compositor
                should handle it as a Wayland core object violation.

                The compositor must verify that the provided surface currently has
                keyboard focus or is active at the moment the compositor processes 
                this request. If this check fails, the compositor must emit `failed` 
                with reason `not_active` on the created object.

                The compositor monitors both key_press and key_release events during
                capture. The shortcut candidate is established on key_press of a valid
                non-modifier key; the terminal event (captured or failed) is sent on
                key_release of that non-modifier key. Pure modifier-only key presses
                (e.g. Ctrl, Alt, Shift) are silently consumed while waiting for a
                non-modifier key. Auto-repeat events are consumed and ignored.

                If the capture is cancelled, interrupted, or the object is destroyed
                while modifier keys are physically held down, the compositor must 
                ensure the client's keyboard state remains consistent, for example 
                by synthesizing key release events or resuming normal event delivery 
                for those modifiers.

                If the next intercepted input is not a valid shortcut input
                (for example: unmodified alphanumeric keys such as a, b, c, d,
                1, 2, 3, unmodified Escape, or pointer button and wheel events),
                the compositor must fail capture with reason `interrupted`.
                For either `interrupted` failure or successful `captured`,
                the triggering input event must be consumed by the compositor
                and must not be forwarded to the client.

                The compositor creates a treeland_shortcut_capture_v1 object and starts
                a one-shot capture flow for this request.

                To prevent resource exhaustion, the compositor may emit `failed`
                with reason `busy` if the client requests a capture while a
                pending capture from the same client already exists.

                This request does not modify existing shortcut bindings.
                This request does not require acquire.
            </description>
            <arg name="surface" type="object" interface="wl_surface"
                 summary="surface used for focus or active validation"/>
            <arg name="seat" type="object" interface="wl_seat" allow-null="true"
                 summary="seat to capture from, or null to capture from any seat. Must be a valid seat owned by the client if non-null"/>
            <arg name="capture" type="new_id" interface="treeland_shortcut_capture_v1"
                 summary="one-shot capture object"/>
        </request>

希望这些建议能帮助你完善 Wayland 协议的设计!如果有任何疑问,欢迎继续探讨。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: wineee, zccrs

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

@zccrs zccrs merged commit 69225c6 into linuxdeepin:master May 27, 2026
7 of 9 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