Skip to content

feat: Add support for modifying the plugin popup cursor shape via the…#1434

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
yixinshark:feat-addCursorShape
Mar 3, 2026
Merged

feat: Add support for modifying the plugin popup cursor shape via the…#1434
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
yixinshark:feat-addCursorShape

Conversation

@yixinshark
Copy link
Contributor

@yixinshark yixinshark commented Feb 5, 2026

… protocol.

add support in plugin-manager-v1.xml

Log: Add support for modifying the plugin popup cursor shape via the protocol.

Summary by Sourcery

Add protocol and UI support for plugin popups to request and update the cursor shape.

New Features:

  • Allow plugin popups to request cursor shape changes via the plugin manager protocol.
  • Expose a configurable cursor shape property on shell surface items used for plugin popups.

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 5, 2026

Reviewer's Guide

Adds protocol and implementation support for plugin popups to request cursor shape changes, plumbing the request from the Wayland plugin manager extension through to the QML shell surface proxy and exposing it as a cursorShape property with bounds checking.

Sequence diagram for plugin popup cursor shape update flow

sequenceDiagram
    actor PluginClient
    participant WaylandPluginProtocol
    participant PluginPopup
    participant ShellSurfaceItemProxy_qml as ShellSurfaceItemProxy
    participant HoverHandler_qml as HoverHandler

    PluginClient->>WaylandPluginProtocol: plugin_popup_set_cursor(cursor_shape)
    WaylandPluginProtocol->>PluginPopup: plugin_popup_set_cursor(resource, cursor_shape)
    PluginPopup->>PluginPopup: plugin_popup_set_cursor(resource, cursor_shape)
    PluginPopup-->>ShellSurfaceItemProxy_qml: cursorShapeRequested(cursor_shape)
    ShellSurfaceItemProxy_qml->>ShellSurfaceItemProxy_qml: onCursorShapeRequested(cursor_shape)
    ShellSurfaceItemProxy_qml->>ShellSurfaceItemProxy_qml: validate cursor_shape range 0-21,24,25
    alt cursor_shape out of range
        ShellSurfaceItemProxy_qml->>ShellSurfaceItemProxy_qml: root.cursorShape = Qt.ArrowCursor
    else cursor_shape in range
        ShellSurfaceItemProxy_qml->>ShellSurfaceItemProxy_qml: root.cursorShape = cursor_shape
    end
    ShellSurfaceItemProxy_qml->>HoverHandler_qml: cursorShape = root.cursorShape
Loading

Class diagram for PluginPopup and ShellSurfaceItemProxy cursor shape support

classDiagram
    class PluginPopup {
        +plugin_popup_source_size(resource: Resource, width: int32_t, height: int32_t)
        +plugin_popup_set_cursor(resource: Resource, cursor_shape: int32_t)
        +plugin_popup_destroy_resource(resource: Resource)
        +plugin_popup_destroy(resource: Resource)
        +cursorShapeRequested(cursorShape: int)
        -m_manager: PluginManager*
    }

    class ShellSurfaceItemProxy {
        +inputEventsEnabled: bool
        +hovered: bool
        +pressed: bool
        +cursorShape: int
        +onCursorShapeRequested(cursorShape: int)
    }

    class HoverHandler_qml {
        +hovered: bool
        +cursorShape: int
    }

    ShellSurfaceItemProxy --> HoverHandler_qml: sets cursorShape
    PluginPopup --> ShellSurfaceItemProxy: emits cursorShapeRequested
Loading

Flow diagram for cursorShape bounds checking in ShellSurfaceItemProxy

flowchart TD
    A[onCursorShapeRequested cursorShape param received] --> B[Check if cursorShape < 0 or cursorShape > 25]
    B -->|yes| C[Set root.cursorShape to Qt.ArrowCursor]
    B -->|no| D[Set root.cursorShape to cursorShape]
    C --> E[HoverHandler.cursorShape uses root.cursorShape]
    D --> E[HoverHandler.cursorShape uses root.cursorShape]
Loading

File-Level Changes

Change Details Files
Wire plugin popup cursor-shape requests from the Wayland protocol into the QML shell surface proxy and expose a configurable cursorShape with validation.
  • Add a cursorShape int property on the shell surface proxy root item with default Qt.ArrowCursor
  • Bind the HoverHandler cursorShape to the root cursorShape property so hover cursor reflects requested value
  • Introduce an onCursorShapeRequested handler in QML that logs the value, validates it against the Qt::CursorShape range, and falls back to ArrowCursor when out of bounds
  • Extend PluginPopup interface to handle the new plugin_popup_set_cursor request and emit a cursorShapeRequested signal carrying the requested cursor shape
panels/dock/tray/ShellSurfaceItemProxy.qml
panels/dock/pluginmanagerextension.cpp
panels/dock/pluginmanagerextension_p.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

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:

  • The onCursorShapeRequested handler in ShellSurfaceItemProxy.qml is currently just a standalone function; unless it is explicitly connected to the PluginPopup::cursorShapeRequested signal elsewhere, this callback will never run—consider wiring the signal to this handler or using a Connections object in the QML to make the linkage explicit.
  • The console.log("onCursorShapeRequested:", cursorShape) in QML will spam logs on every cursor update; consider removing it or guarding it behind a runtime debug flag to avoid excessive logging in normal operation.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The `onCursorShapeRequested` handler in `ShellSurfaceItemProxy.qml` is currently just a standalone function; unless it is explicitly connected to the `PluginPopup::cursorShapeRequested` signal elsewhere, this callback will never run—consider wiring the signal to this handler or using a `Connections` object in the QML to make the linkage explicit.
- The `console.log("onCursorShapeRequested:", cursorShape)` in QML will spam logs on every cursor update; consider removing it or guarding it behind a runtime debug flag to avoid excessive logging in normal operation.

## Individual Comments

### Comment 1
<location> `panels/dock/tray/ShellSurfaceItemProxy.qml:117-122` </location>
<code_context>
             })
         }
+
+        function onCursorShapeRequested(cursorShape)
+        {
+            console.log("onCursorShapeRequested:", cursorShape)
+            // Qt::CursorShape range is 0-21, plus 24 (BitmapCursor) and 25 (CustomCursor).
+            // We set a default if the value is out of logical bounds.
+            if (cursorShape < 0 || cursorShape > 25) {
+                root.cursorShape = Qt.ArrowCursor
+            } else {
</code_context>

<issue_to_address>
**issue (bug_risk):** The cursorShape bounds check lets invalid enum values 22 and 23 through.

The check only filters values < 0 or > 25, so 22 and 23 are still accepted even though the comment marks them as invalid. If Wayland ever sends 22 or 23, this could cause undefined cursor behavior. Please tighten the condition to only accept 0–21, 24, and 25 (e.g. `if ((cursorShape >= 0 && cursorShape <= 21) || cursorShape == 24 || cursorShape == 25)`), or map any other value to a safe default.
</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.

@yixinshark
Copy link
Contributor Author

linuxdeepin/dde-tray-loader#424
这里修改协议,添加支持。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, 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-bot
Copy link

deepin-bot bot commented Feb 5, 2026

TAG Bot

New tag: 2.0.29
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1436

Copy link

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 adds support for plugin popups to request cursor shape changes via the Wayland protocol. The feature allows plugins to communicate their desired cursor shape to the compositor, which then updates the UI accordingly.

Changes:

  • Added protocol support for cursor shape requests in plugin popups
  • Implemented cursor shape handling in the QML UI layer with validation
  • Updated dependency version to require protocol support in dde-tray-loader-dev

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
panels/dock/tray/ShellSurfaceItemProxy.qml Added cursorShape property and signal handler to process cursor shape requests from plugins
panels/dock/pluginmanagerextension_p.h Declared new protocol method and signal for cursor shape requests
panels/dock/pluginmanagerextension.cpp Implemented plugin_popup_set_cursor to emit cursorShapeRequested signal
debian/control Updated dde-tray-loader-dev dependency to require version > 2.0.24 for protocol support

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

@deepin-bot
Copy link

deepin-bot bot commented Feb 27, 2026

TAG Bot

New tag: 2.0.30
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1458

… protocol.

add support in plugin-manager-v1.xml

Log: Add support for modifying the plugin popup cursor shape via the protocol.
@yixinshark yixinshark force-pushed the feat-addCursorShape branch from 1447b43 to ec06e49 Compare March 3, 2026 02:39
@deepin-ci-robot
Copy link

deepin pr auto review

经过对提供的 git diff 的审查,以下是关于语法逻辑、代码质量、代码性能和代码安全的详细审查意见和改进建议:

1. 依赖版本修改 (debian/control)

  • 修改内容:将 dde-tray-loader-dev 的依赖从 (>= 1.99.14) 修改为 (> 2.0.24)
  • 审查意见
    • 语法逻辑:符合 Debian 控制文件的语法规范。
    • 代码质量:依赖版本号的变更通常意味着 API 的不兼容更新或新特性的引入。从 1.99.14 跳跃到 2.0.24 暗示了一次重大的版本更新。
    • 潜在风险:使用严格大于号 > 而非 >= 是一个比较激进的选择。如果上游发布了 2.0.25,当前版本可以安装;但如果上游修复了一个 bug 并发布了 2.0.24.1(假设遵循某些打包规则)或者仅仅是 2.0.24 被认为是稳定版,严格大于 2.0.24 可能会导致无法安装最新的补丁版本(如果版本号格式允许)。
    • 改进建议
      • 确认 2.0.24 之后是否有不兼容的破坏性更新。如果没有,建议使用 (>= 2.0.24) 以确保能获得该版本及后续的补丁修复。
      • 如果确实需要排除 2.0.24 本身(例如该版本有特定 bug),请确保代码逻辑中已经处理了该 bug 或依赖了后续版本的新特性。

2. C++ 后端实现 (panels/dock/pluginmanagerextension.cpppluginmanagerextension_p.h)

  • 修改内容:添加了 plugin_popup_set_cursor 方法,用于接收 Wayland 协议传来的光标形状,并通过信号 cursorShapeRequested 发送出去。
  • 审查意见
    • 语法逻辑:代码逻辑清晰,符合 Qt/Wayland 的扩展编写规范。
    • 代码质量
      • 使用了 Q_UNUSED(resource) 表明该参数在当前实现中未使用,这是良好的编程习惯,避免了编译器警告。
    • 代码安全
      • 参数校验缺失:在 C++ 端直接透传了 cursor_shape 整数。虽然 QML 端做了校验,但在 C++ 层(协议接口层)进行一次基础校验会更健壮,防止无效或恶意的整数值在组件间传递。
    • 改进建议
      • 虽然不是必须的,但建议在 C++ 头文件中为 cursorShape 参数添加注释,说明其预期的取值范围(例如对应 Qt::CursorShape 的枚举值)。

3. QML 前端实现 (panels/dock/tray/ShellSurfaceItemProxy.qml)

  • 修改内容:在 QML 中添加了 cursorShape 属性,监听 C++ 发送的 cursorShapeRequested 信号,并根据值更新 HoverHandler 的光标形状。
  • 审查意见
    • 语法逻辑:QML 语法正确,信号与槽的连接逻辑清晰。
    • 代码质量
      • 调试日志console.log("onCursorShapeRequested:", cursorShape) 在每次光标变化时都会打印。这在开发阶段是有用的,但在生产环境中可能会产生大量的日志噪音,影响性能。
      • 边界检查:代码中包含了对 cursorShape 的范围检查(0-25),这是一个很好的防御性编程实践。
    • 代码性能
      • HoverHandlercursorShape 属性绑定是高效的,只有在 root.cursorShape 改变时才会触发更新。
    • 代码安全
      • 枚举值硬编码:注释中提到 Qt::CursorShape range is 0-21...。这种硬编码的魔法数字维护性较差。如果 Qt 在未来版本中扩展了枚举值,这里就需要手动修改。
    • 改进建议
      1. 移除或条件化日志:建议移除 console.log,或者使用 Qt.debug() 并配合编译/运行时标志来控制是否输出。
      2. 使用枚举而非魔法数字:如果可能,尽量避免硬编码 25。虽然 QML 直接访问 C++ 枚举有时比较麻烦,但可以定义一个常量或者通过 C++ 导出一个最大值常量。
      3. 优化边界检查逻辑:目前的检查逻辑是 if (cursorShape < 0 || cursorShape > 25)。由于 Qt.ArrowCursor (通常是 0) 是默认值,可以简化为:
        if (cursorShape >= 0 && cursorShape <= 25) {
            root.cursorShape = cursorShape
        } else {
            root.cursorShape = Qt.ArrowCursor // 保持默认值
        }
        这样逻辑稍微更符合"只有合法才更新"的思维。

总结

这段代码整体质量不错,实现了在 Wayland 插件弹窗中动态设置光标形状的功能。主要的改进点在于:

  1. 依赖管理:复查 debian/control 中的版本号要求,确保 > 是必要的,或者改为 >=
  2. 维护性:移除 QML 中的调试日志;避免在注释和代码中硬编码枚举值的边界数字。
  3. 健壮性:虽然 QML 做了校验,但在 C++ 接口层增加参数有效性说明或校验会更好。

@yixinshark
Copy link
Contributor Author

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Mar 3, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 630b763 into linuxdeepin:master Mar 3, 2026
9 of 12 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