Repository navigation
fix(notification): fix action buttons in notification center - #1785
Merged
deepin-bot[bot] merged 1 commit intoOct 28, 2026
Merged
deepin-bot[bot] merged 1 commit into
deepin-bot[bot] merged 1 commit into
Conversation
Reviewer's GuideThis PR fixes notification-center action buttons by preventing premature local/server deletion, updating the view from the server’s Removed state, and queuing removals on the manager worker thread after action dispatch. Sequence diagram for notification action execution and removalsequenceDiagram
participant User
participant NotifyModel
participant NotifyAccessor
participant NotificationManager
participant NotifyServerApplet
User->>NotifyModel: invokeAction(id, actionId)
NotifyModel->>NotifyAccessor: invokeAction(entity, actionId)
NotifyAccessor->>NotificationManager: actionInvoked()
NotificationManager->>NotifyServerApplet: removeNotification(id)
NotifyServerApplet->>NotificationManager: QMetaObject.invokeMethod(removeNotification, QueuedConnection)
NotificationManager->>NotificationManager: removeNotification(id)
NotificationManager-->>NotifyAccessor: NotificationStateChanged(Removed)
NotifyAccessor-->>NotifyModel: entityRemoved(id)
NotifyModel->>NotifyModel: removeInternal(id, false)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
18202781743
reviewed
Oct 27, 2026
Ivy233
force-pushed
the
fix/notification-action-invoke-race
branch
from
October 28, 2026 02:40
b08d434 to
67a753f
Compare
Contributor
Author
|
感谢评审建议。
|
18202781743
previously approved these changes
Oct 28, 2026
Ivy233
force-pushed
the
fix/notification-action-invoke-race
branch
from
October 28, 2026 03:36
67a753f to
7f94ba6
Compare
18202781743
reviewed
Oct 28, 2026
The expire rework changed NotifyServerApplet::actionInvoked() from Qt::DirectConnection to Qt::QueuedConnection. NotifyModel::invokeAction() still removes the notification right after dispatching the action, so the queued action could reach the manager after the removal and find no entity, making action buttons in the notification center a no-op. Restore the synchronous dispatch so the action is handled before remove() deletes the entity. The notification center keeps a single delete entry: remove() handles user operations (close, dismiss and action invocation) and deletes both the view row and the database entity. Notifications that move into history after a bubble disappears still arrive through NotificationStateChanged(Processed) and are not deleted by this path. Since actionInvoked() now runs on the main thread while the manager lives on the worker thread, XdgActivation must not be parented to the manager. Doing so triggers "Cannot create children for a parent that is in a different thread". Create it without a parent instead; the tokenReady handler deletes it when the token arrives. The manager is long-lived, so no separate manager-destroyed cleanup is kept for this short-lived object. The synchronous blocking is the same as 2.0.56 and is small and intentional. Log: fix action buttons in the notification center Influence: 1. Click action buttons on notifications in the notification center and verify the actions run 2. Verify normal, overlap, and expanded group notifications are removed from the center after the action executes 3. Verify manually deleting notifications still works 4. Verify action buttons in bubbles and the staging area still work 5. Verify a notification enters the history area after its bubble expires 6. Trigger both x-deepin-action and non-extended actions and verify the dde-shell log has no "Cannot create children for a parent that is in a different thread" warning fix(notification): 恢复动作派发的同步调用 过期逻辑重构把 NotifyServerApplet::actionInvoked() 从 Qt::DirectConnection 改成了 Qt::QueuedConnection，而 NotifyModel::invokeAction() 仍在派发动作后立即删除通知。排队后的动作 可能晚于删除到达 manager，此时实体已不存在，导致通知中心的动作按钮无效。 恢复同步派发，确保动作在 remove() 删除实体前已由 manager 处理。通知中心 只保留一个删除入口：remove() 负责用户操作（关闭、忽略和点击动作）对视图 与数据库的删除。气泡消失后进入历史通知仍通过 NotificationStateChanged(Processed) 处理，不经过该删除入口。 由于 actionInvoked() 现在在主线程执行，而 manager 位于工作线程，不能再把 manager 作为 XdgActivation 的 parent，否则会触发 "Cannot create children for a parent that is in a different thread" 警告。改为不设置 parent，由 tokenReady 回调在 token 到达后 deleteLater() 释放。manager 为长生命周期 对象，因此不再为这个短生命周期对象额外保留 manager 销毁时的清理连接。 该同步阻塞与 2.0.56 一致，影响较小且是有意保留的。 Log: 修复通知中心动作按钮点击无效的问题 Influence: 1. 在通知中心点击通知动作按钮，确认动作正常执行 2. 确认普通、重叠及展开的分组通知在动作执行后从通知中心移除 3. 确认手动删除通知仍然正常 4. 确认气泡和通知中心 staging 区域的动作按钮仍然正常 5. 确认气泡过期后通知进入历史通知区域 6. 分别触发 x-deepin-action 和普通 action，确认 dde-shell 日志中不再出现 "Cannot create children for a parent that is in a different thread" 警告 PMS: BUG-380339
Ivy233
force-pushed
the
fix/notification-action-invoke-race
branch
from
October 28, 2026 04:40
7f94ba6 to
d25ea58
Compare
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ❌评价： 存在逻辑缺陷 ❌ 不通过 潜在问题： 建议： ['将notifyserverapplet.cpp中两处Qt::DirectConnection替换为Qt::BlockingQueuedConnection，既保证同步执行顺序又确保在正确的线程执行', '在notificationmanager.cpp两处XdgActivation创建后添加安全网连接：connect(this, &QObject::destroyed, activation, &QObject::deleteLater); 确保所有路径下对象都能被清理'] 2. 代码质量 ✅评价： 代码结构清晰，注释完整 ✅ 通过 潜在问题： 建议： [] 3. 代码性能 ✅评价： 性能良好，资源使用合理 ✅ 通过 潜在问题： 建议： [] 4. 代码安全 🔒评价： 存在0个安全漏洞 ✅ 通过
安全漏洞详情： 建议： [] 💡 改进建议代码示例// === notifyserverapplet.cpp 修复 ===
// 使用 BlockingQueuedConnection 替代 DirectConnection，
// 保证同步执行顺序的同时确保在 manager 的工作线程执行
void NotifyServerApplet::actionInvoked(qint64 id, uint bubbleId, const QString &actionKey)
{
QMetaObject::invokeMethod(m_manager, "actionInvoked", Qt::BlockingQueuedConnection,
Q_ARG(qint64, id), Q_ARG(uint, bubbleId), Q_ARG(QString, actionKey));
}
void NotifyServerApplet::actionInvoked(qint64 id, const QString &actionKey)
{
QMetaObject::invokeMethod(m_manager, "actionInvoked", Qt::BlockingQueuedConnection,
Q_ARG(qint64, id), Q_ARG(QString, actionKey));
}
// === notificationmanager.cpp 修复 ===
// 保留无父对象创建，但添加 destroyed 安全网连接防止泄漏
// actionInvoked() 中（约第147行）:
auto *activation = new DS_NAMESPACE::XdgActivation();
connect(this, &QObject::destroyed, activation, &QObject::deleteLater);
connect(activation, &DS_NAMESPACE::XdgActivation::tokenReady, this,
[activation](const QString &token) {
// ... 使用 token ...
activation->deleteLater();
});
// doActionInvoked() 中（约第566行）:
auto *activation = new DS_NAMESPACE::XdgActivation();
connect(this, &QObject::destroyed, activation, &QObject::deleteLater);
connect(activation, &DS_NAMESPACE::XdgActivation::tokenReady, this,
[amArgsCopy, activation](const QString &token) {
// ... 使用 token ...
activation->deleteLater();
});本报告由 AI 代码审查工具自动生成 |
18202781743
approved these changes
Oct 28, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, Ivy233 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 |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
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
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.
The notification center removed a notification immediately after dispatching its action, while the action was queued to the notification manager's worker thread. The removal could reach the manager first and delete the entity before actionInvoked() fetched it, so clicking an action button became a no-op.
Log: fix notification center action buttons not responding
Influence:
fix(notification): 修复通知中心通知按钮点击无效
通知中心在派发动作后立即删除通知，而动作是排队投递到通知管理线程的。
删除操作可能先到达 manager 并删除实体，随后 actionInvoked() 获取不到通知， 导致点击动作按钮无效。
Log: 修复通知中心通知按钮点击无响应的问题
Influence:
PMS: BUG-380339
Summary by Sourcery
Preserve notification action execution by ordering action dispatch ahead of notification removal and making activation-token handling independent of the manager lifetime.
Bug Fixes: