fix: fix SNI tray context menu style issue on first click#449
fix: fix SNI tray context menu style issue on first click#449deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Conversation
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
Reviewer's guide (collapsed on small PRs)Reviewer's GuideEnsures 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 handlingsequenceDiagram
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
Sequence diagram for theme change and safe menu updatesequenceDiagram
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
Class diagram for SniTrayProtocolHandler and DBusMenuImporter interactionsclassDiagram
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
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段代码的修改主要是为了处理没有 DBusMenu 的状态通知项(Status Notifier Item)的情况,增加了空指针检查。以下是对这段代码的详细审查和改进建议: 1. 语法逻辑审查修改分析:
潜在问题:
2. 代码质量审查优点:
改进建议:
3. 代码性能审查性能影响:
改进建议:
4. 代码安全审查安全性改进:
潜在风险:
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. 总结这段代码的修改总体上是积极的,增加了对空指针的检查,提高了代码的健壮性。主要的改进点包括:
建议的进一步改进包括:
这些改进将使代码更加健壮、可读和可维护。 |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
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 returnnullptr, 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.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
/forcemerge |
|
This pr force merged! (status: blocked) |
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:
theme change handler
to prevent creating invalid DBusMenuImporter
accessing menu, ensuring ContextMenu() is only called when no valid menu
is available
Log: Fixed SNI tray context menu styling issue on first right-click
Influence:
with correct DTK styling
issues
correctly
is available
fix: 修复SNI托盘右键菜单首次点击样式错误
修复了SNI托盘右键菜单首次点击时显示错误样式的问题。问题发生
在DBusMenuImporter未正确初始化时调用了menuImporter(),导致
menu()->isEmpty()返回错误结果。这触发了回退的ContextMenu()调用,该函数使
用非DTK样式的菜单。
修复内容包括:
创建无效的DBusMenuImporter
在无有效菜单时调用ContextMenu()
Log: 修复SNI托盘右键菜单首次点击时的样式问题
Influence:
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:
Enhancements: