Skip to content

fix: fix SNI tray context menu style issue on first click#449

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
18202781743:master
Apr 20, 2026
Merged

fix: fix SNI tray context menu style issue on first click#449
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
18202781743:master

Conversation

@18202781743
Copy link
Copy Markdown
Contributor

@18202781743 18202781743 commented Apr 20, 2026

Fixed an issue where the SNI tray context menu displayed incorrect
styling on the first right-click. The problem occurred because
menuImporter() was being called before the DBusMenuImporter was properly
initialized, causing menu()->isEmpty() to return incorrect results. This
led to the fallback ContextMenu() being called, which uses a non-DTK
style menu.

The fix includes:

  1. Added null check for menuImporter() before calling updateMenu() in
    theme change handler
  2. Added early return in menuImporter() when menu path is "/NO_DBUSMENU"
    to prevent creating invalid DBusMenuImporter
  3. Modified eventFilter to check if menuImporter() returns null before
    accessing menu, ensuring ContextMenu() is only called when no valid menu
    is available
  4. Ensured proper initialization sequence for DBusMenuImporter

Log: Fixed SNI tray context menu styling issue on first right-click

Influence:

  1. Test right-click on SNI tray icons to verify context menu displays
    with correct DTK styling
  2. Verify menu appears immediately on first right-click without style
    issues
  3. Test theme switching while tray menu is open to ensure menu updates
    correctly
  4. Verify ContextMenu() fallback still works when no valid menu path
    is available
  5. Test left-click activation functionality remains unchanged

fix: 修复SNI托盘右键菜单首次点击样式错误

修复了SNI托盘右键菜单首次点击时显示错误样式的问题。问题发生
在DBusMenuImporter未正确初始化时调用了menuImporter(),导致
menu()->isEmpty()返回错误结果。这触发了回退的ContextMenu()调用,该函数使
用非DTK样式的菜单。

修复内容包括:

  1. 在主题变更处理程序中添加了对menuImporter()的空指针检查
  2. 在menuImporter()中添加了当菜单路径为"/NO_DBUSMENU"时的提前返回,防止
    创建无效的DBusMenuImporter
  3. 修改了eventFilter,在访问菜单前检查menuImporter()是否返回空值,确保仅
    在无有效菜单时调用ContextMenu()
  4. 确保DBusMenuImporter的正确初始化顺序

Log: 修复SNI托盘右键菜单首次点击时的样式问题

Influence:

  1. 测试SNI托盘图标右键点击,验证上下文菜单以正确的DTK样式显示
  2. 验证首次右键点击时菜单立即显示且无样式问题
  3. 测试在托盘菜单打开时切换主题,确保菜单正确更新
  4. 验证当无有效菜单路径时,ContextMenu()回退功能仍正常工作
  5. 测试左键激活功能保持不变

PMS: BUG-357505

Summary by Sourcery

Fix SNI tray context menu to use the correct styled menu on first right-click by ensuring the DBus menu importer is valid before use.

Bug Fixes:

  • Prevent the SNI tray context menu from falling back to a non-DTK styled menu on the first right-click by guarding against uninitialized or invalid DBusMenuImporter instances.

Enhancements:

  • Improve robustness of SNI tray menu handling by adding null checks and early returns around the DBus menu importer and its initialization sequence.

Fixed an issue where the SNI tray context menu displayed incorrect
styling on the first right-click. The problem occurred because
menuImporter() was being called before the DBusMenuImporter was properly
initialized, causing menu()->isEmpty() to return incorrect results. This
led to the fallback ContextMenu() being called, which uses a non-DTK
style menu.

The fix includes:
1. Added null check for menuImporter() before calling updateMenu() in
theme change handler
2. Added early return in menuImporter() when menu path is "/NO_DBUSMENU"
to prevent creating invalid DBusMenuImporter
3. Modified eventFilter to check if menuImporter() returns null before
accessing menu, ensuring ContextMenu() is only called when no valid menu
is available
4. Ensured proper initialization sequence for DBusMenuImporter

Log: Fixed SNI tray context menu styling issue on first right-click

Influence:
1. Test right-click on SNI tray icons to verify context menu displays
with correct DTK styling
2. Verify menu appears immediately on first right-click without style
issues
3. Test theme switching while tray menu is open to ensure menu updates
correctly
4. Verify ContextMenu() fallback still works when no valid menu path
is available
5. Test left-click activation functionality remains unchanged

fix: 修复SNI托盘右键菜单首次点击样式错误

修复了SNI托盘右键菜单首次点击时显示错误样式的问题。问题发生
在DBusMenuImporter未正确初始化时调用了menuImporter(),导致
menu()->isEmpty()返回错误结果。这触发了回退的ContextMenu()调用,该函数使
用非DTK样式的菜单。

修复内容包括:
1. 在主题变更处理程序中添加了对menuImporter()的空指针检查
2. 在menuImporter()中添加了当菜单路径为"/NO_DBUSMENU"时的提前返回,防止
创建无效的DBusMenuImporter
3. 修改了eventFilter,在访问菜单前检查menuImporter()是否返回空值,确保仅
在无有效菜单时调用ContextMenu()
4. 确保DBusMenuImporter的正确初始化顺序

Log: 修复SNI托盘右键菜单首次点击时的样式问题

Influence:
1. 测试SNI托盘图标右键点击,验证上下文菜单以正确的DTK样式显示
2. 验证首次右键点击时菜单立即显示且无样式问题
3. 测试在托盘菜单打开时切换主题,确保菜单正确更新
4. 验证当无有效菜单路径时,ContextMenu()回退功能仍正常工作
5. 测试左键激活功能保持不变

PMS: BUG-357505
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Apr 20, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Ensures the StatusNotifierItem tray context menu only falls back to the non-DTK ContextMenu when there is truly no DBus menu available, by guarding DBusMenuImporter creation and all its usages with null checks and a sentinel menu path check, so the first right-click uses the correctly styled DTK menu and theme updates are handled safely.

Sequence diagram for SNI tray right-click context menu handling

sequenceDiagram
    actor User
    participant SniTrayProtocolHandler
    participant DBusMenuImporter
    participant StatusNotifierItem
    participant DTKMenu

    User->>SniTrayProtocolHandler: eventFilter(mousePress RightButton)
    alt menu path is NO_DBUSMENU
        SniTrayProtocolHandler->>SniTrayProtocolHandler: menuImporter()
        SniTrayProtocolHandler-->>SniTrayProtocolHandler: null
        SniTrayProtocolHandler->>StatusNotifierItem: ContextMenu(0, 0)
    else valid DBus menu path
        SniTrayProtocolHandler->>SniTrayProtocolHandler: menuImporter()
        alt first access and importer not created
            SniTrayProtocolHandler->>DBusMenuImporter: constructor(service, menuPath)
            SniTrayProtocolHandler-->>SniTrayProtocolHandler: DBusMenuImporter instance
        else importer already exists
            SniTrayProtocolHandler-->>SniTrayProtocolHandler: existing DBusMenuImporter
        end
        SniTrayProtocolHandler->>DBusMenuImporter: menu()
        DBusMenuImporter-->>SniTrayProtocolHandler: DTKMenu
        SniTrayProtocolHandler->>DTKMenu: setFixedSize(sizeHint())
        SniTrayProtocolHandler->>DTKMenu: winId()
        SniTrayProtocolHandler-->>User: DTK styled menu shown
    end
Loading

Sequence diagram for theme change and safe menu update

sequenceDiagram
    participant DGuiApplicationHelper
    participant SniTrayProtocolHandler
    participant DBusMenuImporter

    DGuiApplicationHelper->>SniTrayProtocolHandler: themeTypeChanged(themeType)
    SniTrayProtocolHandler->>SniTrayProtocolHandler: menuImporter()
    alt menu path is NO_DBUSMENU
        SniTrayProtocolHandler-->>SniTrayProtocolHandler: null
        SniTrayProtocolHandler-->>DGuiApplicationHelper: no updateMenu call
    else valid DBus menu path
        alt importer not yet created
            SniTrayProtocolHandler->>DBusMenuImporter: constructor(service, menuPath)
            SniTrayProtocolHandler-->>SniTrayProtocolHandler: DBusMenuImporter instance
        else importer already exists
            SniTrayProtocolHandler-->>SniTrayProtocolHandler: existing DBusMenuImporter
        end
        SniTrayProtocolHandler->>DBusMenuImporter: updateMenu(true)
    end
Loading

Class diagram for SniTrayProtocolHandler and DBusMenuImporter interactions

classDiagram
    class SniTrayProtocolHandler {
        -QString m_service
        -QString m_menuPath
        -DBusMenuImporter* m_dbusMenuImporter
        -StatusNotifierItem* m_sniInter
        +SniTrayProtocolHandler(QString sniServicePath, QObject* parent)
        +DBusMenuImporter* menuImporter() const
        +bool eventFilter(QObject* watched, QEvent* event)
    }

    class DBusMenuImporter {
        +QMenu* menu()
        +void updateMenu(bool force)
    }

    class DBusMenu {
        +DBusMenu(QString service, QString menuPath, QObject* parent)
    }

    class StatusNotifierItem {
        +void Activate(int x, int y)
        +void ContextMenu(int x, int y)
        +void NewToolTip()
    }

    class DGuiApplicationHelper {
        +static DGuiApplicationHelper* instance()
        +void themeTypeChanged(ColorType themeType)
    }

    class QMenu {
        +QSize sizeHint()
        +void setFixedSize(QSize size)
        +WId winId()
        +bool isEmpty()
    }

    SniTrayProtocolHandler --> DBusMenuImporter : uses m_dbusMenuImporter
    DBusMenuImporter <|-- DBusMenu
    SniTrayProtocolHandler --> StatusNotifierItem : uses m_sniInter
    DGuiApplicationHelper --> SniTrayProtocolHandler : themeTypeChanged observer
    DBusMenuImporter --> QMenu : returns menu
    SniTrayProtocolHandler --> QMenu : configure menu on right-click
Loading

File-Level Changes

Change Details Files
Guard theme-change menu updates so they only run when a valid DBusMenuImporter exists.
  • Wrap the themeTypeChanged handler’s call to updateMenu(true) in a null check on the DBus menu importer instance
  • Avoid triggering menu updates during theme changes when no DBus-backed menu is available
plugins/application-tray/sniprotocolhandler.cpp
Prevent creation of an invalid DBusMenuImporter when the menu path indicates no DBus menu is available.
  • In menuImporter(), add an early return of nullptr when the menu path equals the sentinel "/NO_DBUSMENU"
  • Only construct the DBusMenu object when the menu path is valid, keeping m_dbusMenuImporter null otherwise
plugins/application-tray/sniprotocolhandler.cpp
Adjust right-click event handling to use the DBus menu only when it is valid and otherwise fall back cleanly to ContextMenu().
  • On right-click, first check whether menuImporter() returns nullptr and call ContextMenu() only in that case
  • Move access to menuImporter()->menu() after the null check so the code only manipulates a valid menu object
  • Retain existing left-click activation behavior while ensuring the context menu is sized and realized only when present
plugins/application-tray/sniprotocolhandler.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
Copy Markdown

deepin pr auto review

这段代码的修改主要是为了处理没有 DBusMenu 的状态通知项(Status Notifier Item)的情况,增加了空指针检查。以下是对这段代码的详细审查和改进建议:

1. 语法逻辑审查

修改分析:

  • 第一处修改:在 themeTypeChanged 的 lambda 中增加了对 menuImporter() 返回值的空指针检查。这是必要的,因为后续修改中 menuImporter() 可能返回 nullptr
  • 第二处修改:在 menuImporter() 函数中增加了对 m_menuPath/NO_DBUSMENU 的特殊处理,直接返回 nullptr。这符合逻辑,表示该应用不支持菜单。
  • 第三处修改:在 eventFilter 中调整了逻辑顺序,先检查 menuImporter() 是否为空,再获取菜单对象。这避免了空指针解引用的风险。

潜在问题:

  • eventFilter 中,如果 menuImporter() 返回 nullptr,代码会调用 m_sniInter->ContextMenu(0, 0)。这看起来是合理的后备方案,但需要确保 m_sniInter 在此时一定有效。
  • 移除了 Q_CHECK_PTR(menu) 宏。这个宏在 Debug 模式下会打印错误信息并终止程序,而在 Release 模式下什么都不做。改为显式的空指针检查是更好的做法。

2. 代码质量审查

优点:

  • 增加了防御性编程,对可能为空的指针进行了检查。
  • 逻辑更加清晰,先检查条件再使用对象。

改进建议:

  • 命名一致性menuImporter() 方法名暗示它总是返回一个有效的对象,但现在它可能返回 nullptr。考虑重命名方法以反映这一点,例如 menuImporterOrNull()maybeMenuImporter()
  • 魔法字符串"/NO_DBUSMENU" 是一个硬编码的字符串,应该定义为常量,例如:
    static const QLatin1String NO_DBUSMENU_PATH("/NO_DBUSMENU");
    然后在代码中使用 NO_DBUSMENU_PATH

3. 代码性能审查

性能影响:

  • 这些修改主要增加了条件判断,对性能的影响可以忽略不计。
  • menuImporter() 中的 const_cast 是一个潜在的性能问题,因为它打破了 const 正确性,但在这种情况下可能是必要的,因为需要延迟初始化 m_dbusMenuImporter

改进建议:

  • 考虑使用 mutable 关键字修饰 m_dbusMenuImporter,这样可以避免 const_cast
    mutable DBusMenuImporter *m_dbusMenuImporter = nullptr;

4. 代码安全审查

安全性改进:

  • 增加空指针检查是很好的安全实践,可以防止空指针解引用导致的崩溃。
  • 确保在调用 menuImporter()->menu() 之前已经检查了 menuImporter() 不为空。

潜在风险:

  • 如果 menuImporter() 返回 nullptr,调用 menu() 会导致崩溃。代码已经通过先检查 menuImporter() 来避免这个问题。
  • eventFilter 中,如果 menuImporter() 返回 nullptr,代码会调用 m_sniInter->ContextMenu(0, 0)。需要确保 m_sniInter 在此时一定有效。

5. 综合改进建议

以下是改进后的代码:

// 在类定义中添加常量
static const QLatin1String NO_DBUSMENU_PATH("/NO_DBUSMENU");

// 在构造函数中
connect(DGuiApplicationHelper::instance(), &DGuiApplicationHelper::themeTypeChanged, this, [this](DGuiApplicationHelper::ColorType themeType) {
    Q_UNUSED(themeType)
    if (auto *importer = menuImporter()) {
        importer->updateMenu(true);
    }
});

// menuImporter() 方法
DBusMenuImporter *SniTrayProtocolHandler::menuImporter() const
{
    if (!m_dbusMenuImporter) {
        if (m_menuPath == NO_DBUSMENU_PATH) {
            return nullptr;
        }
        auto that = const_cast<SniTrayProtocolHandler *>(this);
        that->m_dbusMenuImporter = new DBusMenu(m_service, m_menuPath, that);
    }
    return m_dbusMenuImporter;
}

// eventFilter 方法
bool SniTrayProtocolHandler::eventFilter(QObject *watched, QEvent *event)
{
    // ... 其他代码 ...
    if (mouseEvent->button() == Qt::LeftButton) {
        m_sniInter->Activate(0, 0);
    } else if (mouseEvent->button() == Qt::RightButton) {
        auto *importer = menuImporter();
        if (!importer) {
            m_sniInter->ContextMenu(0, 0);
            return false;
        }

        auto *menu = importer->menu();
        if (!menu) {  // 添加额外的安全检查
            m_sniInter->ContextMenu(0, 0);
            return false;
        }

        menu->setFixedSize(menu->sizeHint());
        menu->winId();
        // ... 其他代码 ...
    }
    // ... 其他代码 ...
}

6. 总结

这段代码的修改总体上是积极的,增加了对空指针的检查,提高了代码的健壮性。主要的改进点包括:

  1. 增加了对 menuImporter() 返回值的空指针检查
  2. 处理了不支持 DBusMenu 的情况
  3. 调整了逻辑顺序以避免潜在的空指针解引用

建议的进一步改进包括:

  • 将魔法字符串定义为常量
  • 考虑使用 mutable 关键字避免 const_cast
  • eventFilter 中增加对 menu 的额外空指针检查
  • 考虑重命名 menuImporter() 方法以反映它可能返回 nullptr 的事实

这些改进将使代码更加健壮、可读和可维护。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

Copy link
Copy Markdown

@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 left some high level feedback:

  • In eventFilter, menuImporter() is invoked twice on the right-click path; consider calling it once, storing the pointer in a local variable, and reusing it to avoid redundant work and potential inconsistencies.
  • Now that menuImporter() can return nullptr, double-check all existing call sites (beyond the ones updated here) to ensure they defensively handle a null return instead of assuming a valid importer.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `eventFilter`, `menuImporter()` is invoked twice on the right-click path; consider calling it once, storing the pointer in a local variable, and reusing it to avoid redundant work and potential inconsistencies.
- Now that `menuImporter()` can return `nullptr`, double-check all existing call sites (beyond the ones updated here) to ensure they defensively handle a null return instead of assuming a valid importer.

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.

@18202781743
Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot
Copy link
Copy Markdown

deepin-bot Bot commented Apr 20, 2026

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit 9ae0b68 into linuxdeepin:master Apr 20, 2026
9 of 11 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.

3 participants