Repository navigation
Conversation
onApplicationPaletteChanged() warned whenever a custom palette was in use, but isCustomPalette() is also true while DTK itself manages the palette (paletteType != UnknownType). DTK's own setPaletteType() path deliberately triggers processThemeChanged() to make Qt re-fetch the platform palette, and the resulting ApplicationPaletteChange event was then misjudged as the application calling QGuiApplication::setPalette, printing a false-positive warning for a normal public API call (e.g. dde-tray-loader reacting to dock color theme changes). Qt sets Qt::AA_SetPalette when the application really calls QGuiApplication::setPalette, and never sets it for DTK's internal theme-change path, so use it as the criterion -- consistent with the other two checks in this file (setPaletteType, setApplicationPalette). Verified with a minimal program under the deepin platform theme: before, both DTK's own setPaletteType and a direct QGuiApplication::setPalette warned; after, setPaletteType is silent while direct setPalette abuse still warns. 修复：仅在 AA_SetPalette 置位时才对 setPalette 滥用告警 onApplicationPaletteChanged() 原先只要处于自定义调色板就告警，但 isCustomPalette() 在 DTK 自身管理调色板（paletteType != UnknownType） 时同样为真。DTK 自己的 setPaletteType() 会有意触发 processThemeChanged() 让 Qt 重新获取平台调色板，由此产生的 ApplicationPaletteChange 事件被误判为应用调用了 QGuiApplication::setPalette，导致正常公开 API 调用（如 dde-tray-loader 响应 dock 颜色主题变化）也误报告警。 Qt 在应用真正调用 QGuiApplication::setPalette 时会置位 Qt::AA_SetPalette，而 DTK 内部主题变更路径永远不会置位，因此改用 该属性作为判据——与本文件另外两处检查（setPaletteType、 setApplicationPalette）保持一致。 已在 deepin 平台主题下用最小程序验证：修复前 DTK 自身 setPaletteType 与直接 QGuiApplication::setPalette 均告警；修复后 setPaletteType 静默，直接 setPalette 滥用仍告警。 PMS: TASK-394335
Contributor
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates palette-change handling to distinguish application-driven QGuiApplication::setPalette() calls from DTK-managed theme refreshes by checking Qt::AA_SetPalette, eliminating false-positive warnings without weakening detection of actual palette abuse. Sequence diagram for palette-change warning discriminationsequenceDiagram
participant App as Application
participant Qt as QGuiApplication
participant DTK as DGuiApplicationHelper
App->>DTK: setPaletteType()
DTK->>DTK: processThemeChanged()
DTK->>Qt: ApplicationPaletteChange
Qt-->>DTK: testAttribute(Qt::AA_SetPalette) = false
DTK-->>App: No warning
App->>Qt: setPalette()
Qt->>DTK: ApplicationPaletteChange
Qt-->>DTK: testAttribute(Qt::AA_SetPalette) = true
DTK-->>App: qWarning()
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}
Contributor
deepin pr auto review🤖 AI 代码审查报告总体评分： 📊 总体评价
🔍 详细分析1. 语法逻辑 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 语法正确，逻辑清晰。if-else if 链正确地将告警条件从 isCustomPalette() 缩窄为 Qt::AA_SetPalette 属性检查。qGuiApp 在同一函数的第459行已被解引用（qGuiApp->palette()），此处使用安全。边界处理完善：当 isCustomPalette() 为 true 但 AA_SetPalette 未置位时，函数静默返回，符合 DTK 内部主题变更的正常流程。 2. 代码质量 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 代码结构清晰，注释完整。新增的4行注释详细解释了使用 AA_SetPalette 作为判据的原因，并引用了 notifyAppThemeChangedByEvent() 函数说明 DTK 内部流程。修改与文件中 setPaletteType()（第480行）和 setApplicationPalette()（第1353行）两处类似检查保持一致，无重复代码，无残留调试信息。 3. 代码性能 ✅评价： 优秀 ✅ 通过 潜在问题： 建议： 性能良好，资源使用合理。testAttribute() 是 Qt 内联方法，时间复杂度 O(1)，仅为属性标志位检查。本次修改还减少了不必要的 qWarning() 调用（消除误报），对性能有轻微正面影响。 4. 代码安全 🔒评价： 优秀 ✅ 通过
安全漏洞详情： 建议： 存在0个安全漏洞，安全合规。本次修改不涉及用户输入处理、外部数据交互或敏感信息操作。qWarning() 输出内容为固定字符串，不含敏感信息，无日志注入风险。无缓冲区溢出、注入或其他安全风险。 💡 改进建议代码示例// 当前修改已为最佳实践，无需额外修复
// 修改前（存在误报）：
// } else {
// qWarning() << "DGuiApplicationHelper: Don't use QGuiApplication::setPalette on DTK application.";
// }
// 修改后（精准告警）：
// } else if (qGuiApp->testAttribute(Qt::AA_SetPalette)) {
// // 仅在调色板变化确由应用自行调用 QGuiApplication::setPalette 引发
// qWarning() << "DGuiApplicationHelper: Don't use QGuiApplication::setPalette on DTK application.";
// }本报告由 AI 代码审查工具自动生成 |
Contributor
|
TAG Bot New tag: 6.7.51 |
This branch has not been deployed
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
2 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.
onApplicationPaletteChanged() warned whenever a custom palette was in use, but isCustomPalette() is also true while DTK itself manages the palette (paletteType != UnknownType). DTK's own setPaletteType() path deliberately triggers processThemeChanged() to make Qt re-fetch the platform palette, and the resulting ApplicationPaletteChange event was then misjudged as the application calling QGuiApplication::setPalette, printing a false-positive warning for a normal public API call (e.g. dde-tray-loader reacting to dock color theme changes).
Qt sets Qt::AA_SetPalette when the application really calls QGuiApplication::setPalette, and never sets it for DTK's internal theme-change path, so use it as the criterion -- consistent with the other two checks in this file (setPaletteType, setApplicationPalette).
Verified with a minimal program under the deepin platform theme: before, both DTK's own setPaletteType and a direct QGuiApplication::setPalette warned; after, setPaletteType is silent while direct setPalette abuse still warns.
修复：仅在 AA_SetPalette 置位时才对 setPalette 滥用告警
onApplicationPaletteChanged() 原先只要处于自定义调色板就告警，但
isCustomPalette() 在 DTK 自身管理调色板（paletteType != UnknownType） 时同样为真。DTK 自己的 setPaletteType() 会有意触发
processThemeChanged() 让 Qt 重新获取平台调色板，由此产生的
ApplicationPaletteChange 事件被误判为应用调用了
QGuiApplication::setPalette，导致正常公开 API 调用（如
dde-tray-loader 响应 dock 颜色主题变化）也误报告警。
Qt 在应用真正调用 QGuiApplication::setPalette 时会置位
Qt::AA_SetPalette，而 DTK 内部主题变更路径永远不会置位，因此改用
该属性作为判据——与本文件另外两处检查（setPaletteType、
setApplicationPalette）保持一致。
已在 deepin 平台主题下用最小程序验证：修复前 DTK 自身
setPaletteType 与直接 QGuiApplication::setPalette 均告警；修复后 setPaletteType 静默，直接 setPalette 滥用仍告警。
PMS: TASK-394335
Summary by Sourcery
Bug Fixes: