Skip to content

fix: guard PanelPopup close behavior against transient X11 grab focus…#1542

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Ivy233:fix-panelpopup-x11-grab-focus-guard
Apr 8, 2026
Merged

fix: guard PanelPopup close behavior against transient X11 grab focus…#1542
deepin-bot[bot] merged 1 commit intolinuxdeepin:masterfrom
Ivy233:fix-panelpopup-x11-grab-focus-guard

Conversation

@Ivy233
Copy link
Copy Markdown
Contributor

@Ivy233 Ivy233 commented Apr 1, 2026

… changes

修复:防止PanelPopup在X11键盘抓取期间因瞬时失焦被误关闭

  1. Detect X11 FocusOut(GRAB)/FocusIn(UNGRAB) in PopupWindow and expose grab-transition signals.

  2. Keep close-on-inactive behavior, but defer close during grab transition and cancel when ungrab focus returns.

  3. Handle active-state transitions only for the current popup owner instance to avoid cross-instance interference.

  4. 在PopupWindow中识别X11的FocusOut(GRAB)/FocusIn(UNGRAB),并向QML暴露抓取过渡信号。

  5. 保留失焦关闭语义,但在抓取过渡期间延迟关闭,并在Ungrab后焦点恢复时取消关闭。

  6. 仅由当前popup owner实例处理active状态变化,避免共享popupWindow下多个实例互相干扰。

PMS: BUG-301109

Summary by Sourcery

Guard panel popup dismissal against transient X11 focus changes caused by keyboard/mouse grabs so popups stay open during grab transitions.

Bug Fixes:

  • Prevent panel popups from being spuriously closed when X11 focus briefly leaves the window due to GRAB/UNGRAB transitions.

Enhancements:

  • Expose X11 grab focus transition state and related grab/ungrab signals from PopupWindow to QML for more precise focus handling in panel popups.
  • Ensure active-state handling and close-on-inactive behavior are scoped to the currently owning popup instance to avoid cross-instance interference.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai bot commented Apr 1, 2026

Reviewer's Guide

Introduces X11 focus-grab aware handling in PopupWindow/PanelPopup so that transient X11 grab/ungrab focus changes do not prematurely close the panel popup, using native XCB focus events, a grab-transition state, and a short QML timer to defer and potentially cancel close-on-inactive behavior per-popup instance.

Sequence diagram for X11 grab/ungrab focus handling in PanelPopup

sequenceDiagram
    participant X11
    participant PopupWindow
    participant PanelPopup
    participant grabInactiveTimer

    X11->>PopupWindow: xcb_focus_out_event (mode GRAB)
    PopupWindow->>PopupWindow: setX11GrabFocusTransition(true)
    PopupWindow-->>PanelPopup: x11FocusOutByGrab()
    PanelPopup->>PanelPopup: grabInactivePending = true
    PanelPopup->>grabInactiveTimer: start(grabInactiveTimeout)

    X11->>PopupWindow: xcb_focus_in_event (mode UNGRAB)
    PopupWindow-->>PanelPopup: x11FocusInByUngrab()
    alt grabInactivePending is true
        PanelPopup->>PanelPopup: grabInactivePending = false
        PanelPopup->>grabInactiveTimer: stop()
        PanelPopup->>PanelPopup: Qt.callLater(check active)
        alt popupWindow.active is false
            PanelPopup->>PanelPopup: close()
        else popupWindow.active is true
            PanelPopup-->>PanelPopup: keep open
        end
    end
    PopupWindow->>PopupWindow: setX11GrabFocusTransition(false)


    %% Normal focus loss without grab
    X11->>PopupWindow: xcb_focus_out_event (non GRAB)
    PopupWindow-->>PanelPopup: active false (property change)
    PanelPopup->>PanelPopup: onActiveChanged
    alt not x11GrabFocusTransition and not grabInactivePending and not active
        PanelPopup->>PanelPopup: close()
    end
Loading

Updated class diagram for PopupWindow and PanelPopup focus-grab handling

classDiagram
    class QQuickApplicationWindow

    class PopupWindow {
        QWindow transientParent
        bool x11GrabFocusTransition
        bool m_dragging
        bool m_pressing
        bool m_x11GrabFocusTransition
        PopupWindow(QWindow *parent)
        void mouseReleaseEvent(QMouseEvent *event)
        void mousePressEvent(QMouseEvent *event)
        void mouseMoveEvent(QMouseEvent *event)
        bool nativeEvent(const QByteArray &eventType, void *message, long *result)
        bool nativeEvent(const QByteArray &eventType, void *message, qintptr *result)
        bool x11GrabFocusTransition()
        void setX11GrabFocusTransition(bool transition)
        x11GrabFocusTransitionChanged()
        x11FocusOutByGrab()
        x11FocusInByUngrab()
    }

    class PanelPopup {
        int popupX
        int popupY
        bool readyBinding
        bool grabInactivePending
        int grabInactiveTimeout
        string windowTitle
        function close()
        function onActiveChanged()
        function onX11FocusOutByGrab()
        function onX11FocusInByUngrab()
        Timer grabInactiveTimer
    }

    PopupWindow <|-- QQuickApplicationWindow
    PanelPopup ..> PopupWindow : uses
    PanelPopup ..> Timer : owns
Loading

File-Level Changes

Change Details Files
Make PanelPopup close-on-inactive logic aware of X11 grab/ungrab transitions and per-owner instance state, deferring closure via a short timer.
  • Add grabInactivePending and grabInactiveTimeout properties plus a one-shot Timer to manage deferred closing after potential grab transitions.
  • Ensure close() clears grabInactive state and stops the timer to avoid stale deferred closures.
  • In activeChanged handling, ignore events for non-current items or hidden popups, and skip immediate closing while a grab transition or pending grabInactive state is in effect.
  • On X11 focus-out-by-grab, set grabInactivePending and start the timer to wait for possible ungrab focus return.
  • On X11 focus-in-by-ungrab, cancel pending inactivity, then re-evaluate popupWindow.active later via Qt.callLater and close only if still inactive.
frame/qml/PanelPopup.qml
Detect X11 focus grab/ungrab transitions at the native event level and expose them and a grab-transition property to QML.
  • Include xcb headers on Linux and override nativeEvent to inspect XCB_FOCUS_IN/OUT events targeting this window.
  • Set an internal m_x11GrabFocusTransition flag and emit x11FocusOutByGrab/x11FocusInByUngrab signals when seeing GRAB/UNGRAB focus events, resetting the flag on ungrab or non-grab focus in.
  • Expose x11GrabFocusTransition as a Q_PROPERTY with notifier to QML and reset it when the window becomes invisible.
  • Add private setter for the transition flag and store it alongside existing popup state fields.
frame/popupwindow.cpp
frame/popupwindow.h

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
Copy Markdown

@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 - I've found 1 issue, and left some high level feedback:

  • In PopupWindow::nativeEvent, the parameters are unused on non-Linux builds due to the #ifdef Q_OS_LINUX block; consider adding Q_UNUSED(eventType), Q_UNUSED(message), and Q_UNUSED(result) in the #else path to avoid compiler warnings.
  • When handling XCB focus events in nativeEvent, you always cast to xcb_focus_in_event_t even for XCB_FOCUS_OUT; using the matching xcb_focus_out_event_t (or a shared helper) for the OUT case would make the code clearer and avoid potential confusion.
  • The 200 ms grabInactiveTimeout is a magic number in QML; consider turning it into a named constant or providing a brief comment explaining why this specific duration is chosen.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `PopupWindow::nativeEvent`, the parameters are unused on non-Linux builds due to the `#ifdef Q_OS_LINUX` block; consider adding `Q_UNUSED(eventType)`, `Q_UNUSED(message)`, and `Q_UNUSED(result)` in the `#else` path to avoid compiler warnings.
- When handling XCB focus events in `nativeEvent`, you always cast to `xcb_focus_in_event_t` even for `XCB_FOCUS_OUT`; using the matching `xcb_focus_out_event_t` (or a shared helper) for the OUT case would make the code clearer and avoid potential confusion.
- The 200 ms `grabInactiveTimeout` is a magic number in QML; consider turning it into a named constant or providing a brief comment explaining why this specific duration is chosen.

## Individual Comments

### Comment 1
<location path="frame/qml/PanelPopup.qml" line_range="160-161" />
<code_context>
+            control.grabInactivePending = false
+            grabInactiveTimer.stop()
+
+            Qt.callLater(function() {
+                if (!popupWindow || !readyBinding || !popup.visible) {
+                    return
+                }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Re-check currentItem / grabInactivePending in the deferred close logic to avoid closing a different popup.

Because there’s a delay between scheduling `Qt.callLater` and its execution, `popupWindow.currentItem` and `grabInactivePending` may change (e.g. popup switched or another focus event fired). That can cause this handler to close a popup no longer associated with `control`. Inside the deferred function, also check that `popupWindow.currentItem === control` and, if relevant, `control.grabInactivePending` before calling `control.close()`.

Suggested implementation:

```
        Qt.callLater(function() {
            // Re-validate association to avoid closing a popup that no longer belongs to this control.
            if (!popupWindow
                    || !readyBinding
                    || popupWindow.currentItem !== control
                    || !popup.visible
                    || control.grabInactivePending) {
                return
            }

```

If the inner `Qt.callLater` block is not located under `grabInactiveTimer.onTriggered` but instead under `onX11FocusInByUngrab`, apply the same replacement there: re-check `popupWindow.currentItem !== control` and `control.grabInactivePending` in the deferred handler before calling `control.close()`. Also ensure that the identifiers (`popupWindow`, `control`, `readyBinding`, `popup`) match the actual names in your file.
</issue_to_address>

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.

@Ivy233 Ivy233 force-pushed the fix-panelpopup-x11-grab-focus-guard branch from 22a7456 to 9970272 Compare April 1, 2026 08:13
@Ivy233 Ivy233 removed the request for review from yixinshark April 1, 2026 08:19
Copy link
Copy Markdown
Contributor

@robertkill robertkill left a comment

Choose a reason for hiding this comment

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

LGTM ✅

这是一个正确的bug修复,解决了PanelPopup在X11 GRAB/UNGRAB期间意外关闭的问题。

核心评价

  • ✅ X11焦点事件处理逻辑正确
  • ✅ 延迟关闭机制设计合理(200ms Timer)
  • ✅ 避免跨instance干扰(currentItem检查)
  • ✅ 所有CI测试通过

建议的小改进(不阻塞合并)(参考sourcery-ai的review):

  1. 非Linux构建时添加Q_UNUSED宏避免编译警告
  2. 考虑为200ms添加注释说明或提取为配置项
  3. 可考虑在Qt.callLater中重新检查currentItem,防止竞态

这些可以在后续PR中优化,当前实现已经足够稳定,建议合并。

@Ivy233 Ivy233 requested a review from robertkill April 1, 2026 08:34
Copy link
Copy Markdown
Contributor

@robertkill robertkill left a comment

Choose a reason for hiding this comment

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

误操作

@Ivy233 Ivy233 force-pushed the fix-panelpopup-x11-grab-focus-guard branch from 9970272 to 73aa543 Compare April 1, 2026 08:44
@Ivy233 Ivy233 requested review from BLumia and robertkill April 1, 2026 08:45
@deepin-bot
Copy link
Copy Markdown

deepin-bot bot commented Apr 7, 2026

TAG Bot

New tag: 2.0.35
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1545

@deepin-bot
Copy link
Copy Markdown

deepin-bot bot commented Apr 7, 2026

TAG Bot

New tag: 2.0.36
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #1547

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, Ivy233, robertkill

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

@Ivy233 Ivy233 force-pushed the fix-panelpopup-x11-grab-focus-guard branch from 73aa543 to 5b0bfb3 Compare April 8, 2026 02:01
… changes

修复:防止PanelPopup在X11键盘抓取期间因瞬时失焦被误关闭

1. Detect X11 FocusOut(GRAB)/FocusIn(UNGRAB) in PopupWindow and expose grab-transition signals.
2. Keep close-on-inactive behavior, but defer close during grab transition and cancel when ungrab focus returns.
3. Handle active-state transitions only for the current popup owner instance to avoid cross-instance interference.

1. 在PopupWindow中识别X11的FocusOut(GRAB)/FocusIn(UNGRAB),并向QML暴露抓取过渡信号。
2. 保留失焦关闭语义,但在抓取过渡期间延迟关闭,并在Ungrab后焦点恢复时取消关闭。
3. 仅由当前popup owner实例处理active状态变化,避免共享popupWindow下多个实例互相干扰。

PMS: BUG-301109
@Ivy233 Ivy233 force-pushed the fix-panelpopup-x11-grab-focus-guard branch from 5b0bfb3 to b5ab25a Compare April 8, 2026 02:12
@Ivy233
Copy link
Copy Markdown
Contributor Author

Ivy233 commented Apr 8, 2026

/merge

@deepin-bot
Copy link
Copy Markdown

deepin-bot bot commented Apr 8, 2026

This pr cannot be merged! (status: unstable)

@Ivy233
Copy link
Copy Markdown
Contributor Author

Ivy233 commented Apr 8, 2026

/forcemerge

@deepin-bot
Copy link
Copy Markdown

deepin-bot bot commented Apr 8, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 94d1d83 into linuxdeepin:master Apr 8, 2026
11 of 12 checks passed
@Ivy233 Ivy233 deleted the fix-panelpopup-x11-grab-focus-guard branch April 8, 2026 02:42
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.

4 participants