Skip to content

fix(fcitx5): Fallback sang preedit mode cho sandbox apps (Flatpak/Snap) — Fix #18#19

Open
dvchd wants to merge 3 commits into
marixdev:masterfrom
dvchd:fix/sandbox-app-preedit-fallback
Open

fix(fcitx5): Fallback sang preedit mode cho sandbox apps (Flatpak/Snap) — Fix #18#19
dvchd wants to merge 3 commits into
marixdev:masterfrom
dvchd:fix/sandbox-app-preedit-fallback

Conversation

@dvchd

@dvchd dvchd commented Apr 2, 2026

Copy link
Copy Markdown

Vấn đề

Issue: #18 — Không gõ được tiếng Việt trên Gnome Ubuntu 25.10

Khi dùng VnKey Fcitx5 với các ứng dụng chạy trong sandbox (Flatpak/Snap) như OnlyOffice Flathub 9.3.1 hay Firefox snap 149, bộ gõ không hoạt động đúng. Kết quả gõ bị nhân đôi ký tự: gõ tieeng ra tieengếng thay vì tiếng.

Nguyên nhân (Root Cause)

Fcitx5 addon sử dụng 2 chế độ xử lý phím:

  1. Direct commit mode (mặc định cho GUI apps): commit từng ký tự ngay lập tức, dùng deleteSurroundingText() để sửa ký tự cũ khi engine yêu cầu backspace.
  2. Preedit mode (cho terminal): duy trì buffer nội bộ, chỉ commitString() khi hoàn thành từ.

Flatpak/Snap sandbox cách ly Input Context, khiến capability SurroundingText không khả dụng. Direct commit mode gọi commitString() cho mỗi keystroke nhưng không thể gọi deleteSurroundingText() → ký tự cũ không bị xóa → nhân đôi.

Giải pháp đã áp dụng (Fix 3 — Runtime capability check)

Tại runtime, kiểm tra capability SurroundingText của Input Context. Nếu thiếu, tự động chuyển sang preedit mode (code path đã có sẵn cho terminal emulators).

Thay đổi trong vnkey-fcitx5.cpp:

  1. Backspace handler: Thêm check SurroundingText, fallback sang preedit mode
  2. ASCII key handler: Tương tự — fallback sang preedit mode khi thiếu capability
  3. updatePreeditDisplay(): Thêm server-side setPreedit() song song với setClientPreedit(), giúp app không hỗ trợ client preedit vẫn thấy feedback qua Fcitx5 panel
  4. commitPreedit(): Clear cả client-side và server-side preedit panel sau khi commit text, tránh text cũ sót lại trên Fcitx5 panel (đặc biệt quan trọng cho sandbox apps chỉ dùng server-side preedit)

Ưu điểm: Tự động — không cần hardcode tên app, ít thay đổi code, tái dụng code path đã kiểm chứng.

Các phương án dự phòng đã cân nhắc

# Phương án Ưu Nhược
1 Detect sandbox qua tên program (/snap/, /.flatpak-info) Phát hiện sớm, rõ ràng Cần duy trì danh sách, bỏ sót sandbox mới
2 Dùng forwardKey() khi thiếu SurroundingText Đơn giản nhất Mất hoàn toàn khả năng gõ tiếng Việt
4 Commit toàn bộ từ rồi reset() mỗi lần Đơn giản Gây flickering, thay đổi toàn bộ từ
3 Runtime capability check (đã áp dụng) Tự động, an toàn, tận dụng code có sẵn Preedit có thể chậm hơn 1 chút

→ Fix 3 được chọn vì nó tự động, an toàn, và tận dụng code đã có.

Testing

Automated

  • vnkey-engine (Rust): 38/38 integration tests passed
  • vnkey-fcitx5 (C++): Build thành công libvnkey.so — 0 warnings, 0 errors
    • Environment: GCC 14.2.0, CMake 4.3.1, Fcitx5 5.1.12 headers/libs (local extract)
    • Build type: Release (-O3)

Manual test (cần thực hiện trên Ubuntu + Fcitx5)

  1. cd vnkey-engine && cargo build --release
  2. cd vnkey-fcitx5 && mkdir build && cd build && cmake .. -DCMAKE_BUILD_TYPE=Release && make
  3. sudo cp libvnkey.so /usr/lib/x86_64-linux-gnu/fcitx5/ && fcitx5 -r -d
  4. Mở OnlyOffice Flathub → gõ Telex: tieengs → nên ra tiếng
  5. Mở Firefox Snap → gõ Telex: xin chaof → nên ra xin chào
  6. Verify GUI apps bình thường (GNOME Text Editor) vẫn hoạt động

Files changed

vnkey-fcitx5/src/vnkey-fcitx5.cpp (+32, -6)

…ndingText

Flatpak/Snap apps (e.g. OnlyOffice Flathub, Firefox snap) do not support
the SurroundingText capability due to sandboxing. Previously, the direct
commit mode would call commitString() for every keystroke but could not
call deleteSurroundingText() to correct previously committed characters.
This caused doubled characters: e.g. typing 'tieeng' produced
'Goõ tieengếng Vieêệt' instead of 'Gõ tiếng Việt'.

Fix: at runtime, detect missing SurroundingText capability and route
these apps through the existing preedit mode (same code path used for
terminal emulators). Preedit mode maintains an internal buffer and only
calls commitString() when a word boundary is reached, ensuring characters
are committed atomically without needing deleteSurroundingText.

Additionally, updatePreeditDisplay() now also sets server-side preedit
(setPreedit) alongside client-side preedit (setClientPreedit), so apps
that don't support ClientPreedit still see feedback via the Fcitx5 panel.

Fixes: marixdev#18
See-also: marixdev#18
@coderabbitai

coderabbitai Bot commented Apr 2, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Updated vnkey-fcitx5 input handling: preedit display now updates both client and editor preedit, and character/backspace processing computes surrounding-text capability to choose between preedit-buffer maintenance or direct-commit flows, with capability-guarded deletion and warning logs when surrounding text is unavailable.

Changes

Cohort / File(s) Summary
Preedit Display & Input Handling
vnkey-fcitx5/src/vnkey-fcitx5.cpp
updatePreeditDisplay() now sets both client and editor preedit. Backspace and printable-character handling compute hasSurrounding and `usePreedit = isTerminal_

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hop and tweak the preedit bright,

Backspace learns when to take flight.
If surrounding text is shy, I stay—
Else I patch the commit away.
A little hop, input feels light.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: implementing a fallback to preedit mode for sandbox applications (Flatpak/Snap) when SurroundingText capability is unavailable, which directly addresses the root cause of character duplication mentioned in the PR objectives.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@vnkey-fcitx5/src/vnkey-fcitx5.cpp`:
- Around line 1073-1074: Remove the redundant redeclaration of hasSurrounding
inside the same if-block: keep the initial declaration "bool hasSurrounding =
ic_->capabilityFlags().test(CapabilityFlag::SurroundingText);" (used with
usePreedit and isTerminal_) and delete the second declaration later in the block
(the duplicate that causes redeclaration); ensure subsequent code uses the
original hasSurrounding variable and adjust references if necessary.
- Around line 873-886: commitPreedit currently clears only the internal preedit_
but leaves the server-side inputPanel preedit visible; update commitPreedit to
call ic_->inputPanel().setPreedit(Text()) (or
ic_->inputPanel().setPreedit(Text()) followed by ic_->updatePreedit() for
safety) so the panel is explicitly cleared when committing, referencing the
commitPreedit function, the preedit_ member, and
inputPanel().setPreedit()/updatePreedit() calls to locate and modify the code.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1249f52b-36ee-4931-b56d-d89ee1345a1f

📥 Commits

Reviewing files that changed from the base of the PR and between a812730 and b1db798.

📒 Files selected for processing (1)
  • vnkey-fcitx5/src/vnkey-fcitx5.cpp

Comment thread vnkey-fcitx5/src/vnkey-fcitx5.cpp
Comment thread vnkey-fcitx5/src/vnkey-fcitx5.cpp
The direct commit code path already has hasSurrounding declared earlier
in the same function scope (line 1073). Remove the duplicate declaration
at line 1108 to fix compilation error.
@dvchd dvchd force-pushed the fix/sandbox-app-preedit-fallback branch from 1e9ad52 to 2849ff8 Compare April 2, 2026 07:22

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
vnkey-fcitx5/src/vnkey-fcitx5.cpp (1)

874-885: ⚠️ Potential issue | 🟠 Major

Clear the server-side preedit when commitPreedit() finishes.

Lines 874 and 885 now populate ic_->inputPanel().setPreedit(...), but fcitx::VnKeyState::commitPreedit(bool) still only clears preedit_. In the sandbox path this leaves the last composition visible in the Fcitx panel after a commit until some later updatePreeditDisplay() or reset() happens.

Please clear the panel-side preedit inside fcitx::VnKeyState::commitPreedit(bool) as soon as preedit_ is emptied, or call updatePreeditDisplay() immediately after preedit_.clear().

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vnkey-fcitx5/src/vnkey-fcitx5.cpp` around lines 874 - 885, In
fcitx::VnKeyState::commitPreedit(bool) you currently only clear the internal
preedit_ buffer; update the server-side panel immediately as well by clearing
the input panel preedit when preedit_ is emptied (e.g. call
ic_->inputPanel().setPreedit(Text()) and/or
ic_->inputPanel().setClientPreedit(Text()) or simply invoke
updatePreeditDisplay() right after preedit_.clear()); ensure the change occurs
in commitPreedit so sandboxed apps using server-side preedit no longer show the
last composition after commit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@vnkey-fcitx5/src/vnkey-fcitx5.cpp`:
- Around line 874-885: In fcitx::VnKeyState::commitPreedit(bool) you currently
only clear the internal preedit_ buffer; update the server-side panel
immediately as well by clearing the input panel preedit when preedit_ is emptied
(e.g. call ic_->inputPanel().setPreedit(Text()) and/or
ic_->inputPanel().setClientPreedit(Text()) or simply invoke
updatePreeditDisplay() right after preedit_.clear()); ensure the change occurs
in commitPreedit so sandboxed apps using server-side preedit no longer show the
last composition after commit.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b88c8211-5f6f-443e-8705-ac4133ede282

📥 Commits

Reviewing files that changed from the base of the PR and between b1db798 and 1e9ad52.

📒 Files selected for processing (1)
  • vnkey-fcitx5/src/vnkey-fcitx5.cpp

After committing preedit text, also clear both client-side
(setClientPreedit) and server-side (setPreedit) inputPanel
preedit to prevent stale text from lingering on the Fcitx5 panel.
This is particularly important for sandbox apps (Flatpak/Snap) that
don't support ClientPreedit and rely on the server-side preedit display.
@dvchd

dvchd commented Apr 2, 2026

Copy link
Copy Markdown
Author

CodeRabbit Review Response

Finding 1: commitPreedit không clear server-side preedit

Verdict: ✅ Valid — đã sửa trong commit 1b395c0.

commitPreedit() chỉ gọi preedit_.clear() rồi commitString() mà không reset inputPanel preedit. Khi dùng preedit mode cho sandbox apps, server-side preedit trên Fcitx5 panel có thể còn hiển thị text cũ sau commit.

Đã thêm clear cả client-side và server-side preedit:

ic_->inputPanel().setClientPreedit(Text());
ic_->inputPanel().setPreedit(Text());
ic_->updatePreedit();

Finding 2: Redundant redeclaration của hasSurrounding

Verdict: ❌ Không hợp lệ — đã sửa từ commit 2849ff8.

Biến hasSurrounding chỉ khai báo 1 lần duy nhất tại dòng 1073, sử dụng chung cho cả usePreedit và phần log phía sau. Commit 2849ff8 đã loại bỏ redeclaration cũ. Không có duplicate nào trong code hiện tại.

@dvchd

dvchd commented Apr 2, 2026

Copy link
Copy Markdown
Author

🧪 Bài kiểm thử tích hợp tự động (Cấp độ 2+)

Mình đã tạo 2 nhánh test riêng biệt để chứng minh bug tồn tại và fix hoạt động đúng, tách bạch rõ ràng.


📂 Hai nhánh test

Nhánh Base từ Chứa fix? Mục đích Kết quả
test/before-fix master ❌ Không Chứng minh bug tồn tại ✅ 5/5 pass (bug confirmed)
test/after-fix fix/sandbox-app-preedit-fallback ✅ Có Xác minh fix hoạt động đúng ✅ 15/15 pass

🔴 Nhánh test/before-fix — Chứng minh bug tồn tại

Chạy trên master (chưa có fix). SandboxSimulator mô phỏng chính xác behavior code CŨ:

  • Luôn dùng direct commit mode, không phân biệt sandbox/normal
  • deleteSurroundingText() bị silent fail trong sandbox → ký tự nhân đôi
  Telex Input  | Kết quả
  -------------|--------------------
  tieeng       | SAI (thừa ký tự)
  Goof         | SAI (thừa ký tự)
  Vieejt       | SAI (thừa ký tự)
  ddoong       | SAI (thừa ký tự)
  thuowc       | SAI (thừa ký tự)

  → BUG XÁC NHẬN: 5/5 từ bị sai output

Root cause: Engine yêu cầu xóa ký tự cũ (backspace), code gọi deleteSurroundingText(), nhưng sandbox app không hỗ trợ SurroundingText capability → silent fail → ký tự nhân đôi.

cd tests/sandbox && make test   # trên nhánh test/before-fix

🟢 Nhánh test/after-fix — Xác minh fix hoạt động đúng

Chạy trên fix branch (đã có fix). FixedSandboxSimulator mô phỏng logic code MỚI:

  • Kiểm tra hasSurroundingText tại runtime
  • Fallback sang preedit mode khi !hasSurroundingText
  • Không gọi deleteSurroundingText() trong sandbox
# Bộ test Kết quả
1 Sandbox == Normal (7 từ Telex) ✅ byte-identical
2 Issue #18: câu nhiều từ ✅ 34 bytes, match
3 Sandbox: 0 deleteSurroundingText ✅ an toàn
4 Normal: vẫn dùng deleteSurroundingText ✅ không ảnh hưởng
5 Backspace trong sandbox
6 commitPreedit xóa cả 2 kênh
7 Terminal == Sandbox
8 UTF-8 integrity
9 Word boundary commit
cd tests/sandbox && make test   # trên nhánh test/after-fix

🏗 Kiến trúc test

test/before-fix (master)
  └── SandboxSimulator       → mô phỏng code CŨ  → output SAI (bug confirmed)
      └── Gọi deleteSurroundingText() → silent fail → ký tự nhân đôi

test/after-fix (fix branch)
  ├── FixedSandboxSimulator  → mô phỏng code MỚI → output ĐÚNG (fix verified)
  │   └── Fallback sang preedit mode → không gọi deleteSurroundingText()
  └── Fix code trong vnkey-fcitx5.cpp (3 commits)

Cả hai nhánh đều link tới thật libvnkey_engine.a (Rust FFI staticlib) — engine không bị mock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant