fix: handle touch outside popup on X11#1604
Merged
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideExtends the X11 MouseGrabEventFilter to handle touch events (QTouchEvent::TouchBegin) so that touches outside a popup window close it, matching existing mouse outside-click behavior, while updating the file copyright header years. Sequence diagram for touch-based popup closing on X11sequenceDiagram
actor User
participant QApplication
participant MouseGrabEventFilter
participant PopupWindow
User ->> QApplication: QTouchEvent TouchBegin
QApplication ->> MouseGrabEventFilter: eventFilter(watched, event)
alt event.type is QEvent_TouchBegin
MouseGrabEventFilter ->> MouseGrabEventFilter: touchEvent.points().first()
MouseGrabEventFilter ->> MouseGrabEventFilter: m_target.geometry().contains(pos.toPoint())
alt [touch outside popup geometry]
MouseGrabEventFilter ->> PopupWindow: outsideMousePressed
MouseGrabEventFilter -->> QApplication: return true
else [touch inside popup geometry]
MouseGrabEventFilter -->> QApplication: return false
end
else other event types
MouseGrabEventFilter -->> QApplication: return false
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:
- The touch handling only considers QEvent::TouchBegin and the first touch point; if multi-touch or subsequent TouchUpdate events can occur outside the popup, consider whether they should also trigger the outside-close behavior to avoid edge cases.
- The new touch branch assumes m_target is valid and uses geometry() directly; if there are code paths where the filter might outlive or be detached from m_target, add a defensive null/visibility check similar to whatever is used for the existing mouse logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The touch handling only considers QEvent::TouchBegin and the first touch point; if multi-touch or subsequent TouchUpdate events can occur outside the popup, consider whether they should also trigger the outside-close behavior to avoid edge cases.
- The new touch branch assumes m_target is valid and uses geometry() directly; if there are code paths where the filter might outlive or be detached from m_target, add a defensive null/visibility check similar to whatever is used for the existing mouse logic.
## Individual Comments
### Comment 1
<location path="frame/utility_x11.cpp" line_range="128-129" />
<code_context>
+ QTouchEvent *touchEvent = static_cast<QTouchEvent *>(event);
+ if (!touchEvent->points().isEmpty()) {
+ QEventPoint pt = touchEvent->points().first();
+ const auto bounding = m_target->geometry();
+ const auto pos = pt.globalPosition();
+ if (!bounding.contains(pos.toPoint())) {
+ qCDebug(dsLog) << "touch outside menu window:" << m_target->winId();
</code_context>
<issue_to_address>
**issue (bug_risk):** Mixing widget-local geometry with global coordinates may mis-detect outside touches
`m_target->geometry()` is in the parent widget’s coordinate system, but `pt.globalPosition()` is in global screen coordinates. Using them together in `bounding.contains()` can misclassify touches when the window moves or is embedded. Either compare in global coordinates (e.g. `m_target->frameGeometry()`) or first map the global point into the target’s coordinates with `m_target->mapFromGlobal(pos.toPoint())` before calling `rect().contains(...)`.
</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. Add QTouchEvent handling to MouseGrabEventFilter in frame/ utility_x11.cpp. 2. Detect QEvent::TouchBegin and read the first touch point's global position. 3. When the touch happens outside the target popup window geometry, emit outsideMousePressed() and consume the event. 4. Keep the existing mouse-based outside-click logic unchanged while extending the same close-popup behavior to touch input. 5. This fixes an issue on touch-screen devices where, after the plugin area context menu was shown, tapping other areas of the taskbar did not respond because touch interactions outside the popup were not being handled. Log: Fixed taskbar touch no-response issue when plugin area context menu is open Influence: 1. On an X11 touch-screen device, open the plugin area context menu and tap outside the popup on different taskbar areas; verify the popup closes and the tapped area responds correctly. 2. Verify existing mouse behavior is unchanged: right-click to open the menu, then click outside to close it. 3. Test tapping inside the popup to confirm normal menu interaction is preserved. 4. Test repeated open/close operations with both mouse and touch input to ensure there are no stuck grab states or lost events. 5. Verify no regressions for non-touch environments on X11. fix: 处理 X11 下弹窗外部触控关闭逻辑 1. 在 frame/utility_x11.cpp 的 MouseGrabEventFilter 中新增 QTouchEvent 处理逻辑。 2. 检测 QEvent::TouchBegin，并读取第一个触控点的全局坐标。 3. 当触控位置位于目标弹窗窗口几何范围之外时，触发 outsideMousePressed() 并消费该事件。 4. 在保留原有鼠标外部点击处理逻辑不变的基础上，将相同的关闭弹窗行为扩展 到触控输入。 5. 该修改修复了触控屏设备上插件区域右键菜单弹出后，点击任务栏其他区域无 响应的问题，原因是此前未处理弹窗外部的触控事件。 Log: 修复插件区域右键菜单打开时任务栏触控无响应问题 Influence: 1. 在 X11 触控屏设备上打开插件区域右键菜单后，点击弹窗外的不同任务栏区 域，确认弹窗会关闭且被点击区域能够正常响应。 2. 验证现有鼠标行为不受影响：右键打开菜单后，鼠标点击外部仍可正常关闭。 3. 测试在弹窗内部点击，确认菜单本身的交互仍然正常。 4. 使用鼠标和触控反复执行打开/关闭操作，确认不存在 grab 状态残留或事件丢 失问题。 5. 验证 X11 非触控环境下无回归。 PMS: BUG-362159 Change-Id: I3c6ed96f01408c3c966f17ae7b999858c0c9ec46
deepin pr auto review你好！我是CodeGeeX，你的智能编程助手。我已经仔细审查了你提供的 Git Diff。 本次修改主要实现了两个目标：一是增加了对触摸事件（ 整体修改逻辑清晰，但在代码安全（健壮性）、逻辑一致性和代码性能方面，我提出以下改进意见： 1. 代码安全与健壮性问题： 在新增的 QTouchEvent *touchEvent = static_cast<QTouchEvent *>(event);
if (!touchEvent->points().isEmpty()) { ... }风险： 虽然 Qt 的事件系统通常会保证事件类型匹配，但 改进建议： 使用 } else if (event->type() == QEvent::TouchBegin) {
QTouchEvent *touchEvent = dynamic_cast<QTouchEvent *>(event);
if (touchEvent && m_target && !touchEvent->points().isEmpty()) {
// ... 逻辑处理
}
}2. 逻辑一致性问题： 在原有的 风险： 如果在 X11 平台下，触摸屏驱动或 Qt 的事件转换同样存在坐标异常（例如 改进建议： 建议将触摸事件的判断逻辑与鼠标事件对齐，保持平台行为的一致性： const auto pos = pt.globalPosition();
if ((pt.position().toPoint().isNull() && !pos.isNull()) ||
!bounding.contains(pos.toPoint())) {
qCDebug(dsLog) << "touch outside menu window:" << m_target->winId();
emit outsideMousePressed();
return true;
}3. 代码性能问题： 在 for (auto item : Utility::instance()->allChildrenWindows(m_target)) {风险： 改进建议： 使用 for (auto *item : Utility::instance()->allChildrenWindows(m_target)) {4. 代码质量与可维护性问题： 修改后， const auto bounding = m_target->frameGeometry();
const auto pos = ...;
if ((... .isNull() && !pos.isNull()) || !bounding.contains(pos.toPoint())) { ... }风险： 违反了 DRY (Don't Repeat Yourself) 原则。如果未来坐标判定逻辑需要调整（例如改用浮点数判定以支持高分辨率），需要同时修改多处，容易遗漏。 改进建议： 可以考虑提取一个私有方法，例如 bool MouseGrabEventFilter::isOutsideTarget(const QPointF &localPos, const QPointF &globalPos) const {
if (!m_target) return true;
const auto bounding = m_target->frameGeometry();
if ((localPos.toPoint().isNull() && !globalPos.isNull()) ||
!bounding.contains(globalPos.toPoint())) {
return true;
}
return false;
}然后在各处直接调用 综合改进后的代码示例结合以上意见，改进后的核心代码部分如下： // 在头文件中建议添加私有方法声明
// bool isOutsideTarget(const QPointF &localPos, const QPointF &globalPos) const;
bool MouseGrabEventFilter::eventFilter(QObject *watched, QEvent *event)
{
if (event->type() == QEvent::MouseButtonPress) {
if (isMainWindow()) {
trySelectGrabWindow(static_cast<QMouseEvent *>(event));
}
} else if (event->type() == QEvent::TouchBegin) {
// 增加类型安全检查与空指针保护
QTouchEvent *touchEvent = dynamic_cast<QTouchEvent *>(event);
if (touchEvent && m_target && !touchEvent->points().isEmpty()) {
QEventPoint pt = touchEvent->points().first();
// 逻辑与鼠标事件保持一致
if (isOutsideTarget(pt.position(), pt.globalPosition())) {
qCDebug(dsLog) << "touch outside menu window:" << m_target->winId();
emit outsideMousePressed();
return true;
}
}
}
return false;
}
void MouseGrabEventFilter::mousePressEvent(QMouseEvent *e)
{
if (isOutsideTarget(e->position(), e->globalPosition())) {
emit outsideMousePressed();
}
}
bool MouseGrabEventFilter::trySelectGrabWindow(QMouseEvent *e)
{
if (isOutsideTarget(e->position(), e->globalPosition())) {
for (auto *item : Utility::instance()->allChildrenWindows(m_target)) { // 使用 auto* 避免拷贝
if (!item->isVisible() || isMainWindow(item))
continue;
// Use frameGeometry() for child windows too
if (item->frameGeometry().contains(e->globalPosition().toPoint())) {
qCDebug(dsLog) << "grab mouse for the window:" << item->winId();
m_target->setMouseGrabEnabled(false);
item->setMouseGrabEnabled(true);
return true; // 假设原代码此处有 return 逻辑，或者放在循环外
}
}
}
return false; // 假设原代码有默认返回值
}
// 新增的辅助函数
bool MouseGrabEventFilter::isOutsideTarget(const QPointF &localPos, const QPointF &globalPos) const
{
if (!m_target) return true;
const auto bounding = m_target->frameGeometry();
return (localPos.toPoint().isNull() && !globalPos.isNull()) ||
!bounding.contains(globalPos.toPoint());
}希望这些建议能帮助你进一步提升代码质量！如果有任何疑问，欢迎继续交流。 |
18202781743
approved these changes
May 21, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, fly602 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 |
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: Fixed taskbar touch no-response issue when plugin area context menu is open
Influence:
fix: 处理 X11 下弹窗外部触控关闭逻辑
Log: 修复插件区域右键菜单打开时任务栏触控无响应问题
Influence:
PMS: BUG-362159
Change-Id: I3c6ed96f01408c3c966f17ae7b999858c0c9ec46
Summary by Sourcery
Handle touch interactions outside popup windows on X11 so that popups close consistently for both mouse and touch input.
Bug Fixes:
Enhancements: