Skip to content

fix: add common palette fallback for dock app backgrounds - #1777

Open
Ivy233 wants to merge 1 commit into
linuxdeepin:masterfrom
Ivy233:wip/wall-batch3-unverified
Open

Ivy233 wants to merge 1 commit into
linuxdeepin:masterfrom
Ivy233:wip/wall-batch3-unverified

Conversation

@Ivy233

@Ivy233 Ivy233 commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

AppItemBackground overrides the base AppletItemBackground palettes with crystal-only definitions, dropping the base's common fallback. While the inherited ColorSelector family is still CommonColor (for example during dock item creation), getColor() cannot resolve the crystal-only entries, and reads of backgroundColor, insideBorderColor, outsideBorderColor and the active* variants log "ColorSelector: The ... is an invalid color". Add common transparent fallbacks to the normal group of the overridden palettes, matching AppletItemBackground. Crystal rendering used by the dock is unchanged.

The previous W075 QProcess lifetime change was withdrawn after review: if the applet is already gone, dde-shell itself has exited or crashed, so the QProcess destructor warning should remain visible for diagnosis instead of being suppressed. The related QPointer/QProcess connection split and the queryTrashCount()/gio change are no longer part of this PR.

The ConfigManager and Appearance1 startup races (W078/W079/W049) are not guarded here: they share the same root cause as the dde-launchpad issue #826 and are left to the dde-session D-Bus dependency DAG instead of adding consumer-side workarounds.

dock AppItemBackground 颜色告警修复:

AppItemBackground 覆盖 AppletItemBackground 的 palette 时只定义了 crystal 颜色族,丢掉了基类的 common 兜底。当继承的 ColorSelector family 仍为 CommonColor(例如 dock item 创建期间)时,getColor() 无法解析仅含 crystal 的条目,读取 backgroundColor、insideBorderColor、outsideBorderColor 及 active* 系列时会打印 "ColorSelector: ... is an invalid color"。现在为各覆盖 palette 的 normal 组补充 common 透明兜底,与 AppletItemBackground 保持一致;dock 正常的 crystal 渲染不变。

按评审讨论,原 W075 的 QProcess 生命周期改动已撤回:applet 已不存在说明 dde-shell 本身已退出或崩溃,此时应保留 QProcess 析构 warning 作为诊断信息,而不是静默它。相关的 QPointer/QProcess 连接拆分以及 queryTrashCount()/gio 改动均不再包含在本 PR 中。

ConfigManager 与 Appearance1 启动竞态(W078/W079/W049)不在本提交中做消费者侧守卫:它们与 dde-launchpad issue #826 同根因,统一由 dde-session 的 D-Bus 依赖 DAG 处理。

PMS: TASK-394335

Summary by Sourcery

Bug Fixes:

  • Add common transparent palette fallbacks to dock app item backgrounds to prevent invalid color warnings during CommonColor resolution while preserving CrystalColor rendering.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Ivy233

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

@sourcery-ai

sourcery-ai Bot commented Oct 21, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR removes process-lifetime warnings, handles startup races by waiting for ConfigManager and refetching state, and fixes dock color warnings by pinning the component to its supported Crystal palette family. Appearance1's related startup race is intentionally left to the session D-Bus dependency graph.

Sequence diagram for startup-race configuration refetch

sequenceDiagram
    participant Applet
    participant ConfigManager
    participant ServiceWatcher
    participant DConfig

    Applet->>ConfigManager: isServiceRegistered()
    alt ConfigManager not registered
        Applet-->>Applet: keep default state
        ConfigManager-->>ServiceWatcher: serviceRegistered()
        ServiceWatcher->>Applet: sync()
        Applet->>DConfig: create()
        DConfig-->>Applet: fetch configured state
    else ConfigManager registered
        Applet->>DConfig: create()
        DConfig-->>Applet: fetch configured state
    end
Loading

Sequence diagram for safe asynchronous process cleanup

sequenceDiagram
    participant MultiTaskView
    participant KWinQuery as QProcess
    participant TaskManager
    participant Gio as QProcess

    MultiTaskView->>KWinQuery: start()
    KWinQuery-->>MultiTaskView: finished()
    alt MultiTaskView still exists
        MultiTaskView->>KWinQuery: readAllStandardOutput()
    end
    MultiTaskView->>KWinQuery: deleteLater()

    TaskManager->>Gio: waitForFinished(1000)
    alt Gio still running
        TaskManager->>Gio: kill()
        TaskManager->>Gio: waitForFinished(1000)
    end
Loading

Flow diagram for dock Crystal palette selection

flowchart LR
    Item[Dock app item] --> Selector[ColorSelector]
    Selector --> Family[Palette.CrystalColor]
    Family --> Colors[backgroundColor and border colors]
Loading

File-Level Changes

Change Details Files
Make asynchronous process ownership and cleanup safe across applet destruction and command timeouts.
  • Run the KWin version query with an unparented process and self-context cleanup.
  • Guard applet updates with QPointer and handle FailedToStart explicitly.
  • Kill and reap timed-out trash queries before stack destruction.
panels/dock/multitaskview/multitaskview.cpp
panels/dock/taskmanager/taskmanager.cpp
Defer DConfig-backed notification settings until the configuration service is available without masking genuinely invalid values.
  • Ignore invalid startup QVariant results while retaining defaults.
  • Watch for ConfigManager registration and refetch both bubble settings.
  • Continue warning for valid values below one.
panels/notification/common/notifysetting.cpp
Avoid display-mode DConfig warnings during daemon startup and retry after service registration.
  • Skip DConfig access while ConfigManager is absent and retain the default state.
  • Use a system-bus service watcher to trigger synchronization when the service appears.
panels/notification/osd/displaymode/displaymodeapplet.cpp
Prevent dock palette lookups from falling back to an unsupported color family.
  • Pin AppItemBackground's ColorSelector to Palette.CrystalColor, matching the palettes it defines.
panels/dock/taskmanager/package/AppItemBackground.qml

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

@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.

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨

Comment thread panels/notification/common/notifysetting.cpp Outdated
@Ivy233
Ivy233 force-pushed the wip/wall-batch3-unverified branch 2 times, most recently from a787cc2 to 09c6592 Compare October 22, 2026 12:12
@Ivy233 Ivy233 changed the title fix: address startup-race, object-lifetime and dock color warnings fix: address object-lifetime and dock color warnings Oct 22, 2026
@Ivy233
Ivy233 requested a review from 18202781743 October 22, 2026 12:23
@Ivy233
Ivy233 force-pushed the wip/wall-batch3-unverified branch from 09c6592 to 1451232 Compare October 23, 2026 09:30
process->deleteLater();
});
connect(process, &QProcess::finished, process, &QProcess::deleteLater);
connect(process, &QProcess::errorOccurred, process, [process](QProcess::ProcessError error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

这意思是,发送了errorOccureed信号,就不发送finished信号了?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

不是。errorOccurred 和 finished 不是互斥的,只有 FailedToStart 是例外:

  • FailedToStart:进程没起来,只发 errorOccurred(FailedToStart),不会发 finished;
  • Crashed:先发 errorOccurred(Crashed),随后仍发 finished(..., CrashExit);
  • Timedout/ReadError/WriteError:可能在进程还在运行时发 errorOccurred,finished 仍可能之后发出。

所以这里只在 errorOccurred 里处理 FailedToStart,其他错误统一由 finished 做 deleteLater;如果对所有 errorOccurred 都 deleteLater,Timedout 等场景可能在进程还在运行时析构 QProcess,反而重新触发 "Destroyed while process ... is still running"。

Qt 6.8 实测:FailedToStart 只有 errorOccurred;Crashed 是 errorOccurred + finished(CrashExit)。注释也已改成更准确的表述。

@Ivy233
Ivy233 force-pushed the wip/wall-batch3-unverified branch from 1451232 to e16f6bf Compare October 23, 2026 09:45
AppItemBackground overrides the base AppletItemBackground palettes with
crystal-only definitions, dropping the base's common fallback. While the
inherited ColorSelector family is still CommonColor (for example during
dock item creation), getColor() cannot resolve the crystal-only entries,
and reads of backgroundColor, insideBorderColor, outsideBorderColor and
the active* variants log "ColorSelector: The ... is an invalid color".
Add common transparent fallbacks to the normal group of the overridden
palettes, matching AppletItemBackground. The crystal rendering used by the
dock is unchanged.

dock AppItemBackground 颜色告警修复:

AppItemBackground 覆盖 AppletItemBackground 的 palette 时只定义了
crystal 颜色族,丢掉了基类的 common 兜底。当继承的 ColorSelector
family 仍为 CommonColor(例如 dock item 创建期间)时,getColor() 无法
解析仅含 crystal 的条目,读取 backgroundColor、insideBorderColor、
outsideBorderColor 及 active* 系列时会打印
"ColorSelector: ... is an invalid color"。现在为各覆盖 palette 的
normal 组补充 common 透明兜底,与 AppletItemBackground 保持一致;
dock 正常的 crystal 渲染不变。

PMS: TASK-394335
@Ivy233 Ivy233 changed the title fix: address object-lifetime and dock color warnings fix: add common palette fallback for dock app backgrounds Oct 23, 2026
@Ivy233
Ivy233 force-pushed the wip/wall-batch3-unverified branch from e16f6bf to ff973df Compare October 23, 2026 10:40
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 100 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 本次提交为 dde-shell 任务管理器应用背景组件添加 common 调色板回退属性,修复了在非 crystal 主题下背景颜色缺失的问题。变更逻辑正确、语法合规,无安全漏洞,代码质量良好。

🔍 详细分析

1. 语法逻辑 ✅

评价: 语法正确,逻辑清晰 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: []


2. 代码质量 ✅

评价: 代码结构清晰,注释完整 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: []


3. 代码性能 ✅

评价: 性能良好,资源使用合理 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: []


4. 代码安全 🔒

评价: 存在0个安全漏洞 ✅ 通过

🔐 发现 0 个安全漏洞

安全漏洞详情:
✅ 未发现安全漏洞

建议: []


💡 改进建议代码示例


本报告由 AI 代码审查工具自动生成

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