Skip to content

Fix iOS long-press selection latching on forever - #4

Open
MarcelKaeding wants to merge 1 commit into
feature/upgrade-to-latestfrom
fix/ios-long-press-latch
Open

Fix iOS long-press selection latching on forever#4
MarcelKaeding wants to merge 1 commit into
feature/upgrade-to-latestfrom
fix/ios-long-press-latch

Conversation

@MarcelKaeding

Copy link
Copy Markdown

Fixes the long-standing iPad/iPhone bug where the document editor gets stuck in a "select-and-highlight" mode: a permanent magnifier, every drag highlighting text by word, and the list slowly auto-scrolling until the magnifier hits the top of the screen. It only clears when something rebuilds the interactor — which is why the reported workaround (open Talk, dismiss it) "fixes" it.

Cause

_onLongPressDown starts a long-press selection and shows the magnifier, but it doesn't set _dragMode — only _onPanStart does. The only path that ends a long-press, _onLongPressEnd, is reachable exclusively through _onDragSelectionEnd, which requires that _dragMode.

So a long-press that never becomes a drag is never torn down. _longPressStrategy stays non-null, and since EagerPanGestureRecognizer.shouldAccept returns true whenever _isLongPressInProgress, the next unrelated pan anywhere in the document is claimed as a long-press drag-selection — magnifier up, auto-scroller running, no drag end to take either back down.

It's reachable whenever the pointer is taken away after the long-press timer fired: another recognizer wins the arena, or a widget's hit-test behaviour changes mid-gesture. Superlist hits it by moving focus between a list's title and its body, which flips every task component between opaque and translucent hit-testing.

Android has never had this — its _onTapUp and _onPanCancel both end an in-progress long-press. This mirrors that on iOS.

Changes

  • End the long-press in _onPanCancel even when no drag started.
  • End it in _onTapUp too, and skip that method's ordinary tap handling so the release doesn't toggle the just-revealed toolbar back off. _didEndLongPressDuringCurrentGesture coordinates the two, since both fire for the same pointer and _onPanCancel runs first.
  • Keep showing the toolbar on release when the long-press produced a collapsed selection (e.g. an empty paragraph), which _onLongPressEnd alone doesn't do.
  • Cancel _tapDownLongPressTimer in dispose(). It's armed on every tap-down and lives outside the gesture arena, so it can outlive the State and raise a magnifier on the ancestor-owned controls controller that no interactor is left to lower.
  • Make _updateOverlayControlsAfterFinishingDragSelection null-safe on the selection, now that it also runs from the tap-up and pan-cancel paths.

Testing

Adds a regression test for the cancelled-press case — without the fix, the following drag raises the magnifier.

Full suite on this branch: 5564 passing, versus 5563 on the branch point, with an identical set of 11 pre-existing failures. No regressions.

Not yet verified on a physical iPad — worth walking the original reporter's steps on a device before closing their report.

`_onLongPressDown` starts a long-press selection and shows the magnifier,
but it doesn't set `_dragMode` — only `_onPanStart` does. The only path
that ends a long-press, `_onLongPressEnd`, is reachable exclusively
through `_onDragSelectionEnd`, which requires that `_dragMode`.

So a long-press that never becomes a drag is never torn down.
`_longPressStrategy` stays non-null, and because
`EagerPanGestureRecognizer.shouldAccept` returns true whenever
`_isLongPressInProgress`, the *next* unrelated pan anywhere in the
document is claimed as a long-press drag-selection: text highlights by
word under the finger, the magnifier comes up, and the drag auto-scroller
starts — with no matching drag end to take any of it back down. The
editor only recovers when something rebuilds the interactor.

This is reachable whenever the pointer is taken away from the document
after the long-press timer fired: another recognizer in the arena wins
it, or the widget's hit-test behaviour changes mid-gesture. Superlist
hits it by moving focus between a list's title and its body, which flips
every task component between opaque and translucent hit-testing.

Android has never had this: its `_onTapUp` and `_onPanCancel` both end an
in-progress long-press. Mirror that on iOS.

- End the long-press in `_onPanCancel` even when no drag started.
- End it in `_onTapUp` too, and skip that method's ordinary tap handling
  so the release doesn't toggle the just-revealed toolbar back off.
  `_didEndLongPressDuringCurrentGesture` coordinates the two, since both
  can fire for the same pointer and `_onPanCancel` runs first.
- Keep showing the toolbar on release even when the long-press produced a
  collapsed selection, e.g. on an empty paragraph.
- Cancel `_tapDownLongPressTimer` in `dispose()`. It's armed on every
  tap-down and lives outside the gesture arena, so it can outlive the
  State and raise a magnifier on the ancestor-owned controls controller
  that no interactor is left to lower.
- Make `_updateOverlayControlsAfterFinishingDragSelection` null-safe on
  the selection, now that it also runs from the tap-up and pan-cancel
  paths, where the selection may already be gone.

Adds a regression test covering the cancelled-press case: without the
fix, the following drag raises the magnifier.
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.

2 participants