Skip to content

fix(tray): add fallback chain for SNI tray left-click Activate failure - #513

Closed
mhduiy wants to merge 1 commit into
masterfrom
agent/pms-bug-bot/c5549a9615f9
Closed

mhduiy wants to merge 1 commit into
masterfrom
agent/pms-bug-bot/c5549a9615f9

Conversation

@mhduiy

@mhduiy mhduiy commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

根因分析

SNI 托盘左键点击处理(sniprotocolhandler.cpp eventFilter)对 Activate DBus 调用采用发后即忘方式,无错误处理、无 fallback。当应用(如百度网盘,Chromium 系)未实现 StatusNotifierItem.Activate 方法时,DBus 调用返回 org.freedesktop.DBus.Error.UnknownMethod 错误后被静默丢弃,导致点击托盘图标无反应。

修复方案

  1. ItemIsMenu 预检:左键点击时先检查 ItemIsMenu 属性,若为 true 则直接弹 DBus 菜单(符合 SNI 规范)。
  2. QDBusPendingCallWatcher 错误监听:用 QDBusPendingCallWatcher 异步监听 Activate 调用结果。
  3. 分级 fallback:Activate 失败 → 尝试 SecondaryActivate → 失败则弹 DBus 菜单或调用 ContextMenu。
  4. DRY 重构:将右键菜单显示逻辑提取为 showSniMenu(),左键 fallback 复用。

改动安全评估

  • 风险等级:中风险
  • 不改变函数签名,不改变公开 API
  • 右键菜单行为不变(仅重构提取方法)
  • Activate 成功的应用行为不变
  • watcher 父对象为 this,析构时自动清理

影响范围

  • 所有使用 SNI 协议的系统托盘应用
  • Activate 成功的应用:行为不变
  • Activate 失败的应用(百度网盘等):从"无反应"变为"有 fallback 响应"
  • ItemIsMenu=true 的托盘项:左键从尝试 Activate 改为直接弹菜单

PMS Bug

https://pms.uniontech.com/bug-view-360241.html

Summary by Sourcery

Improve SNI tray activation reliability by honoring menu items and falling back from Activate to SecondaryActivate and the context menu.

New Features:

  • Add resilient SNI tray left-click handling with menu-aware activation fallbacks.

Bug Fixes:

  • Prevent SNI tray clicks from becoming unresponsive when Activate or SecondaryActivate is unavailable or fails.

Enhancements:

  • Preserve right-click menu behavior while sharing menu display logic with left-click fallbacks.
  • Handle asynchronous DBus activation errors and clean up pending watchers automatically.

When an SNI tray item does not implement the Activate method (e.g.
Chromium-based apps like Baidu Netdisk), the left-click DBus call
fails silently with no fallback, leaving the tray icon unresponsive.

Add QDBusPendingCallWatcher to monitor the Activate call result. On
failure, try SecondaryActivate, then fall back to showing the DBus
menu or calling ContextMenu. Also check ItemIsMenu property before
attempting Activate — items declaring ItemIsMenu=true should show
their menu directly.

The right-click menu display logic is extracted into showSniMenu()
to avoid duplication between right-click and left-click fallback
paths.

Log: SNI托盘左键点击Activate无响应时增加分级fallback
Bug: https://pms.uniontech.com/bug-view-360241.html
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 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

@sourcery-ai

sourcery-ai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR makes SNI tray left-click handling resilient by checking menu-only items, asynchronously detecting Activate failures, and applying an Activate → SecondaryActivate → menu fallback chain. It also extracts shared DBus menu presentation logic without changing public APIs or the existing successful-activation and right-click paths.

Sequence diagram for resilient SNI tray left-click activation

sequenceDiagram
    participant User
    participant Handler as SniTrayProtocolHandler
    participant SNI as StatusNotifierItem
    participant Menu as DBusMenu

    User->>Handler: MouseButtonRelease LeftButton
    alt ItemIsMenu is true
        Handler->>SNI: itemIsMenu()
        Handler->>Handler: showSniMenu(clickPos)
        Handler->>Menu: updateMenu(menu)
        Handler->>Menu: show()
    else ItemIsMenu is false
        Handler->>Handler: tryActivate(clickPos)
        Handler->>SNI: Activate(0, 0)
        SNI-->>Handler: QDBusPendingCallWatcher finished
        alt Activate succeeds
            Handler->>Handler: deleteLater()
        else Activate fails
            Handler->>Handler: trySecondaryActivate(clickPos)
            Handler->>SNI: SecondaryActivate(0, 0)
            SNI-->>Handler: QDBusPendingCallWatcher finished
            alt SecondaryActivate succeeds
                Handler->>Handler: deleteLater()
            else SecondaryActivate fails
                Handler->>Handler: showSniMenu(clickPos)
                Handler->>Menu: updateMenu(menu)
                Handler->>Menu: show()
            end
        end
    end
Loading

File-Level Changes

Change Details Files
Adds ordered fallback handling for failed SNI activation requests.
  • Checks ItemIsMenu and opens the DBus menu directly for left clicks when set.
  • Monitors asynchronous Activate and SecondaryActivate DBus calls for errors.
  • Falls back from Activate to SecondaryActivate, then to the DBus menu.
  • Preserves XDG activation-token flow before attempting activation.
plugins/application-tray/sniprotocolhandler.cpp
plugins/application-tray/sniprotocolhandler.h
Centralizes SNI menu display behavior for reuse across mouse-button paths.
  • Extracts existing right-click menu setup and positioning into showSniMenu().
  • Reuses the extracted method for left-click fallback while preserving right-click behavior.
plugins/application-tray/sniprotocolhandler.cpp
plugins/application-tray/sniprotocolhandler.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="plugins/application-tray/sniprotocolhandler.cpp" line_range="325-327" />
<code_context>
         if (event->type() == QEvent::MouseButtonRelease) {
             QMouseEvent* mouseEvent = static_cast<QMouseEvent*>(event);
             if (mouseEvent->button() == Qt::LeftButton) {
+                if (m_sniInter->itemIsMenu()) {
+                    showSniMenu(mouseEvent->pos());
+                    return false;
+                }
+
</code_context>
<issue_to_address>
**issue (performance):** `itemIsMenu()` performs a synchronous D-Bus property read from the GUI event-filter path, so an unresponsive or disconnected SNI provider blocks the tray UI until the interface timeout expires before processing the click.

**Triggers:** When the SNI service does not respond to the `ItemIsMenu` property request.

**Suggested fix:** Read and cache `ItemIsMenu` asynchronously during item initialization, or avoid a synchronous property lookup in `eventFilter`.
</issue_to_address>

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

Comment on lines +325 to +327
if (m_sniInter->itemIsMenu()) {
showSniMenu(mouseEvent->pos());
return false;

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 (performance): itemIsMenu() performs a synchronous D-Bus property read from the GUI event-filter path, so an unresponsive or disconnected SNI provider blocks the tray UI until the interface timeout expires before processing the click.

Triggers: When the SNI service does not respond to the ItemIsMenu property request.

Suggested fix: Read and cache ItemIsMenu asynchronously during item initialization, or avoid a synchronous property lookup in eventFilter.

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 94 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 未发现安全漏洞,代码结构清晰,fallback 链设计合理,有效修复了百度网盘等 Chromium-based 应用托盘左键点击无响应的问题。存在轻微的代码重复和快速点击场景下的健壮性改进空间。
关联Bug PMS #360241 - 第三方应用百度网盘最小化后,点击其托盘窗口面板的应用图标无反应
审查文件 plugins/application-tray/sniprotocolhandler.cpp, plugins/application-tray/sniprotocolhandler.h

🔍 详细分析

1. 语法逻辑 ✓ (22/25)

评价: 语法正确,逻辑清晰 ✓

潜在问题:

  1. tryActivate 和 trySecondaryActivate(sniprotocolhandler.cpp:402-424)中快速连续点击可能触发多个并发 QDBusPendingCallWatcher,每个独立启动 fallback 链。如果多个 DBus 回复同时失败,showSniMenu() 将被多次调用,导致重叠或重复菜单。属于边界条件处理不当(-3分)。

建议:

  • 考虑在 tryActivate 中添加重入保护,如使用成员变量 m_activateWatcher 跟踪当前 watcher,新点击时取消之前的请求,或使用布尔标志 m_activationInProgress 防止重入

2. 代码质量 ✓ (22/25)

评价: 代码结构清晰,注释完整 ✓

潜在问题:

  1. tryActivate 和 trySecondaryActivate(sniprotocolhandler.cpp:402-424)两个函数结构高度相似,存在代码重复。两者都创建 QDBusPendingCallWatcher、连接 finished 信号、检查 reply.isError()、调用 fallback。可考虑提取为通用 helper 函数(-2分)。
  2. 新增方法 tryActivate、trySecondaryActivate 缺少函数注释说明其功能和 fallback 链设计意图。showSniMenu 保留了原有注释,但新方法应补充说明(-1分)。

建议:

  • 为 tryActivate 和 trySecondaryActivate 添加简要注释说明 fallback 链设计
  • 考虑将 tryActivate 和 trySecondaryActivate 的公共逻辑提取为模板方法,减少重复代码

3. 代码性能 ✓ (20/20)

评价: 性能良好,资源使用合理 ✓

潜在问题:
✓ 未发现性能问题

说明:

  1. 使用异步 QDBusPendingCallWatcher 避免阻塞主线程,设计合理
  2. QDBusPendingCallWatcher 通过 deleteLater() 正确管理内存,无资源泄漏
  3. clickPos 通过 const 引用传递,避免不必要的拷贝

4. 代码安全 ✓ (30/30)

存在0个安全漏洞,安全合规

安全漏洞详情:
✅ 未发现安全漏洞

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

说明:

  • 无用户输入直接处理,无注入风险
  • 无硬编码密钥或敏感信息
  • DBus 调用使用 SNI 标准接口,无路径遍历风险
  • 无缓冲区溢出风险

🔧 改进建议代码示例

// sniprotocolhandler.h - 添加成员变量防止重入
private:
    QPointer<QDBusPendingCallWatcher> m_activateWatcher;

// sniprotocolhandler.cpp - 改进 tryActivate 添加重入保护
void SniTrayProtocolHandler::tryActivate(const QPoint &clickPos)
{
    // 取消之前进行中的 watcher
    if (m_activateWatcher) {
        delete m_activateWatcher;
        m_activateWatcher = nullptr;
    }

    m_activateWatcher = new QDBusPendingCallWatcher(m_sniInter->Activate(0, 0), this);
    connect(m_activateWatcher, &QDBusPendingCallWatcher::finished, this, [this, clickPos]() {
        QDBusPendingReply<> reply = *m_activateWatcher;
        m_activateWatcher->deleteLater();
        m_activateWatcher = nullptr;
        if (reply.isError()) {
            trySecondaryActivate(clickPos);
        }
    });
}

📋 审查总结

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

审查结论: 本次提交针对 PMS Bug #360241(百度网盘托盘左键点击无响应)进行了有效修复。通过添加 QDBusPendingCallWatcher 监控 Activate 调用结果,并实现分级 fallback 链(Activate → SecondaryActivate → showSniMenu),解决了 Chromium-based 应用未实现 Activate 方法时托盘图标无响应的问题。同时将右键菜单显示逻辑提取为 showSniMenu() 函数,减少了代码重复。代码安全无漏洞,建议后续优化重入保护和代码重复问题。


本报告由 AI 代码审查工具自动生成
审查时间: 2026-09-14 19:16:35
OCR 审查: 已完成(发现 1 个 medium 级别建议,归类为代码质量改进项)

@mhduiy

mhduiy commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

该 BUG 是应用自己的问题,桌面环境层面无需解决

@mhduiy mhduiy closed this Sep 15, 2026
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.

2 participants