fix: guard IME composition state before early returns in RangeBinding

[Xuan4781]

Fixes #14533

Issue

After inserting a hyperlink and typing Chinese (or any IME language)
directly after it, the line would freeze and become unselectable. The
cursor would appear briefly then disappear on click. English input was
unaffected, and adding a space after the hyperlink before typing would
avoid the bug.

Fix

• isComposing was only set to true after the if (!to) return early
  return in _onCompositionStart, so single-block cursor positions (e.g.
  typing right after a hyperlink) never set the flag which left
  _onNativeSelectionChanged firing and reconciling the DOM mid-IME composition
• Moved this.isComposing = true to the top of _onCompositionStart before
  any early returns
• Added this.isComposing = false unconditionally at the top of
  _onCompositionEnd so the flag is always cleared

Screenshot Verification

Summary by CodeRabbit

• Bug Fixes
  • Fixed table command insertion anchor behavior when operating within lists
  • Improved composition state management for more consistent text input handling

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes address two separate concerns: (1) improving table command insertion logic to properly traverse parent list blocks when computing the target anchor, and (2) fixing IME composition state tracking by ensuring explicit state transitions occur at event boundaries rather than conditional checks.

Changes

Cohort / File(s)	Summary
Table Command Insertion Logic blocksuite/affine/blocks/table/src/commands.ts	Modified target insertion anchor computation to use a mutable targetModel that climbs upward through parent affine:list blocks instead of always using the initially selected model.
IME Composition State Tracking blocksuite/framework/std/src/inline/range/range-binding.ts	Fixed composition state transitions by ensuring isComposing is set to false at the start of _onCompositionEnd and to true immediately in _onCompositionStart, aligning state changes to event boundaries.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The table command change in commands.ts adjusts insertion anchor handling for nested lists, which is unrelated to the IME composition fix and appears to be out-of-scope.	Remove the unrelated table command changes from this PR; either revert commands.ts changes or move them to a separate focused pull request addressing list context insertion.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and accurately describes the main fix: moving the IME composition state guard before early returns in RangeBinding, which aligns with the primary change in range-binding.ts.
Linked Issues check	✅ Passed	The changes directly address issue #14533 by moving isComposing = true before early returns and setting isComposing = false unconditionally, preventing DOM reconciliation during IME composition that caused the freeze.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

blocksuite/affine/blocks/table/src/commands.ts (1)

23-54: ⚠️ Potential issue | 🟡 Minor

removeEmptyLine cleanup no longer targets the originally selected block.

After the new while-loop reassigns targetModel to an ancestor when the selection lives inside an affine:list, the removeEmptyLine branch at Line 52–54 now inspects targetModel.text?.length on that ancestor (a list block, which typically has no text) instead of the originally selected empty line. As a result:

• The intended cleanup of the empty anchor line inside a list is silently skipped (regression vs. prior behavior when the selection was a list item).
• If an ancestor ever does satisfy text?.length === 0, deleteBlock would be called on the wrong node.

Preserve the original selection for the cleanup check while using the climbed model only as the sibling anchor.

🔧 Proposed fix

-  let targetModel =
+  const originalTargetModel =
     place === 'before'
       ? selectedModels[0]
       : selectedModels[selectedModels.length - 1];
-  if (!targetModel) return;
+  if (!originalTargetModel) return;
+  let targetModel: BlockModel = originalTargetModel;

   let parent = std.store.getParent(targetModel);
   while (parent && parent.flavour === 'affine:list') {
     targetModel = parent;
     parent = std.store.getParent(targetModel);
   }
@@
-  if (removeEmptyLine && targetModel.text?.length === 0) {
-    std.store.deleteBlock(targetModel);
+  if (removeEmptyLine && originalTargetModel.text?.length === 0) {
+    std.store.deleteBlock(originalTargetModel);
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@blocksuite/affine/blocks/table/src/commands.ts` around lines 23 - 54, Save
the originally selected anchor before climbing the parent chain (e.g., store
selectedModels[0] or the initially computed targetModel into a variable like
originalTargetModel) and continue to use the climbed targetModel only for
finding the sibling insertion point and calling std.store.addSiblingBlocks;
then, when handling removeEmptyLine, check originalTargetModel.text?.length and
call std.store.deleteBlock(originalTargetModel) so the cleanup targets the
original selected block rather than the ancestor.

🧹 Nitpick comments (1)

blocksuite/framework/std/src/inline/range/range-binding.ts (1)

87-98: LGTM — flag transitions now align to event boundaries.

Setting isComposing = true at the top of _onCompositionStart (Line 98) correctly guards against _onNativeSelectionChanged reconciling the DOM during IME composition in the single-block cursor case (to == null), which was the root cause of the hyperlink+IME freeze. Likewise, unconditionally clearing the flag at the top of _onCompositionEnd (Line 88) ensures the flag is never left stuck true if the callback path isn't taken. The cross-block path via inline-range-provider.ts (which reads binding?.isComposing) continues to work since the flag is still set during composition.

Minor note: the redundant this.isComposing = false at Line 121 inside _compositionStartCallback is now dead (already cleared at Line 88 before the callback runs at Line 92). Safe to leave, but could be removed for clarity.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@blocksuite/framework/std/src/inline/range/range-binding.ts` around lines 87 -
98, Remove the now-dead assignment that clears the composition flag inside the
composition callback: locate the _compositionStartCallback implementation
(referenced as _compositionStartCallback) within range-binding.ts and delete the
inner "this.isComposing = false" statement (the redundant clear inside the
callback) because isComposing is already set to false at the top of
_onCompositionEnd and clearing it inside the callback is unnecessary and
confusing.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@blocksuite/affine/blocks/table/src/commands.ts`:
- Around line 23-54: Save the originally selected anchor before climbing the
parent chain (e.g., store selectedModels[0] or the initially computed
targetModel into a variable like originalTargetModel) and continue to use the
climbed targetModel only for finding the sibling insertion point and calling
std.store.addSiblingBlocks; then, when handling removeEmptyLine, check
originalTargetModel.text?.length and call
std.store.deleteBlock(originalTargetModel) so the cleanup targets the original
selected block rather than the ancestor.

---

Nitpick comments:
In `@blocksuite/framework/std/src/inline/range/range-binding.ts`:
- Around line 87-98: Remove the now-dead assignment that clears the composition
flag inside the composition callback: locate the _compositionStartCallback
implementation (referenced as _compositionStartCallback) within range-binding.ts
and delete the inner "this.isComposing = false" statement (the redundant clear
inside the callback) because isComposing is already set to false at the top of
_onCompositionEnd and clearing it inside the callback is unnecessary and
confusing.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f9ca97f5-1af6-4809-9b82-150294f52b45

📥 Commits

Reviewing files that changed from the base of the PR and between 0849b34 and 6214b65.

📒 Files selected for processing (2)

• blocksuite/affine/blocks/table/src/commands.ts
• blocksuite/framework/std/src/inline/range/range-binding.ts