fix: rebuild SNI menu on menu path change - #996
Open
GongHeng2017 wants to merge 1 commit into
Open
GongHeng2017 wants to merge 1 commit into
GongHeng2017 wants to merge 1 commit into
Conversation
1. Root cause: onSNIMenuChanged only stored the new menu path without destroying old m_menu and m_dbusMenuImporter, showContextMenu reused stale menu objects with expired DBus connection after input method switch 2. Fix: destroy old m_menu and m_dbusMenuImporter and set them to null in onSNIMenuChanged, so next showContextMenu triggers initMenu() to rebuild with the new menu path 3. Impact: only affects SNI tray items whose menu path changed at runtime, normal menu creation flow unchanged Log: fix SNI tray context menu not opening after input method switch Influence: 1. Test right-click menu after switching input method 2. Test right-click menu without switching input method 3. Verify other SNI app tray menus are unaffected fix: 切换输入法后重建 SNI 右键菜单 1. 根因：onSNIMenuChanged 仅存储新菜单路径，未销毁旧的 m_menu 和 m_dbusMenuImporter，切换输入法后 showContextMenu 复用过期菜单 对象导致右键菜单无法弹出 2. 方案：在 onSNIMenuChanged 中销毁旧 m_menu 和 m_dbusMenuImporter 并置 null，下次 showContextMenu 触发 initMenu() 用新路径重建 3. 影响：仅影响运行时菜单路径变更的 SNI 托盘项，正常菜单创建 流程不受影响 Log: 修复切换输入法后任务栏输入法右键菜单无法打开的问题 Influence: 1. 测试切换输入法后右键菜单能否正常弹出 2. 测试不切换输入法时右键菜单是否正常 3. 验证其他 SNI 应用托盘菜单无回归 PMS: BUG-255089
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates SNI menu-change handling to discard stale menu and importer objects so the next right-click rebuilds the context menu using the new DBus menu path, fixing menus that disappear after runtime SNI path changes while preserving normal initialization and unaffected tray icons. Sequence diagram for rebuilding the SNI menu after a DBus path changesequenceDiagram
participant SNI as SNIService
participant Widget as SNITrayItemWidget
participant Menu as QMenu
participant Importer as DBusMenuImporter
SNI->>Widget: onSNIMenuChanged(value)
Widget->>Widget: m_sniMenuPath = value
Widget->>Menu: delete m_menu
Widget->>Importer: delete m_dbusMenuImporter
Widget->>Widget: m_menu = nullptr
Widget->>Widget: m_dbusMenuImporter = nullptr
Widget->>Widget: showContextMenu()
Widget->>Widget: initMenu()
Widget->>Importer: create importer with m_sniMenuPath
Widget->>Menu: create menu
Widget->>Menu: popup()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: GongHeng2017 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 1.删除顺序正确：先删除子对象m_menu，再删除父对象m_dbusMenuImporter，避免双重释放 2.nullptr检查在delete前虽然技术上非必需（C++允许delete nullptr），但作为防御性编程是好实践 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题：
建议： 建议在清理代码前添加注释，如：// 菜单路径变更时，销毁旧菜单对象，下次访问时将使用新路径重新构建 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 1.清理旧对象释放内存是正确的资源管理方式，避免内存泄漏 2.下次访问菜单时通过initMenu()惰性重建，避免不必要的预创建 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 1.正确的内存管理避免了悬垂指针和潜在的use-after-free风险 2.delete后立即置nullptr防止了野指针访问 💡 改进建议代码示例// 菜单路径变更时，销毁旧菜单对象，下次访问时将使用新路径重新构建
if (m_menu) {
delete m_menu;
m_menu = nullptr;
}
if (m_dbusMenuImporter) {
delete m_dbusMenuImporter;
m_dbusMenuImporter = nullptr;
}本报告由 AI 代码审查工具自动生成 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root Cause Analysis
When fcitx switches input method on 1070, it re-registers the SNI service with a new DBus menu path.
SNITrayItemWidget::onSNIMenuChanged(snitrayitemwidget.cpp:383) only stores the new menu path (m_sniMenuPath = value) but does not destroy or rebuild the existingm_menuandm_dbusMenuImporterobjects. On the next right-click,showContextMenu(snitrayitemwidget.cpp:310) seesm_menuis non-null and skipsinitMenu(), callingm_menu->popup()on a stale DBus connection that can no longer fetch menu data — the menu fails to appear.Fix Approach
In
onSNIMenuChanged, after storing the new menu path, destroy the oldm_menuandm_dbusMenuImporterand set them tonullptr. This forces the nextshowContextMenucall to re-enterinitMenu(), which rebuilds both objects with the updatedm_sniMenuPath. The change is 9 lines, all withinonSNIMenuChanged, matching the root-cause fix proposed in the analysis report.Change Safety Assessment
Code Safety
onSNIMenuChangedwas introduced in commitbce2a499(2024-01-04) as part of a feature change and has never been modified since — no historical fix is being reverted.snitrayitemwidget.cpp:650),onSNIMenuChangedis called when bothm_menuandm_dbusMenuImporterarenullptr, so the newifguards do not trigger — no impact on the initialization path.Business Impact Scope
Affects SNI tray icon right-click context menus in dde-dock. When an SNI application (e.g., fcitx input method) changes its DBus menu path at runtime, the tray widget now properly rebuilds the menu instead of reusing a stale connection. Other SNI applications (calendar, volume, etc.) that do not change their menu path at runtime are unaffected — their
onSNIMenuChangedeither is never called or is called with the same path, where the delete+rebuild is a safe no-op visually.Verification Suggestion
根因分析
在 1070 系统上切换输入法时，fcitx 会重新注册 SNI 服务并使用新的 DBus 菜单路径。
SNITrayItemWidget::onSNIMenuChanged（snitrayitemwidget.cpp:383）仅存储新菜单路径（m_sniMenuPath = value），未销毁或重建已有的m_menu和m_dbusMenuImporter对象。下次右键点击时，showContextMenu（snitrayitemwidget.cpp:310）发现m_menu非 null，跳过initMenu()，直接对过期的 DBus 连接调用m_menu->popup()，无法获取菜单数据，导致菜单不弹出。修复方案
在
onSNIMenuChanged中存储新菜单路径后，销毁旧的m_menu和m_dbusMenuImporter并置为nullptr，使下次showContextMenu重新进入initMenu()用新的m_sniMenuPath重建。改动 9 行，全部在onSNIMenuChanged内，与分析报告中的根因修复建议一致。改动安全评估
代码安全评估
onSNIMenuChanged于 commitbce2a499（2024-01-04）作为功能变更引入，此后从未被修改——不存在撤销历史修复的风险。snitrayitemwidget.cpp:650）调用onSNIMenuChanged时m_menu和m_dbusMenuImporter均为nullptr，新增的if守卫不会触发——对初始化流程无影响。业务影响范围
影响 dde-dock 中 SNI 托盘图标的右键菜单。当 SNI 应用（如 fcitx 输入法）在运行时变更 DBus 菜单路径时，托盘控件现在会正确重建菜单而非复用过期连接。其他不在运行时变更菜单路径的 SNI 应用（日历、音量等）不受影响——其
onSNIMenuChanged要么不被调用，要么以相同路径调用，delete+重建在视觉上是无副作用的。验证建议
Summary by Sourcery
Bug Fixes: