Skip to content

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Dec 29, 2025

判断写反了,导致移除失败。

(注:当前问题只会出现在 wayland 环境)

Summary by Sourcery

Fix undocking logic for xembed system tray icons and improve selection manager logging.

Bug Fixes:

  • Correct undocking condition so existing tray icons can be successfully removed.

Enhancements:

  • Switch selection manager startup logging to use categorized debug output and add a message when undocking fails due to a missing icon.

判断写反了,导致移除失败...

Log:
@BLumia BLumia requested a review from tsic404 December 29, 2025 10:04
@sourcery-ai
Copy link

sourcery-ai bot commented Dec 29, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Fixes undocking logic for xembed tray icons (especially under Wayland) and improves logging category usage in the FdoSelectionManager.

Sequence diagram for updated undock logic in FdoSelectionManager

sequenceDiagram
    participant FdoSelectionManager
    participant TrayManager

    FdoSelectionManager->>TrayManager: haveIcon(winId)
    TrayManager-->>FdoSelectionManager: bool hasIcon

    alt icon not found
        FdoSelectionManager->>FdoSelectionManager: log failed to find winId to undock
        FdoSelectionManager-->>FdoSelectionManager: return
    else icon found
        FdoSelectionManager->>TrayManager: unregisterIcon(winId)
        TrayManager-->>FdoSelectionManager: (icon unregistered)
    end
Loading

Updated class diagram for FdoSelectionManager undock behavior

classDiagram
    class FdoSelectionManager {
        - KSelectionOwner* m_selectionOwner
        - TrayManager* m_trayManager
        + FdoSelectionManager(QObject* parent)
        + void init()
        + void undock(xcb_window_t winId)
    }

    class TrayManager {
        + bool haveIcon(xcb_window_t winId)
        + void unregisterIcon(xcb_window_t winId)
    }

    FdoSelectionManager --> TrayManager : uses

    class SELECTIONMGR {
    }

    FdoSelectionManager ..> SELECTIONMGR : qCDebug(SELECTIONMGR)
Loading

File-Level Changes

Change Details Files
Correct undock logic so that tray icons are only unregistered when they actually exist, adding debug when the icon is missing.
  • Invert the condition checking whether TrayManager has the icon before proceeding with undock
  • Add a categorized debug log when the specified window ID is not found in the tray manager
  • Ensure early return happens only when the icon is missing instead of when it exists
plugins/application-tray/fdoselectionmanager.cpp
Use categorized debug logging for FdoSelectionManager startup.
  • Replace qDebug with qCDebug using the SELECTIONMGR logging category for the startup message
plugins/application-tray/fdoselectionmanager.cpp

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

@deepin-ci-robot
Copy link

deepin pr auto review

我来对这段代码的修改进行审查:

  1. 语法逻辑改进:

    • qDebug 改为 qCDebug 是正确的,这符合Qt的日志分类最佳实践,可以更好地控制日志输出。
    • undock 函数中,将条件判断从 if (m_trayManager->haveIcon(winId)) 改为 if (!m_trayManager->haveIcon(winId)) 是一个重要的逻辑修正,这样可以提前返回无效情况,避免后续不必要的操作。
  2. 代码质量改进:

    • 添加了错误日志 qCDebug(SELECTIONMGR) << "failed to find winId to undock:" << winId;,这有助于调试和问题追踪。
    • 移除了多余的空行,使代码更加紧凑。
  3. 代码性能:

    • 提前返回的改进可以避免后续不必要的 unregisterIcon 调用,提高了性能。
    • 日志系统的改进不会显著影响性能,因为Qt的日志系统本身是经过优化的。
  4. 代码安全:

    • 没有明显的安全性问题,但日志记录的改进有助于追踪潜在问题。

建议:

  1. 可以考虑在 init 函数调用前添加一些错误检查,确保必要的资源已经正确初始化。
  2. undock 函数中,可以考虑添加更多的错误处理,比如检查 winId 的有效性。
  3. 建议在类的构造函数中添加更多的初始化检查,确保所有成员变量都被正确初始化。

总体来说,这些修改提高了代码的可维护性和健壮性,是一个积极的改进。

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 reviewed your changes and they look great!


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.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, tsic404

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

@BLumia BLumia merged commit 5b6db21 into linuxdeepin:master Dec 29, 2025
8 of 10 checks passed
@BLumia BLumia deleted the xembed-tray-undock branch December 29, 2025 10:11
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