Skip to content

fix: icon not updated on taskbar if icon name in desktop changed - #1700

Open
BLumia wants to merge 1 commit into
linuxdeepin:masterfrom
BLumia:icon-changed
Open

fix: icon not updated on taskbar if icon name in desktop changed#1700
BLumia wants to merge 1 commit into
linuxdeepin:masterfrom
BLumia:icon-changed

Conversation

@BLumia

@BLumia BLumia commented Aug 13, 2026

Copy link
Copy Markdown
Member

修复 desktop 文件中 Icon 字段的值变化后,任务栏上对应图标不会更新的问题

根因(3 个问题)

  1. DockGlobalElementModel 缺少 m_appsModel->dataChanged handler — 之前只连接了 m_activeAppModel->dataChanged,docked app(非运行状态)的图标变化被完全忽略。新增 handler 按 desktop ID 匹配转发。
  2. DockGlobalElementModel 的 m_activeAppModel->dataChanged handler 中 return 应为 continue — 第一行找不到时直接返回,后续行收不到信号。
  3. RoleCombineModel 的 m_indexMap.key() 只返回第一个匹配项 — 多个窗口映射到同一 app 时,只有第一个窗口收到 dataChanged。改为遍历所有映射项,所有匹配的窗口都收到信号。

注:如果在使用目前【最新】的 dde-application-manager 的话,可以使用 dde-dconfig set -a org.deepin.dde.application-manager -r org.deepin.dde.am.appoverride -s /dde-file-manager/wayland -k Icon -v 'deepin-music' 来测试,dde-file-manager 为要修改图标的 appid,wayland 可以为 x11 或 wayland。这种测试方式不需要实际修改 desktop 文件。

Log:

Summary by Sourcery

Ensure taskbar dock icons and combined role models correctly propagate dataChanged signals when underlying app or desktop entries change, especially for multiple windows mapped to the same application.

Bug Fixes:

  • Fix docked taskbar icons not updating when the corresponding desktop file Icon or DesktopId changes in the apps model.
  • Fix active app handling in DockGlobalElementModel so dataChanged signals do not stop after the first unmatched entry.
  • Fix RoleCombineModel so dataChanged on a minor row is forwarded to all major rows mapped to that minor row instead of only the first.

Enhancements:

  • Improve DockGlobalElementModel mapping refresh logic for docked entries when rows are inserted in the apps model.
  • Refine test helper models to emit dataChanged via setData with explicit role handling, better mirroring real model behavior.

Tests:

  • Add coverage to RoleCombineModel to verify dataChanged on a minor row is propagated to all major rows mapped to it, including updated data values.

修复 desktop 文件中 Icon 字段的值变化后,任务栏上对应图标不会更新的问题

Log:

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @BLumia, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@sourcery-ai

sourcery-ai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR fixes taskbar icon updates by correctly propagating dataChanged signals from the apps/desktop models through DockGlobalElementModel and RoleCombineModel, and strengthens tests to cover multiple-window-to-app mappings and proper dataChanged emission.

Sequence diagram for taskbar icon update propagation

sequenceDiagram
    participant DesktopModel as DesktopModel_m_appsModel
    participant DockModel as DockGlobalElementModel
    participant TaskbarView as TaskbarView

    DesktopModel->>DockModel: dataChanged(topLeft,bottomRight,roles)
    loop each row i in [topLeft.row..bottomRight.row]
        DockModel->>DesktopModel: index(i,0).data(DesktopIdRole)
        DockModel->>DockModel: find id in m_data
        alt id found
            DockModel->>TaskbarView: dataChanged(index(pos,0), index(pos,0), roles)
        end
    end
Loading

File-Level Changes

Change Details Files
Ensure docked taskbar items update when the underlying apps model (desktop file Icon changes) emits dataChanged.
  • After rows are inserted into the apps model, re-resolve docked items whose minor row index is -1 by matching DesktopIdRole to set the correct apps-model row.
  • Fix the m_activeAppModel dataChanged handler to use continue instead of return when a matching dock item is not found so later entries still receive updates.
  • Add a new dataChanged handler on m_appsModel that matches changed rows by DesktopIdRole and emits dataChanged for the corresponding DockGlobalElementModel rows.
panels/dock/taskmanager/dockglobalelementmodel.cpp
Propagate minor-model dataChanged to all major rows that map to the same minor row in RoleCombineModel.
  • Replace single key lookup in m_indexMap with iteration over all mappings and compare against the minor (row,column) pair so every major index mapped to the changed minor gets updated.
  • For each matched major index, recompute the minor index via the provided mapping function, refresh m_indexMap, and emit dataChanged with the minor roles.
panels/dock/taskmanager/rolecombinemodel.cpp
Extend tests to cover multi-mapping behavior and to allow programmatic data mutation in test models.
  • Add MinorDataChangedForwardToAllMappedMajorRows test to verify that a single minor row’s dataChanged is forwarded to all major rows mapped to it and that combined data is updated.
  • Refactor DataA/DataB constructors to store a model pointer instead of ignoring the parent, so setData can trigger dataChanged on the owning model.
  • Change DataA::setData and DataB::setData to scan rows for matching idRole and emit dataChanged on the specific row instead of using match() with a range emission.
  • Implement setData overrides in TestModelA and TestModelB to delegate to the DataA/DataB instances and enable QAbstractItemModel::setData-based updates used in the new tests.
tests/panels/dock/taskmanager/rolecombinemodeltests.cpp
tests/panels/dock/taskmanager/combinemodela.cpp
tests/panels/dock/taskmanager/combinemodelb.cpp
tests/panels/dock/taskmanager/combinemodela.h
tests/panels/dock/taskmanager/combinemodelb.h

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

★ 总体评分:95分

■ 【总体评价】

代码修复了多窗口同应用时信号未正确转发的缺陷,逻辑严谨且补充了完善的单元测试。
核心逻辑修复准确,测试覆盖全面,仅在极端数据量下存在理论上的性能损耗。

■ 【详细分析】

  • 1.语法逻辑(完全正确)✓

rolecombinemodel.cpp 中,将原先使用 m_indexMap.key() 单次查找替换为迭代器全表遍历,正确解决了多个 Major 行映射到同一 Minor 行时仅首个 Major 行收到信号的问题。在 dockglobalelementmodel.cpp 中,将 rowsRemoved 槽函数中的 return 修正为 continue,避免了批量删除时遇到未匹配项直接退出整个函数的致命错误。
潜在问题:无
建议:无需修改,逻辑已完全正确。

  • 2.代码质量(良好)✓

代码结构清晰,修复针对性强。在 tests/panels/dock/taskmanager/ 目录下同步更新了 combinemodela.cppcombinemodelb.cpp,完善了 setData 方法,并新增了 MinorDataChangedForwardToAllMappedMajorRows 测试用例,有效验证了修复逻辑。
潜在问题:无
建议:建议在 rolecombinemodel.cpp 的复杂嵌套循环处添加简要注释,说明为何需要全表遍历。

  • 3.代码性能(无性能问题)✓

rolecombinemodel.cpp 中,将 O(1) 的 QMap::key() 查找改为 O(N) 的全表遍历。虽然时间复杂度增加,但考虑到任务栏窗口数量有限,实际性能损耗可忽略不计,换取了逻辑的正确性。
潜在问题:若未来 m_indexMap 规模显著扩大,全表遍历可能导致卡顿。
建议:若后续有性能压力,可考虑维护一个 QMultiHash<QPair<int,int>, QPair<int,int>> 的反向映射表以实现 O(1) 查找。

  • 4.代码安全(存在0个安全漏洞)✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
本次代码修改未引入任何安全漏洞,无外部输入注入风险,无内存越界或空指针解引用问题。

  • 建议:继续保持现有的安全编码规范。

■ 【改进建议代码示例】

// 针对 rolecombinemodel.cpp 的性能优化建议(当前实现已足够,此为可选优化)
// 若 m_indexMap 数据量增大,可维护反向映射
// 在类定义中添加:QMultiHash<QPair<int,int>, QPair<int,int>> m_reverseIndexMap;

// 修改后的 dataChanged 槽函数:
connect(m_minor, &QAbstractItemModel::dataChanged, this,
    [this, majorRoles, func](const QModelIndex &topLeft, const QModelIndex &bottomRight, const QList<int> &roles){
        Q_UNUSED(roles)
        QPair<int,int> minorPair;
        for (int i = topLeft.row(); i <= bottomRight.row(); i++) {
            for (int j =  topLeft.column(); j <= bottomRight.column(); j++) {
                minorPair = qMakePair(i, j);
                // 使用 QMultiHash 的 values() 方法获取所有映射到该 minor 的 major 位置
                QList<QPair<int,int>> majorPositions = m_reverseIndexMap.values(minorPair);
                for (const auto &majorPos : majorPositions) {
                    auto majorIndex = sourceModel()->index(majorPos.first, majorPos.second);
                    if (!majorIndex.isValid())
                        continue;

                    auto minorIndex = func(majorIndex.data(majorRoles), m_minor);
                    if (!minorIndex.isValid())
                        continue;

                    // 更新映射关系
                    m_reverseIndexMap.remove(minorPair, majorPos);
                    auto newMinorPair = qMakePair(minorIndex.row(), minorIndex.column());
                    m_indexMap[majorPos] = newMinorPair;
                    m_reverseIndexMap.insert(newMinorPair, majorPos);
                    
                    Q_EMIT dataChanged(majorIndex, majorIndex, m_minorRolesMap.values());
                }
            }
        }
});

Comment thread panels/dock/taskmanager/dockglobalelementmodel.cpp
@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

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