Skip to content

fix: address runtime warnings across shell and dock - #1727

Open
Ivy233 wants to merge 1 commit into
linuxdeepin:masterfrom
Ivy233:build/enable-wall-for-local-build
Open

Ivy233 wants to merge 1 commit into
linuxdeepin:masterfrom
Ivy233:build/enable-wall-for-local-build

Conversation

@Ivy233

@Ivy233 Ivy233 commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Fix compiler and QML runtime warnings across dde-shell and the dock.

The changes address:

  • Missing build-tree translations: emit .qm files directly into the build-tree package directory via qt_add_translations' QM_OUTPUT_DIRECTORY (compatible with Qt versions before 6.8), and skip missing plugin translation directories quietly in appletloader.cpp.
  • Unresolved Wayland QML base types (qmltyperegistrar warning): derive the dock Wayland extension classes (pluginmanagerextension, xdgactivationmanager) from the non-template Qt base classes (QWaylandCompositorExtension, QWaylandShellSurface) and implement extensionInterface() explicitly, so the base types can be resolved.
  • Invalid DConfig migration requests: correct the old-configuration subpath calculation in dconfigMigrate() (previously miscomputed from newFirstIndex, producing malformed resource paths such as /org.deepin.dde.network/).
  • Tooltip positioning: guard the tooltip position binding in SurfacePopup.qml against a temporarily null shellSurface.

Note: optimizing the logging for missing DConfig resources is handled on the dtkcore side (the two warnings Can't acquire config manager and DConfig is invalid are printed by dtkcore, and the meta lookup is dde-dconfig-daemon's internal logic), so no caller-side metadata check is added in this PR. The subpath calculation fix above is kept as it is a plain path-computation bug.

Log: fix runtime warnings across shell and dock
PMS: TASK-394335

Summary by Sourcery

Eliminate shell and dock compiler and runtime warnings by fixing translation handling, Wayland type resolution, DConfig migration paths, and transient tooltip state handling.

Bug Fixes:

  • Prevent invalid DConfig migrations and suppress warnings when configuration metadata or plugin translation directories are absent.
  • Resolve dock Wayland QML type warnings by using concrete Qt Wayland extension base classes.
  • Prevent tooltip positioning errors while the popup shell surface is temporarily unavailable.

Enhancements:

  • Place generated package translation files directly in the build-tree translation directory.

Build:

  • Configure Qt translation generation to emit QM files into each package's build-tree translation directory.

@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 Sep 9, 2026 •

Copy link
Copy Markdown

Reviewer's Guide

The PR aligns local CMake warning behavior with dpkg-buildpackage and hardens build-tree runtime behavior by improving translation placement, optional configuration checks, QML/Wayland metadata, drag previews, tooltip null handling, and D-Bus re-registration.

Sequence diagram for safe optional configuration migration

sequenceDiagram
    participant Shell
    participant Filesystem
    participant DConfig

    Shell->>Shell: dconfigMetaExists(newAppId, newName)
    Shell->>Shell: dconfigMetaExists(oldAppId, oldName)
    alt Either metadata file is missing
        Shell-->>Shell: return false
    else Both metadata files exist
        Shell->>DConfig: create(newAppId, newName, newPath)
        Shell->>DConfig: create(oldAppId, oldName, oldPath)
        DConfig-->>Shell: DConfig instances
        Shell->>DConfig: isValid()
    end
Loading

Sequence diagram for optional plugin translation loading

sequenceDiagram
    participant AppletLoader
    participant QDir
    participant QTranslator
    participant QApplication

    AppletLoader->>QDir: exists(pluginTranslationDir)
    alt Translation directory is missing
        AppletLoader-->>AppletLoader: Skip translation loading
    else Translation directory exists
        AppletLoader->>QTranslator: load(locale, pluginId, suffix, pluginTranslationDir)
        alt Matching translation loads
            QTranslator-->>AppletLoader: true
            AppletLoader->>QApplication: installTranslator(translator)
        else No matching translation loads
            QTranslator-->>AppletLoader: false
            AppletLoader-->>AppletLoader: deleteLater()
        end
    end
Loading

Flow diagram for safer dock drag previews and tooltip positioning

flowchart TD
    A[Drag starts] --> B["grabToImage(callback, Qt.size(item.width, item.height))"]
    B --> C[Set Drag.imageSource]
    D[Tooltip position binding] --> E{shellSurface exists?}
    E -->|Yes| F[Calculate surface-relative position]
    E -->|No| G[Use zero position]
Loading

File-Level Changes

Change Details Files
Align local CMake compilation diagnostics with Debian package builds.
  • Add -Wall and format-security warnings alongside existing -Werror for C and C++ targets.
(CMake build configuration; file path not included in supplied diff)
Make build-tree runtime resources and optional configuration handling more robust.
  • Emit translation QM files into the build-tree package directory.
  • Skip invalid KWin configuration access when the resource is unavailable.
  • Avoid warnings for absent plugin translation directories.
  • Skip DConfig migration when either metadata file is not installed.
cmake/DDEShellPackageMacros.cmake
panels/dock/multitaskview/multitaskview.cpp
shell/appletloader.cpp
shell/shell.cpp
Improve QML integration and drag-preview behavior.
  • Register Wayland protocol base-class metatypes for QML type generation.
  • Pass item dimensions explicitly when capturing drag images.
  • Guard tray tooltip positioning against a null shell surface.
panels/dock/CMakeLists.txt
panels/dock/waylandbase_metatypes.json
panels/dock/taskmanager/package/AppItem.qml
panels/dock/tray/package/ActionLegacyTrayPluginDelegate.qml
panels/dock/tray/quickpanel/DragItem.qml
panels/dock/tray/SurfacePopup.qml
Prevent stale D-Bus object registrations from breaking OSD panel initialization.
  • Unregister existing OSD and root object paths before registering the new panel instance.
panels/notification/osd/osdpanel.cpp
Update copyright years for modified shell and OSD sources.
  • Extend copyright notices through 2026.
panels/notification/osd/osdpanel.cpp
shell/shell.cpp

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 CMakeLists.txt Outdated
@Ivy233
Ivy233 marked this pull request as draft September 9, 2026 07:56
@deepin-bot

deepin-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown

TAG Bot

New tag: 2.0.54
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1733

@deepin-bot

deepin-bot Bot commented Sep 22, 2026

Copy link
Copy Markdown

TAG Bot

New tag: 2.0.55
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1752

@deepin-bot

deepin-bot Bot commented Sep 23, 2026

Copy link
Copy Markdown

TAG Bot

New tag: 2.0.56
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1753

@Ivy233
Ivy233 force-pushed the build/enable-wall-for-local-build branch 2 times, most recently from 85e8a59 to e37dfed Compare September 27, 2026 14:25
@Ivy233
Ivy233 marked this pull request as ready for review September 27, 2026 14:29
@Ivy233
Ivy233 requested a review from BLumia September 27, 2026 14:29

@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 found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="cmake/DDEShellPackageMacros.cmake" line_range="86-88" />
<code_context>
+    # Generate the qm files directly into the build-tree package directory, so
+    # that running dde-shell from the build tree can find the translations.
+    # (OUTPUT_LOCATION is honored by qt_add_translations since Qt 6.8.)
+    set_source_files_properties(${TRANSLATION_FILES}
+        PROPERTIES OUTPUT_LOCATION "${package_dirs}/translations"
+    )

     add_custom_target(${_config_PACKAGE}_translation ALL
</code_context>
<issue_to_address>
**issue (broader_impact):** On supported Qt versions where `qt_add_translations` does not honor `OUTPUT_LOCATION` (Qt 5 and Qt 6 before 6.8), the generated `.qm` files are not placed in the build-tree package's `translations` directory, so running dde-shell from the build tree still cannot load these translations.

**Triggers:** When the project is built with Qt 5 or Qt 6 older than 6.8.

**Suggested fix:** Require Qt 6.8 or newer for this behavior, or retain a version-specific output configuration for older Qt versions.
</issue_to_address>

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

Comment thread cmake/DDEShellPackageMacros.cmake Outdated
@Ivy233
Ivy233 force-pushed the build/enable-wall-for-local-build branch 3 times, most recently from aec9962 to 4322d26 Compare September 29, 2026 08:08
Comment thread panels/notification/osd/osdpanel.cpp Outdated
Comment thread shell/shell.cpp Outdated
Comment thread panels/dock/multitaskview/multitaskview.cpp Outdated
@Ivy233
Ivy233 force-pushed the build/enable-wall-for-local-build branch from 4322d26 to d8e9dd1 Compare September 30, 2026 06:19
@Ivy233 Ivy233 changed the title build: align local compile flags with dpkg-buildpackage fix: address runtime warnings across shell and dock Sep 30, 2026
@Ivy233
Ivy233 requested a review from 18202781743 September 30, 2026 06:28
Fix runtime and build warnings in dde-shell:

- Generate package translations in the build-tree package directory so local runs can load them.
- Derive the dock Wayland extension classes directly from the non-template Qt base classes (QWaylandCompositorExtension, QWaylandShellSurface) and implement extensionInterface() explicitly, so qmltyperegistrar can resolve their base types.
- Skip missing plugin translation directories without warning.
- Correct the old configuration subpath calculation in DConfig migrations.
- Guard tooltip positioning when the tray shell surface is temporarily null.

修复 dde-shell 及任务栏的运行时和构建告警:

- 将插件翻译直接生成到构建树 package 目录,确保本地运行时能够加载。
- dock 的 Wayland 扩展类改为直接继承非模板 Qt 基类(QWaylandCompositorExtension、QWaylandShellSurface),并显式实现 extensionInterface(),使 qmltyperegistrar 能够解析其基类类型。
- 插件翻译目录不存在时跳过加载,避免产生告警。
- 修正 DConfig 迁移中旧配置 subpath 的计算。
- tray shell surface 暂时为空时保护 tooltip 位置计算。

PMS: TASK-394335
@Ivy233
Ivy233 force-pushed the build/enable-wall-for-local-build branch 5 times, most recently from d8e9dd1 to 915aab7 Compare October 3, 2026 15:26
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

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

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 总体评分 98 分,大于 70 分通过阈值,代码质量符合要求。本次 PR 修复了 dde-shell 和 dock 的编译器和运行时警告,涉及翻译文件输出、Wayland QML 类型解析、DConfig 迁移路径计算和 Tooltip 空指针保护,代码修改正确且与 commit 目的一致。

🔍 详细分析

1. 语法逻辑 ✅

评价: 优秀 ✅ 通过

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

建议: 语法正确,逻辑清晰,无需修改


2. 代码质量 ✅

评价: 优秀 ✅ 通过

潜在问题:

  1. panels/dock/pluginmanagerextension_p.h:53 - PluginScale 仍使用 QWaylandCompositorExtensionTemplate 模板基类但添加了冗余的 extensionInterface() override,与其他类迁移到非模板基类的做法不一致

建议: 建议将 PluginScale 也迁移到 QWaylandCompositorExtension 非模板基类以保持一致性,或移除冗余的 extensionInterface() override


3. 代码性能 ✅

评价: 优秀 ✅ 通过

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

建议: 性能良好,资源使用合理,无需优化


4. 代码安全 🔒

评价: 优秀 ✅ 通过

🔐 发现 0 个安全漏洞

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

建议: 安全合规,无需安全加固


💡 改进建议代码示例

// 建议:将 PluginScale 也迁移到非模板基类,保持一致性
class PluginScale : public QWaylandCompositorExtension, public QtWaylandServer::wp_fractional_scale_v1
{
    Q_OBJECT
public:
    PluginScale(PluginScaleManager *manager, QWaylandSurface *surface, const QWaylandResource &resource);
    const struct wl_interface *extensionInterface() const override
    {
        return QtWaylandServer::wp_fractional_scale_v1::interface();
    }
    void wp_fractional_scale_v1_destroy(Resource *resource) override;
};

// 同时在 pluginmanagerextension.cpp 中更新构造函数:
PluginScale::PluginScale(PluginScaleManager *manager, QWaylandSurface *surface, const QWaylandResource &resource)
    : QWaylandCompositorExtension(nullptr)
    , m_manager(manager)
    , m_surface(surface)
{
    init(resource.resource());
    setExtensionContainer(surface);
    QWaylandCompositorExtension::initialize();
}

本报告由 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.

4 participants