fix(dock): keep taskbar entry when an app replaces its window - #1748
Merged
Merged
Conversation
Reviewer's GuideThe PR prevents taskbar entries from disappearing during Electron-style window replacement by reconciling removals against the live active-app model, updating mappings after row shifts, and adding defensive handling for unresolved windows and transient AppItem gaps. Sequence diagram for preserving the taskbar entry during window replacementsequenceDiagram
participant ActiveAppModel
participant DockGlobalElementModel
participant AppItem
participant QTimer
ActiveAppModel-->>DockGlobalElementModel: rowsRemoved
DockGlobalElementModel->>DockGlobalElementModel: findActiveRowByDesktopId(id)
alt app still has an active window
DockGlobalElementModel->>DockGlobalElementModel: renumber remaining rows
DockGlobalElementModel->>DockGlobalElementModel: findActiveRowByDesktopId(id)
DockGlobalElementModel-->>DockGlobalElementModel: emit dataChanged
else app is no longer active
DockGlobalElementModel-->>DockGlobalElementModel: remove dock row
end
AppItem->>QTimer: singleShot(500)
QTimer-->>AppItem: callback
alt hasWindow() or isDocked()
QTimer-->>AppItem: keep AppItem
else no window and not docked
AppItem->>AppItem: deleteLater()
end
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}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="panels/dock/taskmanager/appitem.cpp" line_range="354-358" />
<code_context>
+ // replaces one window with another (an Electron splash window closing as the
+ // main window appears). Defer the decision so a window arriving in that gap
+ // can re-attach instead of losing the item permanently.
+ QTimer::singleShot(500, this, [this]() {
+ if (!hasWindow() && !isDocked()) {
+ deleteLater();
+ }
+ });
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Every invocation schedules an independent 500 ms deletion callback, but later invocations do not cancel or reset earlier callbacks. If a transient window appears and disappears again, an older callback can delete the AppItem only shortly after the latest window disappeared, before the intended grace period has elapsed.
**Triggers:** When the AppItem becomes empty, briefly receives a window, becomes empty again, and the handover interval exceeds the original timer's remaining time.
**Suggested fix:** Use a restartable member QTimer or a generation/token check so only the callback associated with the latest empty-state transition can delete the AppItem.
```suggestion
const auto deletionGeneration = property("_appItemDeletionGeneration").toInt() + 1;
setProperty("_appItemDeletionGeneration", deletionGeneration);
QTimer::singleShot(500, this, [this, deletionGeneration]() {
if (property("_appItemDeletionGeneration").toInt() == deletionGeneration &&
!hasWindow() && !isDocked()) {
deleteLater();
}
});
```
</issue_to_address>
xionglinlin
force-pushed
the
bug/367743
branch
from
September 20, 2026 01:40
7e8af4d to
553ab36
Compare
BLumia
reviewed
Sep 20, 2026
BLumia
left a comment
BLumia
left a comment
Member
There was a problem hiding this comment.
而新的主窗口之后也不会再产生新的 insert 通知，导致任务栏入口永久消失。
根因不应该是这个吗？应当确保 model 里的信息总是对的，加 timer 去延后删除是在绕过这个问题吧。
Contributor
Author
附件是问题出现时候打印的日志信息和对日志的分析： |
Member
那我觉得应该修这个？ |
Contributor
Author
修的就是这个啊，代码在：panels/dock/taskmanager/dockglobalelementmodel.cpp |
xionglinlin
force-pushed
the
bug/367743
branch
from
September 20, 2026 09:44
553ab36 to
3135bf1
Compare
Contributor
Author
|
实际多次验证了一下，去掉QTimer的延时，问题也已经修复了 |
Member
|
哦我之前的意思是只修那个。现在看上去合理了。 我本地也验一下就加分。 |
wjyrich
approved these changes
Sep 21, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wjyrich, xionglinlin 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 |
An Electron app such as SiYuan shows a splash window and then a main window. When the splash closes, the active-app model reports the row as removed, but because the notification is queued the model already holds the main window in that same row. The handler only looked for another cached entry of the app in m_data, found none, and deleted the dock row even though the app was still running. The main window's own insert notification is delivered too late (the row it refers to no longer exists by then) and gets discarded, so the taskbar entry disappeared permanently. Fix: 1. dockglobalelementmodel.cpp: decide by querying whether the app is still present in the active-app model, instead of trusting the cached m_data entries, and re-point the kept row at its current active row after the remaining rows are renumbered. 2. taskmanager.cpp: skip a window whose desktop file could not be resolved instead of dereferencing a null pointer. Note on the handover gap: an earlier revision of this change also deferred the AppItem deletion behind a 500ms grace timer in appitem.cpp. Measurements with the Electron splash->main handover showed that this path is never taken: the main window is mapped before the splash is destroyed, so the window list never becomes empty during the handover and the timer never protects anything. The only times the timer fired were when the application had really exited, where deleting immediately is correct. The grace period has therefore been dropped; the delete decision stays immediate. Log: keep taskbar entry when an app replaces its window fix(dock): 应用替换窗口时保持任务栏入口 Electron 应用（例如思源笔记 SiYuan）启动时会先显示启动窗口（splash window），随后再创建主窗口。当启动窗口关闭时，active-app model 会将该行 报告为已移除。但由于通知是异步排队处理的，此时 model 实际上已经在同一行中 持有了新的主窗口。处理逻辑之前只会在缓存的 m_data 中查找该应用是否还有 其他缓存项，由于找不到，就直接删除 dock 中对应的任务栏行，尽管该应用实际 上仍在运行。而主窗口自身的 insert 通知投递得太晚（此时它所指的行已不复 存在）而被丢弃，导致任务栏入口永久消失。 修复： 1. dockglobalelementmodel.cpp：不再依赖缓存的 m_data 条目判断应用是否 仍然存在，而是通过查询 active-app model 判断应用是否仍处于活动状态； 同时在剩余行重新编号后，将保留的任务栏行重新指向该应用当前对应的 active row。 2. taskmanager.cpp：当窗口无法解析对应的 desktop file 时直接跳过该 窗口，避免对空指针进行解引用。 关于交接间隙：本变更的早期版本还曾在 appitem.cpp 中用 500ms 宽限定时器 延迟删除 AppItem。实测 Electron 的 splash→main 交接过程不会走到该路径： 主窗口先于 splash 销毁被映射，因此交接期间窗口列表从不为空，该定时器保护 不到任何场景；它唯一触发的时机都是应用确实已退出，此时立即删除才是正确的。 因此去掉了宽限期，删除判断保持立即执行。 Log: 应用窗口交接时不再丢失任务栏图标 PMS: BUG-367743 Change-Id: I8e2d524b3ee590353601cfa947cd48b6781e73c0
xionglinlin
force-pushed
the
bug/367743
branch
from
September 21, 2026 09:22
3135bf1 to
49f8d0a
Compare
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 良好 ✅ 通过 潜在问题：
建议： 1. 考虑将pendingDataChangedRows改为存储desktop ID而非位置索引，在最终发射dataChanged信号时通过ID重新查找当前位置，避免位置失效问题。2. null指针修复（taskmanager.cpp第355行）是重要的安全改进，有效防止了desktopfile为空时的崩溃。 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 1. 注释质量优秀，清晰解释了队列通知机制导致的窗口替换问题和修复思路。2. 新增findActiveRowByDesktopId函数封装良好，包含空值检查。3. 代码风格与现有代码库一致。 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 1. findActiveRowByDesktopId使用match()进行O(n)查找，但在窗口替换场景中调用频率低，性能影响可忽略。2. 使用std::as_const(pendingRowRefresh)避免了不必要的拷贝，是良好实践。 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 1. taskmanager.cpp中新增的null检查修复了潜在的空指针解引用崩溃，提升了代码安全性。2. 日志中仅输出window->id()（窗口标识符），无敏感信息泄露风险。3. 无注入风险、无硬编码密钥、无权限绕过问题。 💡 改进建议代码示例// 暂无代码示例本报告由 AI 代码审查工具自动生成 |
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
4 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.
An Electron app such as SiYuan shows a splash window and then a main window. When the splash closes, the active-app model reports the row as removed, but because the notification is queued the model already holds the main window in that same row. The handler only looked for another cached entry of the app in m_data, found none, and deleted the dock row even though the app was still running. The main window never produced a new insert notification afterwards, so the taskbar entry disappeared permanently.
Fix:
Log: keep taskbar entry when an app replaces its window
fix(dock): 应用替换窗口时保持任务栏入口
Electron 应用（例如思源笔记 SiYuan）启动时会先显示启动窗口（splash
window），随后再创建主窗口。当启动窗口关闭时，active-app model 会将该行报告为已移除。但由于通知 是异步排队处理的，此时 model 实际上已经在同一行中持有了新的主窗口。
处理逻辑之前只会在缓存的 m_data 中查找该应用是否还有其他缓存项，由于
找不到，就直接删除 dock 中对应的任务栏行，尽管该应用实际上仍在运行。
而新的主窗口之后也不会再产生新的 insert 通知，导致任务栏入口永久消失。
修复：
同时在剩余行重新编号后，将保留的任务栏行重新指向该应用当前对应的
active row。
重新关联到已有的 AppItem。
Log: 应用窗口交接时不再丢失任务栏图标
PMS: BUG-367743
Change-Id: I8e2d524b3ee590353601cfa947cd48b6781e73c0
Summary by Sourcery
Keep taskbar entries associated with running applications across window replacement and safely handle unidentified windows.
Bug Fixes:
Enhancements: