Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deepin-wm 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 GuideUpdates WXWayland to proactively mirror the active seat keyboard’s modifier state to XWayland whenever another Wayland client has focus, while handling keyboard replacement, destruction, nullable seats, and XWayland startup ordering. Sequence diagram for syncing keyboard modifiers to XWaylandsequenceDiagram
participant Keyboard as Seat keyboard
participant WXWayland as WXWayland
participant XWayland as XWayland server
participant Client as Focused Wayland client
Keyboard->>WXWayland: modifiers event
WXWayland->>WXWayland: syncModifiersToXWayland()
alt XWayland does not hold keyboard focus
WXWayland->>XWayland: wl_keyboard_send_modifiers(depressed, latched, locked, group)
XWayland->>XWayland: Update XKB modifier state
else XWayland holds keyboard focus
WXWayland->>WXWayland: Skip explicit synchronization
end
State diagram for WXWayland keyboard trackingstateDiagram-v2
[*] --> NoKeyboard
NoKeyboard --> WatchingKeyboard: setSeat(seat) / watchSeatKeyboard()
WatchingKeyboard --> WatchingKeyboard: keyboardChanged / reattach listeners and syncModifiersToXWayland()
WatchingKeyboard --> WatchingKeyboard: modifiers / syncModifiersToXWayland()
WatchingKeyboard --> NoKeyboard: keyboard destroy
NoKeyboard --> WatchingKeyboard: replacement keyboard available
WatchingKeyboard --> WaitingForXWayland: setSeat before XWayland ready
WaitingForXWayland --> WatchingKeyboard: ready / syncModifiersToXWayland()
File-Level Changes
Assessment against linked issues
Possibly linked issues
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
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="waylib/src/server/protocols/wxwayland.cpp" line_range="489-498" />
<code_context>
+
+ // The seat keyboard can be replaced (e.g. by the input method's virtual
+ // keyboard) or destroyed at any time, so re-attach on changes.
+ if (!seatKeyboardChangedConnection) {
+ if (auto *seatObject = WSeat::fromHandle(seat)) {
+ seatKeyboardChangedConnection = QObject::connect(seatObject,
+ &WSeat::keyboardChanged,
+ q, [this] {
+ watchSeatKeyboard();
+ syncModifiersToXWayland();
+ });
+ }
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The seat keyboard-change connection is created only when `seatKeyboardChangedConnection` is empty and is never disconnected or replaced when `setSeat()` switches from one seat to another. After the first seat has been watched, keyboard replacement on the new seat (including virtual-keyboard switching) is not observed, so the XWayland modifier listener is not reattached and its state becomes stale.
**Triggers:** When `setSeat()` is called with a different non-null seat after a seat has already been configured.
**Suggested fix:** Disconnect the existing connection whenever the seat changes, then connect `keyboardChanged` on the new seat and store the connection associated with that seat.
</issue_to_address>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Three moderate findings remain unresolved, along with an issue-reference discrepancy.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Synchronizes compositor keyboard modifier state to XWayland when it lacks keyboard focus.
Changes:
- Sends modifier updates to XWayland.
- Handles keyboard replacement, destruction, seat reassignment, and startup ordering.
- Allows clearing the associated seat.
| File | Review summary |
|---|---|
waylib/src/server/protocols/wxwayland.cpp |
Adds modifier synchronization and lifecycle handling. Outstanding findings: three moderate issues (votes: 3, 1, 1) involving stale seat/keyboard listeners and inert resources, plus one nit (vote: 1) regarding the unrelated Fixes #501 reference. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+513
to
+519
| W_D(WXWayland); | ||
|
|
||
| if (auto handle = this->handle()) | ||
| wlr_xwayland_set_seat(handle, seat->handle()); | ||
| wlr_xwayland_set_seat(handle, seat ? seat->handle() : nullptr); | ||
|
|
||
| d->watchSeatKeyboard(); | ||
| d->syncModifiersToXWayland(); |
Contributor
Author
There was a problem hiding this comment.
Fixed: the connection is now disconnected and reset on every watchSeatKeyboard() call (i.e. on every setSeat() change) before being rebound to the current seat. Current head: 45e4d6a (rebased onto master).
zccrs
requested changes
Sep 21, 2026
Collaborator
|
在enter到xwayland窗口后，将已经存在的修饰键发送过去不行吗？不增减新的键盘监控链路 |
Member
这个链路就是为了在没有在操作xwayland窗口时也允许xwayland程序查询修饰键的，为了支持x的全局查询修饰键的接口。 |
deepin-wm
force-pushed
the
agent/developer/1806fa699580
branch
from
September 21, 2026 05:29
a68aae4 to
250eb52
Compare
Contributor
Author
|
Thanks for the reviews. All three findings are addressed in 250eb52 (forced update):
Rebuilt on the build machine: incremental build passes with 0 errors. |
1. Listen to the seat keyboard modifiers event and proactively push wl_keyboard.modifiers to the XWayland server client when XWayland itself does not hold the keyboard focus. 2. Handle keyboard device switch/destroy and the timing where the seat is set before the XWayland server starts (re-sync on ready). 3. Allow setSeat to accept a null seat. X11 APIs that query keyboard state (e.g. XkbGetState, Qt QGuiApplication::keyboardModifiers) now reflect the current modifier state even when the keyboard focus is on a Wayland client. Log: XWayland 应用通过 X11 接口查询到的修饰键状态（Caps/Num Lock 等）不再过期。 Influence: 1. Run an XWayland app, press Caps Lock/Num Lock while focus is on a Wayland window, then check the state via X11 query APIs. 2. Switch the input method virtual keyboard on/off and verify modifier state stays correct. 3. Verify no regression in normal keyboard focus flow on XWayland apps. fix(xwayland): 同步键盘修饰键状态到 XWayland 1. 监听 seat 键盘修饰键事件，在 XWayland 自身不持有键盘焦点时 主动向其推送 wl_keyboard.modifiers。 2. 处理键盘设备切换/销毁、seat 先于 XWayland 启动的时序。 3. setSeat 支持传入空 seat。 Log: XWayland 应用通过 X11 接口查询到的修饰键状态不再过期。 Influence: 1. 在 Wayland 窗口上按 Caps/Num Lock，切换到 XWayland 应用后用 X11 接口验证锁定状态。 2. 开关输入法虚拟键盘，验证修饰键状态保持正确。 3. 验证 XWayland 应用正常键盘焦点流程无回归。
deepin-wm
force-pushed
the
agent/developer/1806fa699580
branch
from
September 30, 2026 03:55
250eb52 to
45e4d6a
Compare
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
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.
确认结果
此前键盘修饰键状态没有主动同步给 XWayland：
wl_keyboard.modifiers事件只会发给当前持有键盘焦点的客户端（wlrootswlr_seat_keyboard_send_modifiers只发给keyboard_state.focused_client）；xwayland-input.c的keyboard_handle_modifiers）会无条件把收到的wl_keyboard.modifiers应用到自己的 XKB 状态——即使它没有键盘焦点。因此当键盘焦点在某个 Wayland 客户端时（最常见场景），XWayland 收不到修饰键变化，其内部 XKB 状态停留在旧值，X11 侧查询接口（如
XkbGetState、QtQGuiApplication::keyboardModifiers）读到的就是过期状态。例如：在 Wayland 窗口里按 Caps Lock/Num Lock，切到 XWayland 应用后状态查询接口不会反映新的锁定状态。修复内容
waylib/src/server/protocols/wxwayland.cpp：wlr_keyboard.events.modifiers）修饰键变化；depressed/latched/locked/group通过wl_keyboard.modifiers推送给 XWayland 服务器客户端；setSeat先于 XWayland 启动的时序（ready后补发一次）；setSeat修正为可传入空 seat；seat 切换时重新绑定keyboardChanged连接。验证
远程构建机器全量编译通过（0 error）：
cmake --preset=default && cmake --build build -j60，libwaylibserver.so.0.10.0与build/src/treeland均构建成功。对应 Multica issue: WM-501
Summary by Sourcery
Keep XWayland's keyboard modifier state synchronized with the active Wayland seat.
Bug Fixes:
Enhancements: