Skip to content

sync: from linuxdeepin/dde-session-shell #423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

deepin-ci-robot
Copy link
Contributor

@deepin-ci-robot deepin-ci-robot commented May 22, 2025

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

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

Summary by Sourcery

Incorporate upstream session-shell enhancements to temporarily switch to a default US keyboard layout on the login screen when a custom layout is detected and to improve inhibitor warning icons by fetching them via the desktop-specific DBus service with fallbacks.

Enhancements:

  • Temporarily switch to US keyboard layout on greeter when a special layout is configured, storing and restoring the original layout after authentication.
  • Enhance inhibitor warning view to fetch application icons via DSGApplication DBus interface under DSS_SNIPE, falling back to theme icons based on the executable name.

@deepin-ci-robot
Copy link
Contributor Author

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

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

Copy link

sourcery-ai bot commented May 22, 2025

Reviewer's Guide

This PR syncs source from linuxdeepin/dde-session-shell, adding automatic keyboard layout switching on the lock screen for special layouts and improving inhibitor warning view icon resolution via DesktopSpec integration.

Sequence Diagram for Keyboard Layout Management at Login

sequenceDiagram
    actor User
    participant LockContent
    participant OS_Keyboard_Settings

    LockContent->>LockContent: init(model)
    alt Login Screen and Special Keyboard Layout is detected
        LockContent->>LockContent: keyboardLayoutHasSpecialSetting() returns true
        LockContent->>OS_Keyboard_Settings: getCurrentKBLayoutAndVariant()
        activate OS_Keyboard_Settings
        OS_Keyboard_Settings-->>LockContent: originalLayout
        deactivate OS_Keyboard_Settings
        LockContent->>LockContent: store originalLayout in m_originalKBLayout
        LockContent->>OS_Keyboard_Settings: setKBLayoutAndVariant("us")
        activate OS_Keyboard_Settings
        OS_Keyboard_Settings-->>LockContent: success
        deactivate OS_Keyboard_Settings
    end

    User->>LockContent: Authenticates
    LockContent->>LockContent: authFinished(successful=true)
    alt Original Layout Was Saved (m_originalKBLayout is not empty)
        LockContent->>OS_Keyboard_Settings: setKBLayoutAndVariant(m_originalKBLayout)
        activate OS_Keyboard_Settings
        OS_Keyboard_Settings-->>LockContent: success
        deactivate OS_Keyboard_Settings
    end
    LockContent->>User: Login successful, UI updated
Loading

Sequence Diagram for D-Bus Based Icon Fetching in InhibitWarnView

sequenceDiagram
    participant IWV as InhibitWarnView
    participant DSGA as DSGApplication
    participant AMS as "AppManager Service (D-Bus)"

    Note over IWV: In setInhibitorList, for inhibitor with no icon & ENABLE_DSS_SNIPE
    IWV->>DSGA: getId(pid)
    activate DSGA
    DSGA-->>IWV: appId
    deactivate DSGA

    opt appId is valid
        IWV->>AMS: GetProperty("Icons") via DDBusSender
        activate AMS
        AMS-->>IWV: iconsData (QMap)
        deactivate AMS
        IWV->>IWV: iconName = iconsData.value("Desktop Entry")
    end
Loading

Updated Class Diagram for LockContent

classDiagram
    class LockContent {
        -m_originalKBLayout : QString
        +init(model: SessionBaseModel*) : void
        +initConnections() : void
        +keyboardLayoutHasSpecialSetting() : bool
        +getCurrentKBLayoutAndVariant() : QString
        +setKBLayoutAndVariant(layoutVariant: const QString &) : void
    }
Loading

File-Level Changes

Change Details Files
Automatic keyboard layout switching based on special settings in the lock screen
  • Detect special keyboard configuration in /etc/default/keyboard at init and store original layout
  • Switch to English layout ('us') for login greeter when special layout is present
  • Restore original layout after successful authentication
  • Introduce helper methods: keyboardLayoutHasSpecialSetting, getCurrentKBLayoutAndVariant, setKBLayoutAndVariant
  • Add m_originalKBLayout member and corresponding header declarations
src/session-widgets/lockcontent.cpp
src/session-widgets/lockcontent.h
Enhanced inhibitor warning view to fetch icons via DesktopSpec when enabled
  • Wrap DesktopSpec icon resolution under ENABLE_DSS_SNIPE flag with DSGApplication and DDBusSender
  • Fallback to extracting executable name from /proc//exe and using theme icon
  • Add necessary includes and D-Bus service/interface definitions
src/widgets/inhibitwarnview.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

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @deepin-ci-robot - I've reviewed your changes - here's some feedback:

  • Avoid blocking the UI thread with synchronous QProcess calls for setxkbmap and -query; consider using asynchronous processes or startDetached to prevent freezes.
  • In InhibitWarnView you shadow iconName inside the nested block, which can lead to empty or incorrect icons—use a single variable scope to accumulate the icon name instead.
Here's what I looked at during the review
  • 🟡 General issues: 4 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

if (executable_info.exists()) {
icon = QIcon::fromTheme(executable_info.fileName());
// 玲珑应用的exe指向的路径是容器内的路径,在容器外无法访问,因此不能判断文件是否存在
QString iconName = executable_info.fileName();
Copy link

Choose a reason for hiding this comment

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

issue (bug_risk): Remove redundant redeclaration of iconName to avoid shadowing

The inner declaration creates a new variable, so the filename isn't assigned to the outer iconName. Use 'iconName =' instead to update the existing variable.

@@ -86,16 +98,41 @@
for (const InhibitorData &inhibitor : list) {
QIcon icon;

if (inhibitor.icon.isEmpty() && inhibitor.pid) {
if (inhibitor.icon.isEmpty()) {
Copy link

Choose a reason for hiding this comment

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

suggestion (bug_risk): Restore inhibitor.pid check to maintain correct fallback behavior

Without the pid check, cases with an empty PID may bypass the fallback logic, resulting in no icon. Please restore the && inhibitor.pid condition.

Suggested change
if (inhibitor.icon.isEmpty()) {
if (inhibitor.icon.isEmpty() && inhibitor.pid) {

{
QProcess p;
p.start("/usr/bin/setxkbmap", {"-query"});
p.waitForFinished();
Copy link

Choose a reason for hiding this comment

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

issue (bug_risk): Add a timeout and error check to waitForFinished

Using waitForFinished() without a timeout can cause the process to hang if it stalls. Please use the timeout overload and check the process exit status and standard error to handle possible failures.

return variant.isEmpty() ? layout : layout + "+" + variant;
}

void LockContent::setKBLayoutAndVariant(const QString &layoutVariant)
Copy link

Choose a reason for hiding this comment

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

suggestion (bug_risk): Validate layoutVariant parts before invoking setxkbmap

Sanitize each part of layoutVariant to ensure only valid values are passed to setxkbmap, and check for exactly one + separator.

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

Source-pull-request: linuxdeepin/dde-session-shell#10
@mhduiy mhduiy force-pushed the sync-pr-10-nosync branch from b0f0b9e to cedcab3 Compare May 28, 2025 08:30
@deepin-ci-robot
Copy link
Contributor Author

deepin pr auto review

关键摘要:

  • init函数中,新增的键盘布局切换逻辑应该有相应的错误处理机制,以防setKBLayoutAndVariant执行失败。
  • keyboardLayoutHasSpecialSetting函数中,读取配置文件时没有处理可能出现的异常情况,例如文件不存在或读取失败。
  • getCurrentKBLayoutAndVariant函数中,使用QProcess执行命令获取键盘布局信息,应该添加错误处理逻辑,以防命令执行失败。
  • setKBLayoutAndVariant函数中,直接执行系统命令切换键盘布局,没有检查命令执行结果,可能会导致键盘布局设置不成功。
  • 新增的函数keyboardLayoutHasSpecialSettinggetCurrentKBLayoutAndVariantsetKBLayoutAndVariant应该添加注释说明其用途和功能。
  • init函数中,m_originalKBLayout的初始化应该放在构造函数中,而不是在init函数中。

是否建议立即修改:

@deepin-bot
Copy link

deepin-bot bot commented May 28, 2025

TAG Bot

New tag: 6.0.39
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #425

@deepin-bot
Copy link

deepin-bot bot commented Jun 12, 2025

TAG Bot

New tag: 6.0.40
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #430

@deepin-bot
Copy link

deepin-bot bot commented Jun 23, 2025

TAG Bot

New tag: 6.0.41
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #432

@deepin-bot
Copy link

deepin-bot bot commented Jul 3, 2025

TAG Bot

New tag: 6.0.42
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #435

@deepin-bot
Copy link

deepin-bot bot commented Jul 11, 2025

TAG Bot

New tag: 6.0.43
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #443

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.

1 participant