Skip to content

Fix vertical caret navigation (arrow up/down) landing on the wrong column#2741

Open
ecgoogle wants to merge 1 commit into
singerdmx:masterfrom
ecgoogle:fix/vertical-caret-navigation
Open

Fix vertical caret navigation (arrow up/down) landing on the wrong column#2741
ecgoogle wants to merge 1 commit into
singerdmx:masterfrom
ecgoogle:fix/vertical-caret-navigation

Conversation

@ecgoogle

Copy link
Copy Markdown

Description

Vertical caret navigation (Arrow Up/Down) places the caret on a seemingly random horizontal column — middle of the line, second character, end of line — and sometimes skips a visual line. The problem gets worse the more the text soft-wraps and is most visible in documents with empty or short lines. On the web it is worse still: the caret can jump horizontally by many words at once.

This PR fixes four independent bugs that together produce that behavior.

1. TextAffinity was dropped in the up/down resolution paths

At a soft line-wrap boundary the same text offset is ambiguous: it is the end of visual line k with TextAffinity.upstream, and the start of visual line k+1 with TextAffinity.downstream. RenderEditor.getTextPositionAbove/Below and RenderEditableTextBlock.getPositionAbove/Below rebuilt TextPosition(offset: ...) without copying the affinity — both on the way down (the local position handed to the child) and on the way up (the result). The caret therefore started from, or landed on, the wrong visual line, which reads as "the caret skipped a line" or "it jumped to the end/start of the line".

2. QuillVerticalCaretMovementRun had no goal column

The run only stored the current TextPosition, so each arrow step recomputed the target column from the current caret position. Crossing a short or empty line clamped the column to that line — permanently, for the rest of the run. Flutter's own TextField (VerticalCaretMovementRun) and every major editor remember the column the run started on and restore it on the next line that is long enough. The run now captures the starting caret dx on its first move and snaps every step back to that goal column (only the horizontal component is re-resolved; the line chosen by getTextPositionAbove/Below is kept).

3. On the web, arrow up/down were handled by the browser, not by the editor

The framework's DefaultTextEditingShortcuts maps the arrow keys to DoNothingAndStopPropagationTextIntent on the web, and the editor registers DoNothingAction(consumesKey: false), so the keydown reaches the browser, which moves the caret inside the hidden DOM input used for text editing. That is correct for DOM-rendered text fields, but the Quill editor is rendered by Flutter: the hidden element has a completely different text layout (width, font, wrapping), so the browser-computed caret landed on an arbitrary visual column — off by entire words, and increasingly wrong as zoom or window size changes the canvas wrapping. With this delegation in place, fixes 1–2 were not even reachable on the web.

defaultSinlgeActivatorIntents() now binds Arrow Up/Down (with and without Shift) to ExtendSelectionVerticallyToAdjacentLineIntent when kIsWeb is true. The editor's Shortcuts widget sits closer to the focused node than DefaultTextEditingShortcuts, so these bindings take precedence and vertical navigation runs through the editor's real geometry. Horizontal arrows are intentionally left to the browser: they operate on plain character offsets, which map 1:1.

4. On the web, the cached vertical run was never invalidated

_didChangeTextEditingValue returns early in its kIsWeb branch — before the call to adjacentLineAction.stopCurrentVerticalRunIfSelectionChanges(). After repositioning the caret with a mouse click, the next arrow key continued the cached run from the stale position. This was latent as long as bug 3 sent the arrow keys to the browser; it surfaces as soon as the editor handles them. The invalidation call is now made at the top of the method, on all platforms.

Tests

test/editor/vertical_caret_navigation_test.dart (new) drives a real QuillEditor with synthetic arrow-key events:

  • goal column restored after crossing an empty line (down and up);
  • goal column restored after crossing a shorter line;
  • goal column restored after crossing an empty bullet list item;
  • a selection change between arrow presses invalidates the cached run (regression for bug 4);
  • sanity: down/up between equal-length lines keeps the column.

The regression tests were verified to fail against the unpatched code (the column stays clamped to the start/end of the crossed line) and pass with the fix.

Authorship

These fixes were diagnosed, implemented, and tested by Claude Code (Anthropic), model Fable 5 (Mythos 5), working on a Flutter app where the bugs were originally reproduced. The patch was first validated in real use against flutter_quill 11.5.0 via a local fork before being ported to master. The submitting human reviewed the changes and verified the behavior manually on Flutter web (caret navigation through wrapped paragraphs, empty lines, and lists, with and without text scaling).

Related Issues

Type of Change

  • Feature: New functionality without breaking existing features.
  • 🛠️ Bug fix: Resolves an issue without altering current behavior.
  • 🧹 Refactor: Code reorganization, no behavior change.
  • Breaking: Alters existing functionality and requires updates.
  • 🧪 Tests: New or modified tests
  • 📝 Documentation: Updates or additions to documentation.
  • 🗑️ Chore: Routine tasks, or maintenance.
  • Build configuration change: Build/configuration changes.

Four independent bugs made arrow up/down place the caret on a wrong
horizontal column (and sometimes the wrong visual line):

1. TextAffinity was dropped while resolving the position above/below in
   RenderEditor and RenderEditableTextBlock, so at soft line-wrap
   boundaries the movement started from, or landed on, the wrong visual
   line.

2. QuillVerticalCaretMovementRun had no goal column: crossing a short or
   empty line permanently clamped the caret to that line's column. The
   run now captures the starting caret dx and snaps every step back to
   it, mirroring Flutter's own VerticalCaretMovementRun.

3. On the web, arrow up/down were delegated to the browser
   (DefaultTextEditingShortcuts maps them to
   DoNothingAndStopPropagationTextIntent), which moved the caret based
   on the hidden DOM input's unrelated text layout. The editor now binds
   the vertical movements itself when kIsWeb.

4. On the web, the cached vertical movement run was never invalidated
   (early return in _didChangeTextEditingValue), so after repositioning
   the caret with a mouse click the next arrow key continued from the
   stale position.

Diagnosed, implemented, and tested by Claude Code (Anthropic),
model Fable 5 (Mythos 5).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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