fix: treat empty token in place_below as bottom sentinel - #1399
Merged
Merged
Conversation
Reviewer's GuideImplements remote subsurface protocol v2 by adding explicit above-all/below-all stacking requests, changing empty or unknown sibling tokens to invalid_sibling errors, and advertising interface version 2 while retaining version-gated compatibility for v1 clients. Sequence diagram for remote subsurface v2 stacking requestssequenceDiagram
participant Client
participant Manager as WRemoteSubsurfaceManagerV1
participant Subsurface as RemoteSubsurfaceContext
participant Parent
Client->>Manager: bind(version=2)
Client->>Subsurface: place_above_all()
Subsurface->>Manager: placeChildAboveParentTop(this)
Manager->>Parent: syncRemoteSubsurfaceOrder(parent)
Parent-->>Client: remote subsurface above siblings and parent content
Client->>Subsurface: place_below_all()
Subsurface->>Manager: placeChildBelowParentBottom(this)
Manager->>Parent: syncRemoteSubsurfaceOrder(parent)
Parent-->>Client: remote subsurface below siblings and parent content
Sequence diagram for invalid remote subsurface sibling tokenssequenceDiagram
participant Client
participant Subsurface as RemoteSubsurfaceContext
participant Manager as WRemoteSubsurfaceManagerV1
Client->>Subsurface: place_above(sibling_token)
Subsurface->>Manager: findExportedByToken(sibling_token)
Manager-->>Subsurface: no matching sibling
Subsurface-->>Client: invalid_sibling(sibling_token)
Client->>Subsurface: place_below(sibling_token)
Subsurface->>Manager: findExportedByToken(sibling_token)
Manager-->>Subsurface: no matching sibling
Subsurface-->>Client: invalid_sibling(sibling_token)
Note over Client,Subsurface: Z-order remains unchanged
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
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="waylib/src/server/protocols/wremotesubsurfacemanagerv1.cpp" line_range="185-189" />
<code_context>
void set_position(Resource *resource, int32_t x, int32_t y) override;
void place_above(Resource *resource, const QString &sibling_token) override;
void place_below(Resource *resource, const QString &sibling_token) override;
+ void place_above_all(Resource *resource) override;
+ void place_below_all(Resource *resource) override;
</code_context>
<issue_to_address>
**issue (bug_risk):** The implementation adds `place_above_all` and `place_below_all` overrides and advertises interface version 2, but the diff does not update the `treeland-remote-subsurface-unstable-v1.xml` protocol source used by CMake to generate the QtWayland server base classes. The generated base therefore has no v2 requests, so these `override` declarations fail to compile; if the generated code is supplied externally, the server instead advertises version 2 without exposing the new requests in the wire protocol.
**Suggested fix:** Update the protocol XML (and its generated bindings or protocol dependency) to define both requests at version 2 before compiling this implementation.
</issue_to_address>
1. An empty sibling_token in place_below now places the sub-surface
at the very bottom of the sibling stack, symmetric to the
empty-token "top" behavior of place_above
2. Add placeChildBelowParentBottom helper that prepends to
belowChildren and syncs the z-order
3. Matches the updated treeland-remote-subsurface-unstable-v1
protocol, which documents the empty string as a sentinel for
both restack requests
Log: place_below with an empty sibling_token no longer emits
invalid_sibling; it now moves the sub-surface to the very bottom of
the stacking order, as place_above("") already moves to the top.
Influence:
1. Send place_below with an empty token and verify the remote
subsurface is placed below all siblings and below parent content
2. Send place_above with an empty token and verify it still moves
the subsurface to the very top
3. Send place_above/place_below with an unknown token and verify
invalid_sibling is emitted and z-order is unchanged
4. Regression: ordering by parent token and sibling token keeps
working as before
fix: place_below 空 token 作为置底哨兵值处理
1. place_below 传入空 sibling_token 时现在会将子表面放到兄弟
堆叠的最底部，与 place_above 空 token "置顶"行为对称
2. 新增 placeChildBelowParentBottom 辅助函数，prepend 到
belowChildren 并同步 z-order
3. 与更新后的 treeland-remote-subsurface-unstable-v1 协议一致，
该协议将空字符串记录为两个重排请求的哨兵值
Log: place_below 传入空 sibling_token 不再发送 invalid_sibling，
而是将子表面移到堆叠最底部，与 place_above("") 已有的置顶行为
对称。
Influence:
1. 发送携带空 token 的 place_below，验证远程子表面被放到所有
兄弟之下且位于父内容之下
2. 发送携带空 token 的 place_above，验证其仍将子表面移到最顶部
3. 发送携带未知 token 的 place_above/place_below，验证发出
invalid_sibling 事件且 z-order 不变
4. 回归：按父 token 和兄弟 token 的排序行为保持不变
PMS: TASK-393795
wineee
force-pushed
the
wine-update-0.2
branch
from
September 14, 2026 09:35
159bd39 to
b70c6d6
Compare
wineee
changed the title
zccrs
approved these changes
Sep 14, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wineee, zccrs 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 |
Member
Author
|
/merge |
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.
at the very bottom of the sibling stack, symmetric to the
empty-token "top" behavior of place_above
belowChildren and syncs the z-order
protocol, which documents the empty string as a sentinel for
both restack requests
Log: place_below with an empty sibling_token no longer emits
invalid_sibling; it now moves the sub-surface to the very bottom of
the stacking order, as place_above("") already moves to the top.
Influence:
subsurface is placed below all siblings and below parent content
the subsurface to the very top
invalid_sibling is emitted and z-order is unchanged
working as before
fix: place_below 空 token 作为置底哨兵值处理
堆叠的最底部，与 place_above 空 token "置顶"行为对称
belowChildren 并同步 z-order
该协议将空字符串记录为两个重排请求的哨兵值
Log: place_below 传入空 sibling_token 不再发送 invalid_sibling，
而是将子表面移到堆叠最底部，与 place_above("") 已有的置顶行为
对称。
Influence:
兄弟之下且位于父内容之下
invalid_sibling 事件且 z-order 不变