Skip to content

fix(popup): remove focus workaround - #658

Open
mhduiy wants to merge 1 commit into
linuxdeepin:masterfrom
mhduiy:bug-372273-popup-input-method-focus
Open

fix(popup): remove focus workaround#658
mhduiy wants to merge 1 commit into
linuxdeepin:masterfrom
mhduiy:bug-372273-popup-input-method-focus

Conversation

@mhduiy

@mhduiy mhduiy commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • remove native popup activation and parent focus restoration from PopupHandle
  • remove ComboBox focus-event filtering now that QtBase owns popup input focus propagation
  • remove the obsolete workaround test while retaining popup handle creation coverage

Verification

  • rebuilt unit-test
  • ut_DPopupWindowHandle.popupWindowCreatesHandle: passed
  • ut_DComboBox.popupTypeSelectsMatchingBackground: passed

Requires: deepin-community/qt6-base#55
PMS: https://pms.uniontech.com/bug-view-372273.html

Summary by Sourcery

Simplify popup window handling now that QtBase manages popup focus, and clean up related tests.

Bug Fixes:

  • Remove custom focus transfer handling between ComboBox and popup windows that is now redundant with QtBase popup focus propagation.

Enhancements:

  • Drop obsolete focus ownership tracking and event filtering from DPopupWindowHandle to rely on standard Qt focus behavior.
  • Remove the legacy ComboBox focus workaround test while keeping coverage of popup handle creation.

1. Remove native popup activation and parent focus restoration from PopupHandle.
2. Drop ComboBox focus event filtering after moving popup input focus handling to QtBase.
3. Remove the obsolete workaround test while retaining popup handle creation coverage.

Log: Let QtBase manage logical input focus for non-activating popups.
Influence: Popup input requires the matching QtBase fix.

fix(popup): 移除焦点变通逻辑

1. 移除 PopupHandle 对原生弹窗的激活和父级焦点恢复逻辑。
2. 输入焦点处理迁移到 QtBase 后,删除 ComboBox 焦点事件过滤。
3. 删除过时的变通测试,同时保留弹窗句柄创建覆盖。

Log: 由 QtBase 统一管理非激活弹窗的逻辑输入焦点。
PMS: BUG-372273
Influence: 弹窗输入依赖配套的 QtBase 修复。

@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 have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot

Copy link
Copy Markdown
Contributor

[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 Aug 13, 2026

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

Reviewer's Guide

Removes the custom popup focus management and ComboBox-specific focus workaround now that QtBase owns popup input focus handling, and cleans up the associated test and dependencies while keeping basic popup handle behavior covered.

File-Level Changes

Change Details Files
Simplified DPopupWindowHandle by dropping custom focus request/restore logic and ComboBox-specific focus event filtering, delegating focus handling entirely to QtBase.
  • Removed includes and usage of QPointer for temporary focus restore tracking and QWindowSystemInterface for manual focus window changes
  • Deleted requestPopupFocus() and restoreParentFocus() methods and the associated state members tracking focus owner and restore item
  • Stopped listening to popup visibility changes and removed restoreParentFocus() calls on window change
  • Removed special-case eventFilter handling that suppressed FocusOut events for ComboBox owners while the popup window had focus
src/private/dpopupwindowhandle.cpp
src/private/dpopupwindowhandle_p.h
Cleaned up unit tests by removing the obsolete ComboBox focus workaround test while retaining coverage that popup windows create handles correctly.
  • Removed the comboBoxKeepsPopupOpenWhenFocusMovesToPopupWindow test case that relied on manual focus window changes via QWindowSystemInterface
  • Dropped now-unused QQuickItem, QQuickWindow, and QWindowSystemInterface includes from the test file
tests/ut_dtkdeclatative_qmls.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

@deepin-ci-robot

Copy link
Copy Markdown
Contributor

deepin pr auto review

★ 总体评分:100分

■ 【总体评价】

代码通过移除非原生的焦点干预逻辑彻底修复了ComboBox弹出窗口焦点混乱的BUG,交还Qt原生机制处理,质量极高。
逻辑完全正确,代码质量优秀,无性能与安全问题,属于教科书式的减法重构。

■ 【详细分析】

  • 1.语法逻辑 完全正确✓

diff移除了eventFilter中针对FocusOut的强制拦截、requestPopupFocusrestoreParentFocus函数及其调用链,消除了原代码中由于手动干预QWindowSystemInterface::handleFocusWindowChanged导致的焦点状态不一致与异常关闭隐患,逻辑清理彻底无遗留。
潜在问题:无
建议:无需额外修改

  • 2.代码质量 优秀✓

删除了m_focusOwnerm_restoreFocusItemm_popupFocusRequested等冗余成员变量及相关头文件(如QPointerQDebugqwindowsysteminterface.h),同步清理了失去意义的单元测试comboBoxKeepsPopupOpenWhenFocusMovesToPopupWindow,大幅降低了圈复杂度,符合将复杂行为交由框架底层处理的设计规范。
潜在问题:无
建议:无需额外修改

  • 3.代码性能 高效✓

移除了QMetaObject::invokeMethodQueuedConnection异步调用以及频繁的属性字符串查询(如m_popup->property),减少了不必要的事件循环投递和字符串匹配开销,提升了Popup显隐时的响应效率。
建议:无需额外修改

  • 4.代码安全 存在0个安全漏洞✓

漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个
本次变更为纯删除操作,未引入任何新的输入处理、内存分配或权限变更逻辑,无安全攻击面。
建议:无需额外修改

■ 【改进建议代码示例】

// 以下为清理后的 dpopupwindowhandle_p.h 核心结构示例
// 展示移除冗余焦点管理变量后的简洁状态

class DPopupWindowHandle : public QObject, public QQuickItemChangeListener
{
    Q_OBJECT

private Q_SLOTS:
    void updateEnabled();
    void onWindowChanged(QQuickWindow *window);

private:
    QQuickWindow *popupWindow() const;
    QQuickItem *popupItem() const;
    void popupItemReparented();

    bool isEnabled() const;
    void adjustPopupPosition();
    
private:
    QObject *m_popup = nullptr;
    bool m_enabled = false;
    QPointer<QQuickWindow> m_parentWindow = nullptr;
    QPointer<QQuickWindow> m_popupWin = nullptr;
    QPointer<QQuickItem> m_trackedItem = nullptr;
    // 已安全移除:m_restoreFocusItem, m_focusOwner, m_popupFocusRequested
};

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