fix: remove noise qWarning in setWmWindowTypes when platform window not ready - #405
Conversation
|
Skipping CI for Draft Pull Request. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR refines DWindowManagerHelper::setWmWindowTypes for the Qt6 path so that it silently no-ops when the platform window is not yet created or is non-XCB, removing an unconditional runtime qWarning while preserving behavior on valid X11/XCB windows and keeping the Qt5 path unchanged. Sequence diagram for updated setWmWindowTypes behavior on Qt6sequenceDiagram
participant QMLWindow
participant DWindowManagerHelper
participant QWindow
participant QXcbWindow_P
QMLWindow->>DWindowManagerHelper: setWmWindowTypes(window, types)
DWindowManagerHelper->>DWindowManagerHelper: [if window]
DWindowManagerHelper->>QWindow: handle()
DWindowManagerHelper->>DWindowManagerHelper: dynamic_cast QXcbWindow_P
alt [xcb handle available]
DWindowManagerHelper->>QXcbWindow_P: setWindowType(D_XCB_WINDOW_TYPE)
else [no window or non-XCB/handle nullptr]
DWindowManagerHelper->>DWindowManagerHelper: [silent no-op]
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
08ba021 to
f11b7f5
Compare
d2232d7 to
2db9147
Compare
| if (!platformWindow) | ||
| return; | ||
|
|
||
| // 本次范围限定 X11:xcb QPA 下 handle 非空必为 QXcbWindow(含子类 QXcbForeignWindow), |
There was a problem hiding this comment.
这个else如果变成了死代码,是不是直接去掉就好了,
要是避免警告太多,这里可以把这个warning变成debug的日志输出,方便之后调试,而不是在前面直接都return,
There was a problem hiding this comment.
这里是不确定影响面,所以保留原来的语义。
只处理handle本身为空导致的转换失败的场景,避免警告
There was a problem hiding this comment.
那else也走不到了呀,放开搞,这个接口看着就没适配wayland,可以不用管,之后需要再适配,
There was a problem hiding this comment.
尽量少让代码到中间状态,这样越到后面越难维护了,
要么没有用的删掉,要么给它降个级,不输出,
2db9147 to
fd35a36
Compare
fd35a36 to
3c049f6
Compare
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In the Qt6 branch, consider caching
window->handle()in a local variable before the null check anddynamic_castto avoid calling it twice and to make the intent clearer. - You might want to leave a lightweight debug log (e.g. under a verbose flag) for the non-XCB/non-ready cases to help future debugging while still avoiding noisy runtime warnings.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the Qt6 branch, consider caching `window->handle()` in a local variable before the null check and `dynamic_cast` to avoid calling it twice and to make the intent clearer.
- You might want to leave a lightweight debug log (e.g. under a verbose flag) for the non-XCB/non-ready cases to help future debugging while still avoiding noisy runtime warnings.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
3c049f6 to
c279a63
Compare
…ot ready 1. Remove the unconditional qWarning in setWmWindowTypes Qt6 branch 2. Guard with if (window) and dynamic_cast to QXcbWindow, call handle() once 3. Treat null window, null handle and non-XCB (Wayland) as silent no-op 4. Restore Qt5 silent behavior, matching popupSystemWindowMenu Influence: 1. Verify no warning when opening CreateAccountDialog in dde-control-center 2. Confirm wmWindowTypes still applies on X11 after platform window is created 3. Verify no regression on Wayland/Treeland where the call is a no-op fix: 移除 setWmWindowTypes 平台窗口未就绪时的噪声告警 1. 删除 setWmWindowTypes Qt6 分支中无条件的 qWarning 2. 外层判 window 非空,内层 dynamic_cast 到 QXcbWindow,handle() 只调一次 3. window 为空、handle 为空或非 XCB 平台(Wayland)时静默 no-op 4. 恢复 Qt5 静默行为,与 popupSystemWindowMenu、setMotifFunctions 等一致 Influence: 1. 验证控制中心账户模块打开新建用户对话框时不再打印该警告 2. 确认 X11 下平台窗口创建后 wmWindowTypes 仍能正常生效 3. 验证 Wayland/Treeland 下该调用为 no-op,无回归
c279a63 to
ae63a02
Compare
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider returning early when
windowis null to avoid an extra nestedifand keep the control flow consistent with other helpers in this file. - The new comment block is in Chinese while the surrounding code is in English; consider standardizing the comment language to match the rest of the file for consistency.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider returning early when `window` is null to avoid an extra nested `if` and keep the control flow consistent with other helpers in this file.
- The new comment block is in Chinese while the surrounding code is in English; consider standardizing the comment language to match the rest of the file for consistency.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
deepin pr auto review★ 总体评分:100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // 当前代码已为最佳实践,无需额外修改,此处展示其完整上下文以供参考
#if QT_VERSION >= QT_VERSION_CHECK(6, 0, 0)
typedef QNativeInterface::Private::QXcbWindow QXcbWindow_P;
// _NET_WM_WINDOW_TYPE 是 X11 专属属性,仅在 XCB 平台窗口已创建时生效。
// 当 window 为空、handle 为空(平台窗口尚未创建,常见于 QML 组件完成阶段)
// 或为非 XCB 平台(如 Wayland/Treeland)时无法应用,静默忽略即可,
// 与 Qt5 下 QXcbWindowFunctions::setWmWindowType 及本类 setMotifFunctions/
// setMotifDecorations/popupSystemWindowMenu 的处理方式一致,避免对 QML 窗口产生噪声告警。
if (window) {
if (auto w = dynamic_cast<QXcbWindow_P *>(window->handle())) {
w->setWindowType(static_cast<D_XCB_WINDOW_TYPE>(_types));
}
}
#else
QXcbWindowFunctions::setWmWindowType(window, static_cast<D_XCB_WINDOW_TYPE>(_types));
#endif |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, 52cyb 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 |
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Summary
Remove the noisy runtime warning emitted by
DWindowManagerHelper::setWmWindowTypeswhen the platform window is not ready or not on XCB.When adding a new user in dde-control-center (accounts module), the following warning was logged:
Root cause
setWmWindowTypesis invoked during the QML component-complete phase (via theD.DWindow.wmWindowTypesbinding in dtkdeclarative), at which pointwindow->handle()is stillnullptr. The Qt6 code unconditionallyqWarnings wheneverdynamic_cast<QXcbWindow_P*>(window->handle())fails. That cast fails in two normal situations that are not errors:window->handle()isnullptr(common during the QML component-complete phase whereD.DWindow.wmWindowTypesis evaluated before the underlyingQWindowis exposed).QWaylandWindow, not anQXcbWindow._NET_WM_WINDOW_TYPEis an X11-only property, so in both cases there is legitimately nothing to do. The Qt5 path (QXcbWindowFunctions::setWmWindowType) was already a silent no-op here; the Qt6 port introduced the unconditionalqWarningand turned a normal no-op into noise.Change
File:
src/kernel/dwindowmanagerhelper.cpp—DWindowManagerHelper::setWmWindowTypes, Qt6 branch only.qWarning(the wholeelsebranch).if (window)(null-pointer safety), thendynamic_cast<QXcbWindow_P*>(window->handle())—window->handle()is called only once; a null handle safely yieldsnullptrfrom the cast (silent no-op).popupSystemWindowMenu,setMotifFunctionsandsetMotifDecorationsin the same file, and restoring the Qt5 silent behavior.#elsebranch and the#ifdef Q_OS_LINUXguard are unchanged. No header / API / ABI change.Testing
tests/src/ut_dwindowmanagerhelper.cpponly expects "no crash" for this function; behavior is preserved.wmWindowTypesstill applies on X11 once the platform window is created.Related Multica issue: DDE-111
Summary by Sourcery
Bug Fixes: