Skip to content

fix: elide placeholder text within textMargins in DLineEditEx - #89

Closed
mhduiy wants to merge 1 commit into
linuxdeepin:masterfrom
mhduiy:agent/pms-bug-bot/8ea37c6f5f07
Closed

mhduiy wants to merge 1 commit into
linuxdeepin:masterfrom
mhduiy:agent/pms-bug-bot/8ea37c6f5f07

Conversation

@mhduiy

@mhduiy mhduiy commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Root Cause Analysis

DLineEditEx::paintEvent() uses rect().width() (the full widget width) as the elision width when drawing placeholder text, ignoring the textMargins that AuthPassword::updatePasswordTextMargins() has already set on the internal QLineEdit to account for right-side icons (capslock status, password visibility toggle, password hint). This causes the elided placeholder text to extend into the icon area and visually overlap with the icons. Key evidence: dlineeditex.cpp:163 uses rect().width() while auth_password.cpp:972-991 correctly sets textMargins — but paintEvent never reads them.

Fix

Read lineEdit()->geometry() and lineEdit()->textMargins() in paintEvent() to compute the effective text rect (subtracting icon-occupied margins), then use textRect.width() for elision and draw within textRect. This dynamically adapts to any icon configuration. The implementation matches the analysis report's fix suggestion with no deviation.

Change Safety Assessment

Code Safety

  • Risk Level: Low
  • The elision code (lines 159-171) was introduced by commit 2c6d4497 to fix BUG-322639 (long placeholder text wrapping/abnormal display). This fix does NOT revert that behavior — it only corrects the width parameter passed to elidedText, keeping the elision and tooltip logic intact.
  • No external callers found (References=0); paintEvent is a Qt virtual override invoked by the paint system.

Business Impact Scope

Affected module: Lock screen / Login password field — placeholder text rendering when the password field is empty and focused (e.g., error prompts like "Verification failed, 4 chances left"). The fix ensures the placeholder text is elided and drawn within the area that excludes the right-side icon buttons, preventing visual overlap.

Verification Suggestion

Test the lock screen / login password field: enter a wrong password, then verify the error placeholder text (after the field clears) does not overlap with the capslock, password visibility, or password hint icons. Also verify that DLineEditEx without icons still displays placeholder text correctly centered.


根因分析

DLineEditEx::paintEvent() 在绘制占位文本时使用 rect().width()(控件全宽)作为省略宽度,忽略了 AuthPassword::updatePasswordTextMargins() 已在内部 QLineEdit 上设置的 textMargins(用于为右侧图标——大写状态、密码显示、密码提示——预留空间)。这导致省略后的占位文本延伸到图标区域,与右侧图标视觉重叠。关键证据:dlineeditex.cpp:163 使用 rect().width(),而 auth_password.cpp:972-991 已正确设置 textMargins,但 paintEvent 从未读取它。

修复方案

在 paintEvent() 中读取 lineEdit()->geometry() 和 lineEdit()->textMargins(),计算扣除图标占用边距后的有效文本 rect,用 textRect.width() 做省略并在 textRect 内居中绘制。此方案动态适应任意图标配置,与分析报告的修复建议一致,无偏离。

改动安全评估

代码安全评估

  • 风险等级: 低风险
  • 省略逻辑(第 159-171 行)由 commit 2c6d4497 引入以修复 BUG-322639(长占位文本换行/显示异常)。本次修复不会撤销该行为——仅修正传入 elidedText 的宽度参数,保留省略和 ToolTip 逻辑不变。
  • 无外部调用者(References=0);paintEvent 为 Qt 虚函数重写,由绘制系统调用。

业务影响范围

受影响模块:锁屏/登录密码框——密码框为空且有焦点时的占位文本绘制(如"Verification failed, 4 chances left"错误提示)。修复确保占位文本在扣除右侧图标按钮区域后进行省略和绘制,避免视觉重叠。

验证建议

测试锁屏/登录密码框:输入错误密码后验证密码框清空时错误提示占位文本不与大写状态、密码显示、密码提示图标重叠;同时验证无图标的 DLineEditEx 占位文本仍正常居中显示。

PMS: BUG-351887

Summary by Sourcery

Keep DLineEditEx placeholder text within the usable field area when margins or embedded controls reduce the available width.

Bug Fixes:

  • Prevent placeholder text in password fields from overlapping right-side icons by eliding and drawing it within the available text area.

Enhancements:

  • Account for both text margins and layout content margins when sizing placeholder text and calculating its display rectangle.

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

Sorry @mhduiy, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 5 hours and 43 minutes by commenting @sourcery-ai review. Upgrade to get a review now.

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mhduiy

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 14, 2026

Copy link
Copy Markdown
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Updates DLineEditEx placeholder painting to account for QLineEdit text margins, preventing long placeholder text from overlapping right-side icons while preserving existing elision and tooltip behavior.

Sequence diagram for margin-aware placeholder painting

sequenceDiagram
    participant PaintSystem
    participant DLineEditEx
    participant QLineEdit

    PaintSystem->>DLineEditEx: paintEvent(event)
    DLineEditEx->>QLineEdit: placeholderText()
    DLineEditEx->>QLineEdit: geometry()
    DLineEditEx->>QLineEdit: textMargins()
    DLineEditEx->>DLineEditEx: elidedText(placeholderText, Qt::ElideRight, textRect.width())
    DLineEditEx->>DLineEditEx: drawText(textRect, Qt::AlignCenter | Qt::TextSingleLine, elidedText)
Loading

File-Level Changes

Change Details Files
Constrain placeholder elision and rendering to the internal line edit’s usable text area.
  • Read the internal line edit geometry and text margins.
  • Build a text rectangle that excludes left and right margin space reserved for icons.
  • Use the constrained width for right elision and draw the placeholder within that rectangle.
  • Preserve existing tooltip behavior for elided placeholder text.
src/widgets/dlineeditex.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

1. Root cause: first fix (BUG-351887) read textMargins but missed the
   passwordLayout contentsMargins(10,0,10,0) set in AuthPassword::initUI,
   leaving ~10px gap on the right where text still overlapped icons
2. Fix: in paintEvent(), combine textMargins + layout()->contentsMargins()
   to compute the effective text rect; in setPlaceholderTextFont(), use
   the same effective width as the font-shrink threshold
3. Impact: only affects placeholder text rendering when input is empty
   and focused; normal text input/display unaffected

Log: 修复占位文本省略宽度未包含 layout contentsMargins 导致仍与图标重叠

Influence:
1. 测试输入错误密码后密码框清空时占位提示文本不与图标重叠
2. 验证大写状态、密码显示、密码提示图标区域无文本覆盖
3. 测试无图标的 DLineEditEx 占位文本正常居中显示

PMS: BUG-351887
@mhduiy
mhduiy force-pushed the agent/pms-bug-bot/8ea37c6f5f07 branch from e2e3c43 to 1dac660 Compare September 22, 2026 14:29
deepin-ci-robot added a commit to linuxdeepin/dde-session-shell-snipe that referenced this pull request Sep 22, 2026
Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#89
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

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

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
PR linuxdeepin/dde-session-shell#89
Commit fix: include layout contentsMargins in placeholder text elision
PMS BUG-351887
修改文件 src/widgets/dlineeditex.cpp (+24/-3)
评分详情 代码修复了占位文本与右侧图标重叠的问题,逻辑正确,无安全漏洞。存在少量代码质量和边界条件改进建议。

🔍 详细分析

1. 语法逻辑 ✓(22/25分)

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

潜在问题:

  1. src/widgets/dlineeditex.cpp:89-91,函数 setPlaceholderTextFont:availWidth 可能为 0 或负值(当 textMargins + layoutMargins 之和超过 width() 时,例如控件尚未完成布局或尺寸极小),导致 while 循环条件 QFontMetrics(fontTmp).boundingRect(text).width() > availWidth 始终为真,函数在 fontTmp.pointSize() <= 1 时提前返回而不调用 setFont。虽然已有的 pointSize() <= 1 守卫防止了死循环,但建议增加 availWidth <= 0 的提前返回守卫以增强健壮性。——非常重要

建议:

int availWidth = width() - textMargins.left() - textMargins.right()
               - layoutMargins.left() - layoutMargins.right();
if (availWidth <= 0) {
    qWarning() << "DLineEditEx available width is non-positive:" << availWidth;
    return;
}
while (QFontMetrics(fontTmp).boundingRect(text).width() > availWidth) {

2. 代码质量 ✓(20/25分)

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

潜在问题:

  1. src/widgets/dlineeditex.cpp:84-88 和 174-178,函数 setPlaceholderTextFont / paintEvent:textMargins + layoutMargins 的计算逻辑在两个函数中重复,建议抽取为辅助方法(如 effectiveTextWidth() 或 availableTextRect())以提高可维护性。——非常重要
  2. src/widgets/dlineeditex.cpp:94,函数 setPlaceholderTextFont:qDebug 日志输出 width() 作为 "line edit width",但 while 循环实际比较的是 availWidth,调试信息与实际逻辑不一致,可能误导调试。建议将 width() 改为 availWidth。——非常重要

建议:

// 抽取公共计算逻辑为辅助方法
int DLineEditEx::effectiveTextWidth() const
{
    QMargins textMargins = lineEdit()->textMargins();
    QMargins layoutMargins(0, 0, 0, 0);
    if (auto *layout = lineEdit()->layout()) {
        layoutMargins = layout->contentsMargins();
    }
    return width() - textMargins.left() - textMargins.right()
                   - layoutMargins.left() - layoutMargins.right();
}

// 修复 qDebug 日志
qDebug() << "Password line edit placeholder text width : "
         << QFontMetrics(fontTmp).boundingRect(text).width()
         << " available width : " << availWidth;

3. 代码性能 ✓(20/20分)

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

潜在问题:
✅ 未发现性能问题

新增的 textMargins()、layout()、contentsMargins()、geometry() 调用均为轻量级属性读取,不会产生性能影响。


4. 代码安全 ✓(30/30分)

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

🔐 发现 0 个安全漏洞

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

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个


📝 代码变更概述

本次 PR 修复了 BUG-351887,解决 DLineEditEx 中占位文本与右侧图标(大写状态、密码显示、密码提示)视觉重叠的问题。

修改点 1:setPlaceholderTextFont()(第 79-102 行)

  • 原代码使用 width()(控件全宽)作为字体缩小阈值
  • 新代码计算 availWidth,扣除 textMargins 和 layout contentsMargins,确保字体缩小的判断基于实际可用文本宽度

修改点 2:paintEvent()(第 154-195 行)

  • 原代码使用 rect().width() 做文本省略,用 rect() 绘制
  • 新代码计算 textRect,基于 lineEdit()->geometry() 并扣除 margins,确保省略和绘制都在有效文本区域内进行

审查结论:
代码修复与 commit message 描述的目的一致,正确实现了 BUG-351887 的修复方案。逻辑清晰,注释完整,无安全风险。建议处理上述轻微的边界条件和代码质量问题以进一步提升健壮性。


本报告由 AI 代码审查工具自动生成
扫描时间:2026-09-22 22:32:00

@mhduiy

mhduiy commented Sep 24, 2026

Copy link
Copy Markdown
Contributor Author

已创建替代 PR #99(https://github.com/linuxdeepin/dde-session-shell/pull/99),使用主仓库分支而非 fork 分支,解决 cppcheck CI 因 fork PR 安全限制无法运行的问题。请在新 PR 上审核合并。

@mhduiy mhduiy closed this Sep 24, 2026
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.

2 participants