sync: from linuxdeepin/dde-session-shell

[deepin-ci-robot]

Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#66

[sourcery-ai]

Sorry @deepin-ci-robot, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[deepin-ci-robot]

deepin pr auto review

这段代码是对 LockContent::tryGrabKeyboard 函数中调用 D-Bus 通知接口的部分进行了修改。以下是对该 diff 的审查意见，包括语法逻辑、代码质量、代码性能和代码安全方面的建议：

1. 语法逻辑

• 拼写修正：

  • #ifndef ENABLE_DSS_SNIP 被修改为 #ifndef ENABLE_DSS_SNIPE。
  • 意见：这看起来像是一个宏定义的拼写修正。请确保项目中所有引用该宏的地方（CMakeLists.txt, .pro 文件或其他源文件）都已同步更新为 ENABLE_DSS_SNIPE，否则会导致预编译逻辑错误，使得部分代码块未被正确编译。

• 参数顺序与逻辑：

  • org.freedesktop.Notifications.Notify 方法的标准签名是 (susuassasa)。
  • 原代码在 #else 分支中（即 ENABLE_DSS_SNIPE 开启时），参数顺序为：appName ("dde-lock"), replaces_id (0), app_icon ("")。
  • 新代码在 #else 分支中，参数顺序变更为：appName ("dde-lock"), replaces_id (0), app_icon ("notification-lock-screen-failed")。
  • 意见：根据 Notify 接口标准，第 3 个参数是 app_icon（图标名称/路径）。将空字符串 "" 修改为具体的图标字符串 "notification-lock-screen-failed" 在逻辑上是合理的，目的是显示特定的失败图标。但在 #ifndef 分支（未开启 SNIPE）中，第 3 个参数依然是空字符串 ""。建议确认：在非 SNIPE 模式下是否不需要显示特定图标？如果需要，应同步修改 #ifndef 分支中的对应参数。

2. 代码质量

• 代码重复与维护性：
  • 问题：#ifndef 和 #else 分支中存在大量重复的参数调用（如 .arg(static_cast<uint>(0)) 和 .arg(QString("")) 的位置和类型）。
  • 建议：虽然这是 D-Bus 调用的链式写法，但过多的条件编译分支降低了可读性。建议将通用的参数提取出来，或者确保两个分支的差异仅在于必要的字符串内容（如 AppName 和 Icon），而不是参数的顺序或类型。
  • 示例改进：如果逻辑允许，可以尝试构建参数列表后再调用，或者明确注释为什么两个分支的图标参数处理不一致。

3. 代码性能

• 性能影响：
  • 这部分代码仅在"锁定屏幕失败"这一异常路径上执行，频率极低。
  • 涉及的 QString 构造和 static_cast 开销可以忽略不计。
  • 意见：无需针对性能进行特殊优化。

4. 代码安全

• 类型安全：
  • 代码使用了 .arg(static_cast<uint>(0))，这很好，确保了传递给 D-Bus 的 replaces_id 参数类型明确为无符号整数，避免了隐式转换可能带来的歧义。
• 国际化 (i18n)：
  • 代码中使用了 tr("Lock Screen") 和 tr("Failed to lock screen")，符合国际化规范，安全无虞。
• 输入验证：
  • 传递的字符串字面量（如 "dde-lock", "notification-lock-screen-failed"）是硬编码的，不存在注入风险。
  • 建议：请确保 "notification-lock-screen-failed" 是系统中确实存在的有效图标名称（或路径）。如果图标名称无效，通知可能无法显示预期的图标，但这属于资源/配置问题，不属于代码安全漏洞。

总结与改进建议

这段代码的主要目的是在特定编译宏（ENABLE_DSS_SNIPE）开启时，为锁屏失败的通知指定一个特定的图标。

建议修改后的代码逻辑（仅供参考，以保持一致性）：

如果非 SNIPE 模式下也应该显示图标，或者为了保持结构一致，建议统一处理。如果必须不同，请务必添加注释说明原因。

            // ... 前面的代码 ...
            .interface("org.freedesktop.Notifications")
            .method(QString("Notify"))
#ifndef ENABLE_DSS_SNIPE
            .arg(tr("Lock Screen"))          // App Name
            .arg(static_cast<uint>(0))       // Replaces ID
            .arg(QString("dialog-warning"))  // App Icon (建议非SNIPE模式下也指定一个通用图标，如dialog-warning)
#else
            .arg(QString("dde-lock"))        // App Name
            .arg(static_cast<uint>(0))       // Replaces ID
            .arg(QString("notification-lock-screen-failed")) // App Icon
#endif
            .arg(QString(""))                // Summary (标题)
            .arg(tr("Failed to lock screen")) // Body (正文)
            .arg(QStringList())              // Actions
            // ... 后面的代码 ...

特别注意：
原 diff 中 #ifndef 分支新增的两行：

            .arg(static_cast<uint>(0))
            .arg(QString(""))

实际上填补了之前该分支可能缺失的 replaces_id 和 app_icon 参数位置。请务必仔细核对 D-Bus 接口定义，确保参数顺序和数量在两个分支中都是严格匹配 (susuassasa) 签名的，否则会导致 D-Bus 调用失败或崩溃。目前的 diff 看起来是在修正参数对齐问题，这是正确的方向。

[deepin-bot]

TAG Bot

New tag: 6.0.57
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #503

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-ci-robot, yixinshark

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment