fix(waylib): guard text-input-v3 focus transitions to avoid crash#1066
Conversation
|
Hi @LFRon. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideGuard and synchronize text-input-v3 focus transitions with wlroots’ raw focus state to avoid assertion failures when focus is resent, and ensure all tracked text inputs are properly left on notifyLeave(). Sequence diagram for guarded text-input-v3 focus transitionssequenceDiagram
actor Client
participant WInputMethodHelper
participant WTextInputV3
participant WSurface
participant wlroots_text_input_v3
Client->>WInputMethodHelper: notifyLeave()
WInputMethodHelper->>WTextInputV3: focusedSurface()
alt any_text_input_has_focused_surface
WInputMethodHelper->>WTextInputV3: sendLeave()
WTextInputV3->>wlroots_text_input_v3: send_leave()
end
Client->>WTextInputV3: sendEnter(surface)
WTextInputV3->>WSurface: handle()
WTextInputV3->>wlroots_text_input_v3: read focused_surface
alt focused_surface == targetSurface
WTextInputV3-->>Client: no-op
else focused_surface != targetSurface
opt focused_surface_non_null
WTextInputV3->>wlroots_text_input_v3: send_leave()
end
WTextInputV3->>wlroots_text_input_v3: send_enter(targetSurface)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
WTextInputV3::sendEnter/sendLeave, consider using the existingfocusedSurface()accessor (or refactoring it) instead of directly accessinghandle()->handle()->focused_surfaceso the focus-state logic is centralized in one place. - In
WInputMethodHelper::notifyLeave, now that you iterate over alltextInputs, it may be clearer and safer to iterate over a copy or a stable container ifsendLeave()can indirectly mutated->textInputs, to avoid potential invalidation during the loop.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `WTextInputV3::sendEnter`/`sendLeave`, consider using the existing `focusedSurface()` accessor (or refactoring it) instead of directly accessing `handle()->handle()->focused_surface` so the focus-state logic is centralized in one place.
- In `WInputMethodHelper::notifyLeave`, now that you iterate over all `textInputs`, it may be clearer and safer to iterate over a copy or a stable container if `sendLeave()` can indirectly mutate `d->textInputs`, to avoid potential invalidation during the loop.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
ece45ba to
9aba4b2
Compare
|
附上崩溃堆栈: |
9aba4b2 to
cfcb9ce
Compare
wlroots requires wlr_text_input_v3_send_enter() to be called only when the text input has no focused surface. WInputMethodHelper could resend keyboard focus while wlroots still kept a raw focused_surface, causing the focused_surface == NULL assertion to abort. Make WTextInputV3::sendEnter() synchronize with the wlroots focus state by no-oping when the target surface is already focused and sending leave before entering a different surface. Also make sendLeave() check the raw wlroots focused_surface directly, and let notifyLeave() clear all currently focused text inputs instead of only the first wrapper-visible one. This keeps text-input-v3 focus transitions idempotent and avoids crashes when focus is resent for Xwayland/Wayland surfaces.
cfcb9ce to
e6ef117
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: LFRon, 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 |
|
此PR我看着ok, 感谢你的贡献。 |
该treeland崩溃在我使用Android Studio时遇到
wlroots requires wlr_text_input_v3_send_enter() to be called only when the text input has no focused surface. WInputMethodHelper could resend keyboard focus while wlroots still kept a raw focused_surface, causing the focused_surface == NULL assertion to abort.
Make WTextInputV3::sendEnter() synchronize with the wlroots focus state by no-oping when the target surface is already focused and sending leave before entering a different surface. Also make sendLeave() check the raw wlroots focused_surface directly, and let notifyLeave() clear all currently focused text inputs instead of only the first wrapper-visible one.
This keeps text-input-v3 focus transitions idempotent and avoids crashes when focus is resent for Xwayland/Wayland surfaces.
Summary by Sourcery
Guard text-input-v3 focus transitions to align with wlroots expectations and prevent crashes when focus is resent.
Bug Fixes: