Fix iOS long-press selection latching on forever - #4
Open
MarcelKaeding wants to merge 1 commit into
Open
Conversation
`_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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
_onLongPressDownstarts a long-press selection and shows the magnifier, but it doesn't set_dragMode— only_onPanStartdoes. 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.
_longPressStrategystays non-null, and sinceEagerPanGestureRecognizer.shouldAcceptreturns 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
opaqueandtranslucenthit-testing.Android has never had this — its
_onTapUpand_onPanCancelboth end an in-progress long-press. This mirrors that on iOS.Changes
_onPanCanceleven when no drag started._onTapUptoo, and skip that method's ordinary tap handling so the release doesn't toggle the just-revealed toolbar back off._didEndLongPressDuringCurrentGesturecoordinates the two, since both fire for the same pointer and_onPanCancelruns first._onLongPressEndalone doesn't do._tapDownLongPressTimerindispose(). It's armed on every tap-down and lives outside the gesture arena, so it can outlive theStateand raise a magnifier on the ancestor-owned controls controller that no interactor is left to lower._updateOverlayControlsAfterFinishingDragSelectionnull-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.