fix(popup): focus window popups for input#638
Open
mhduiy wants to merge 1 commit into
Open
Conversation
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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Reviewer's GuideEnsures window-mode popups request focus and activation on show so that embedded input controls can receive focus and trigger the input method, by wiring popup focus properties in QML to a new focus-request helper in the popup window handle. Sequence diagram for popup window focus request on showsequenceDiagram
participant PopupWin as QQuickWindow_m_popupWin
participant Handle as DPopupWindowHandle
participant Popup as QObject_m_popup
participant Item as QQuickItem_popupItem
PopupWin->>Handle: eventFilter(watched=PopupWin, event: QEvent::Show)
alt watched is m_popupWin and event is Show
Handle->>Handle: requestPopupFocus()
Handle->>Handle: QMetaObject::invokeMethod(this, lambda, QueuedConnection)
Note over Handle: Queued lambda executes later
Handle->>Popup: property focus
alt isEnabled and popup focus and popupWin visible
Handle->>PopupWin: requestActivate()
Handle->>Item: forceActiveFocus(Qt::ActiveWindowFocusReason)
end
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
requestPopupFocus(), instead of capturingpopupItem()in aQPointer<QQuickItem>before the queued invocation, consider resolving the current popup item inside the lambda to avoid focusing a stale or deleted item if the content changes between theShowevent and the queued execution. - The check
popup->property("focus").toBool()is performed both before and inside the queued lambda; you could drop the outer check and rely on the inner one to reduce duplicated property lookups and keep the decision logic in a single place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `requestPopupFocus()`, instead of capturing `popupItem()` in a `QPointer<QQuickItem>` before the queued invocation, consider resolving the current popup item inside the lambda to avoid focusing a stale or deleted item if the content changes between the `Show` event and the queued execution.
- The check `popup->property("focus").toBool()` is performed both before and inside the queued lambda; you could drop the outer check and rely on the inner one to reduce duplicated property lookups and keep the decision logic in a single place.
## Individual Comments
### Comment 1
<location path="src/private/dpopupwindowhandle.cpp" line_range="287-306" />
<code_context>
m_popupWin->setPosition(newPos);
}
+void DPopupWindowHandle::requestPopupFocus()
+{
+ if (!m_popup || !isEnabled() || !m_popupWin || !m_popupWin->isVisible())
+ return;
+ if (!m_popup->property("focus").toBool())
+ return;
+
+ QPointer<QObject> popup = m_popup;
+ QPointer<QQuickWindow> popupWin = m_popupWin;
+ QPointer<QQuickItem> item = popupItem();
+
+ QMetaObject::invokeMethod(this, [this, popup, popupWin, item] {
+ if (!isEnabled() || !popup || !popup->property("focus").toBool() || !popupWin || popupWin != m_popupWin || !popupWin->isVisible())
+ return;
+
+ popupWin->requestActivate();
+ if (item && item->window() == popupWin)
+ item->forceActiveFocus(Qt::ActiveWindowFocusReason);
+ }, Qt::QueuedConnection);
+}
+
DQUICK_END_NAMESPACE
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid capturing a potentially stale popup item and consider resolving it at invocation time.
Because `item` is captured before the queued call executes, it may no longer match the actual popup item when the lambda runs (e.g. due to reparenting or delayed creation changing `popupItem()`). Instead, construct the `QPointer<QQuickItem>` inside the lambda by calling `popupItem()` there, then validate it (and its window) before requesting focus.
```suggestion
void DPopupWindowHandle::requestPopupFocus()
{
if (!m_popup || !isEnabled() || !m_popupWin || !m_popupWin->isVisible())
return;
if (!m_popup->property("focus").toBool())
return;
QPointer<QObject> popup = m_popup;
QPointer<QQuickWindow> popupWin = m_popupWin;
QMetaObject::invokeMethod(this, [this, popup, popupWin] {
if (!isEnabled() || !popup || !popup->property("focus").toBool() || !popupWin || popupWin != m_popupWin || !popupWin->isVisible())
return;
popupWin->requestActivate();
QPointer<QQuickItem> item = popupItem();
if (item && item->window() == popupWin)
item->forceActiveFocus(Qt::ActiveWindowFocusReason);
}, Qt::QueuedConnection);
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1. Enable focus for popups using window mode. 2. Request activation when the popup window is shown. 3. Ensure input controls inside popup windows can trigger the input method. Log: Focus window popups on show so embedded input controls can open the input method. fix(popup): 让窗口弹窗支持输入法 1. 为窗口模式的弹窗启用焦点。 2. 在弹窗窗口显示时请求激活。 3. 确保窗口弹窗内的输入控件可以调起输入法。 Log: 显示窗口弹窗时获取焦点，使内部输入控件可以调起输入法。 pms: BUG-366067
Contributor
deepin pr auto review★ 总体评分：95分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 简要注释：在 Popup 对应的窗口 Show 事件时触发焦点请求逻辑，确保弹窗显示时能自动获取焦点
void DPopupWindowHandle::requestPopupFocus()
{
if (!m_popup || !isEnabled() || !m_popupWin || !m_popupWin->isVisible())
return;
if (!m_popup->property("focus").toBool())
return;
QPointer<QObject> popup = m_popup;
QPointer<QQuickWindow> popupWin = m_popupWin;
// 使用异步调用防止在 Show 事件链中直接操作引发异常
QMetaObject::invokeMethod(this, [this, popup, popupWin] {
// 二次校验确保对象生命周期安全及状态一致
if (!isEnabled() || !popup || !popup->property("focus").toBool() || !popupWin || popupWin != m_popupWin || !popupWin->isVisible())
return;
QPointer<QQuickItem> item = popupItem();
if (!item || item->window() != popupWin)
return;
popupWin->requestActivate();
item->forceActiveFocus(Qt::ActiveWindowFocusReason);
}, Qt::QueuedConnection);
} |
18202781743
reviewed
Jun 16, 2026
| return; | ||
|
|
||
| popupWin->requestActivate(); | ||
| item->forceActiveFocus(Qt::ActiveWindowFocusReason); |
Contributor
There was a problem hiding this comment.
| QPointer<QQuickWindow> popupWin = m_popupWin; | ||
|
|
||
| QMetaObject::invokeMethod(this, [this, popup, popupWin] { | ||
| if (!isEnabled() || !popup || !popup->property("focus").toBool() || !popupWin || popupWin != m_popupWin || !popupWin->isVisible()) |
Contributor
There was a problem hiding this comment.
这个在this里有成员变量，是QPointer的，还需要在外面传入么，
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Log: Focus window popups on show so embedded input controls can open the input method.
fix(popup): 让窗口弹窗支持输入法
Log: 显示窗口弹窗时获取焦点，使内部输入控件可以调起输入法。
Summary by Sourcery
Ensure window-based popups gain focus and activation when shown so embedded input controls can properly receive input and trigger the input method.
Bug Fixes:
Enhancements: