Skip to content

Put the caret where a withheld tap landed when iOS 27 reverts it - #5983

Closed
ethan-james wants to merge 15 commits into
mainfrom
copilot/fix/5981-same-thought-revert
Closed

ethan-james wants to merge 15 commits into
mainfrom
copilot/fix/5981-same-thought-revert

Conversation

@ethan-james

@ethan-james ethan-james commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #5981.

After a double tap on iOS 27, a tap within the thought being edited is withheld (#5660), and iOS moves the caret into the tapped word and then back to where it was before the tap, within about 300ms. A new useRevertCorrection hook, mounted beside useCaretRestore on the thought being edited and on a focused note, notices that return while the tap is still open and puts the caret where the tap landed, on the next animation frame. On a thought it goes through the same setCaretOffset as useEditMode's mousedown handler, so the cursor offset in Redux follows the corrected caret.

The return is detected from the selection itself rather than after a fixed delay: on devices it came between 150ms and 280ms after the touchstart, and on BrowserStack at about 145ms. A move back to the offset the caret had before the touch is only treated as a revert while the touch is still open, and only after the selection first moved away from it. There is no time limit: the watch ends with the touch, at its touchend or touchcancel or once it moves farther than TOUCH_SLOP, or on any other input, namely a touchstart, mousedown, keydown or beforeinput anywhere. A tap whose computed offset is the caret's own position, such as a tap on the caret, is left alone.

Plan

1. Existing surface area

  • docs/cursor-and-caret.md "Caret restoration on iOS": useCaretRestore "keeps the caret where it belongs, scoped to isTouch && isSafari(). It registers a selectionchange listener that restores the caret to its editable when the selection has left it". The restore is to one end of the text "rather than to a remembered position, which cannot go stale against an edit that shortens the value".
  • src/components/Editable/useCaretRestore.ts already tracks the press on the editable and reacts to selectionchange, but deliberately ignores a press in progress:
    const onTouchStart = () => {
      pressingRef.current = true
      scrubbingRef.current = false
    }
    ...
    if (pressingRef.current || lastTouch.isRecent()) return
    It is mounted for the thought being edited (useEditMode.ts:66, useCaretRestore({ editableRef: contentRef, enabled: isCursor && !!editing })) and for a focused note (Note.tsx:83).
  • src/device/getCaretOffset.ts:459 computes the offset a tap at a point places the caret at, with iOS-style word snapping, and is what useEditMode's onMouseDown uses for a tap that is not withheld:
    const getCaretOffset = (editable: HTMLElement | null, { clientX, clientY }: Coordinates): CaretOffsetResult => {
  • src/device/selection.ts:259-261 offsetFromNode(node) returns the caret offset within an editable, or null when the selection is outside it:
    export const offsetFromNode = (node: Node): number | null => {
      const selection = window.getSelection()
      if (!selection?.focusNode || !node.contains(selection.focusNode)) return null
  • src/stores/osVersionStore.ts gates other iOS 27 workarounds (useLongPress.ts:135):
    if (!pressing || (osVersionStore.getState() ?? 0) < 27) return

2. Build vs. extend

Extend two existing pieces. initEvents' capture-phase touchstart already latches per-touch state into touchStore (pressOnCaret, measuring the caret once per touch), so it records the watch there too, and its capture-phase listeners end it on any other input. The correction itself is a new hook, useRevertCorrection, mounted wherever useCaretRestore is (the thought being edited and a focused note), with the same scoping. useCaretRestore keeps its one job, the keyboard trackpad. On a thought, useEditMode passes its setCaretOffset, hoisted out of its caret effect so that onMouseDown and the correction share it.

3. Adjacent behaviour

4. Approaches

  • Timer after the touch (as in Re-place the caret on iOS when WebKit reverts a tap near the caret #5655): rejected. The revert's timing varies, and a timer either races it or adds a visible delay.
  • In useEditMode's caret effect (the prototype): rejected. It would cover thoughts only and duplicate the press tracking useCaretRestore already does.
  • All in useCaretRestore, with its own listeners per editable: rejected. It duplicates the touch tracking initEvents already does for every touch.
  • Correction inside useCaretRestore: rejected. It mixes two unrelated jobs behind one selectionchange listener and only sets the DOM selection, leaving the cursor offset in Redux behind.
  • Watch in initEvents and touchStore, correction in useRevertCorrection, applied through useEditMode's setCaretOffset on thoughts: chosen.

5. Diff sketch

  • touchStore.ts: a revertWatch field.
  • initEvents.ts: in the capture-phase touchstart, on WebKit 27, record the focused editable, the touch point and the collapsed caret offset of a single-finger touch. Capture-phase listeners end the watch on touchend, touchcancel, mousedown, keydown, beforeinput, and a touchmove beyond TOUCH_SLOP.
  • useRevertCorrection.ts (new): on selectionchange, mark a move away from that offset, and on the return place the caret at getCaretOffset of the touch point, through onCorrect if given.
  • useEditMode.ts: hoist setCaretOffset and pass it to useRevertCorrection. Note.tsx: mount useRevertCorrection.
  • docs/cursor-and-caret.md: a paragraph under "Caret restoration on iOS".
  • The BrowserStack test in doubleTap.ios27.ts, unskipped.

ethan-james and others added 2 commits October 14, 2026 10:33
On iOS 27, after a double tap, a tap in the middle of another word of the
thought being edited moves the caret there and then back to where it was
(#5981).

Co-Authored-By: GitHub Copilot Claude Opus 5.5 <223556219+Copilot@users.noreply.github.com>
On WebKit 27, a tap within the thought being edited whose touchend is
withheld gets no mouse events, and iOS moves the caret to the tap and then
back to where it was. useCaretRestore records the touch point and caret at
touchstart and, when the selection returns to that offset while the touch
is still open, puts the caret at the tapped offset a frame later.

Co-Authored-By: GitHub Copilot Claude Opus 5.5 <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor
Preview Deployment · Oct 27, 2026, 6:08 PM UTC · 10a4e72

Preview deployment

They replaced getCaretOffset with a mock and simulated iOS's selection
moves, and the iOS test in doubleTap.ios27.ts already covers the same
behavior on a device.

Co-Authored-By: GitHub Copilot Claude Opus 5.5 <223556219+Copilot@users.noreply.github.com>
A caret drag that crossed its starting offset within 500ms was taken for a
revert, flashing the caret to the tapped word edge mid-drag.

Co-Authored-By: GitHub Copilot Claude Opus 5.5 <223556219+Copilot@users.noreply.github.com>
A touch or mousedown anywhere, or a keydown or beforeinput on the editable,
ends the watch, so the correction no longer depends on how long iOS takes
to revert.

Co-Authored-By: GitHub Copilot Claude Opus 5.5 <223556219+Copilot@users.noreply.github.com>
The watch moves into touchStore, written by initEvents' capture-phase
touchstart alongside pressOnCaret and cleared by its capture-phase
listeners for every other input. useCaretRestore keeps only the
correction, for its own editable.

Co-Authored-By: GitHub Copilot Claude Opus 5.5 <223556219+Copilot@users.noreply.github.com>
A touchmove without touches, as dispatched in initEvents' unit tests, threw
in the listener.

Co-Authored-By: GitHub Copilot Claude Opus 5.5 <223556219+Copilot@users.noreply.github.com>
Co-Authored-By: GitHub Copilot Claude Opus 5.5 <223556219+Copilot@users.noreply.github.com>
useCaretRestore goes back to restoring the caret after the keyboard
trackpad. The correction gets its own hook, mounted beside it, and on a
thought useEditMode applies it with the same setCaretOffset its mousedown
handler uses, so the cursor offset in Redux follows the corrected caret.

Co-Authored-By: GitHub Copilot Claude Opus 5.5 <223556219+Copilot@users.noreply.github.com>
if (offset === null) return

if (!watch.moved && offset !== watch.before) touchStore.update({ revertWatch: { ...watch, moved: true } })
if (!watch.moved || offset !== watch.before) return

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Too many return statements in this function, but they all have a purpose. Would it be better to move them into helper functions?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The return statements are reasonable IMO, but it might be helpful to include a comment for each explaining what that purpose is.

@trevinhofmann trevinhofmann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Generally LGTM, just a few minor comments below.

if (offset === null) return

if (!watch.moved && offset !== watch.before) touchStore.update({ revertWatch: { ...watch, moved: true } })
if (!watch.moved || offset !== watch.before) return

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The return statements are reasonable IMO, but it might be helpful to include a comment for each explaining what that purpose is.

await browser.pause(1000)

const offset = await browser.execute(() => document.getSelection()?.focusOffset ?? null)
expect(offset).toBeGreaterThanOrEqual('Existentialism '.length)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is there a reason to use toBeGreaterThanOrEqual rather than asserting the exact expected offset?

@ethan-james ethan-james Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Copilot says:

The exact value would be 15, 'Existentialism '.length, the start of "eternally". But asserting it exactly would make the test depend on details that don't matter to the user.

An exact 15 tests em's word snapping, not the bug. It would depend on where the synthesized tap lands relative to the middle of the word. 19 is only half a character from tying with the end. A slightly different landing could snap to 24, which is just as correct.

I would support that argument mainly because our word snapping works a little differently from Safari's, and I foresee a future in which we fix that because it can be annoying to have the caret bouncing around.

Comment on lines +86 to +87
// iOS moves the caret back within about 300ms of the tap
await browser.pause(1000)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

There's a potential race condition here, but I'm not sure how we can avoid it without adding complexity to the implementation.

Maybe we could wait for the offset where iOS moves the caret, and then wait for the offset where we place the caret back? There's likely a separate worse race condition there, though, where the caret is placed back before the test detects the original move by iOS.

IMO, this await browser.pause isn't ideal, but acceptable as-is and the comment clarifying the 300ms-vs-1000ms race helps justify it.

@ethan-james ethan-james Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

OK, I worked out a version that waits for the revert and correct events, or passes if they don't arrive by the timeout. That way it will fail if a revert goes uncorrected, but will pass if the revert never occurs.

Record the caret's offsets in the page from the tap on, and wait for it to
leave, return to where it was, and leave again. Without the fix the offsets
stop at the revert ([24,19,14]) and the wait fails, naming them. With it they
end in the correction ([24,19,14,15]), so the test no longer waits a full
second, and can no longer pass when iOS does not revert the tap at all.

Co-Authored-By: GitHub Copilot Claude Opus 5.5 <223556219+Copilot@users.noreply.github.com>
)

Wait until the caret has left its offset, returned, and left again, or for at
most 1s, then assert only that it is in the tapped word. With the fix the test
finishes as soon as the correction lands (about 110ms on BrowserStack). If iOS
stops reverting the tap, the caret stays where iOS put it and the test still
passes, since that is the behavior #5981 asks for.

Co-Authored-By: GitHub Copilot Claude Opus 5.5 <223556219+Copilot@users.noreply.github.com>
…ndler

Co-Authored-By: GitHub Copilot Claude Opus 5.5 <223556219+Copilot@users.noreply.github.com>
@ethan-james

ethan-james commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator Author

If #6123 is merged, the revert correction will go into #6002 and the iOS 27-specific part can be discarded.

@trevinhofmann trevinhofmann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, pending the merge conflicts and #6123 / #6002 related changes. @ethan-james - LMK if you'd like another review if any meaningful changes are pushed based on those.

@raineorshine

Copy link
Copy Markdown
Contributor

Hi, thanks for the submission. I have spent some time looking at the proposed change.

I have some concerns about the risk that we would be taking on. This PR adds multiple effects, refs, timers, requestAnimationFrame, selectionchange listeners, out-of-dispatch direct store access, direct DOM access, persistent ministore state, and manual browser selection. It's simply too much complexity for what it does. I don't think I would be comfortable merging it as it stands.

Does the strategy change at all given that #6123 has been discontinued and we plan on waiting for the next Safari patch to fix the withheld touch in iOS27?

FYI I'm still seeing incorrect caret placement with this PR, though it appears slightly different than the original issue. Here the caret is set at the end of "eternally" after tapping at the end of "Existentialism". I'm not sure if this would be considered to share the same root cause or if this PR is meant to have a more limited scope.

IMG_0881

I'm going to put the original issue #5981 on hold since we have higher priority issues. I would still be interested in hearing any other thoughts you have on this. Thank you!

ethan-james added a commit that referenced this pull request Oct 27, 2026
A tap a few characters from the caret gets no mouse events, so onMouseDown
never places the caret, and after the touchend iOS moves the caret to the
tapped word and then back to where it was (#4220). useRevertCorrection, mounted
by useEditMode on the thought being edited, records the caret offset and the
touch point at the touchend, takes a selectionchange back to that offset after
a move away as the revert, and places the caret at getCaretOffset of the touch
point, dispatching setCursor so the Redux offset follows. The watch ends on the
next touchstart, mousedown, keydown or beforeinput.

This replaces #5983's revert correction. The case it was built for, a revert
before a touchend that iOS 27 withholds (#5981), is waiting on a Safari fix,
so the watch no longer spans the touch: it needs no touchStore state, no
listeners in initEvents and no TOUCH_SLOP check for drags.

Co-Authored-By: Claude claude-opus-5-5 (medium) <noreply@anthropic.com>
@ethan-james
ethan-james removed this pull request from stack #6112 October 27, 2026 22:23
@ethan-james

ethan-james commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator Author

Hi, thanks for the submission. I have spent some time looking at the proposed change.

I have some concerns about the risk that we would be taking on. This PR adds multiple effects, refs, timers, requestAnimationFrame, selectionchange listeners, out-of-dispatch direct store access, direct DOM access, persistent ministore state, and manual browser selection. It's simply too much complexity for what it does. I don't think I would be comfortable merging it as it stands.

Does the strategy change at all given that #6123 has been discontinued and we plan on waiting for the next Safari patch to fix the withheld touch in iOS27?

#5981 is specific to iOS 27, so everything in here is required to make it work there. This was also the base branch for #6002, which fixes #4220. Since we're not going forward with this PR, I moved the core architecture over into that branch and rebased it on main. That will run on all iOS versions and doesn't fix #5981, but it seems to be about half as complicated. That solution gets rid of the changes in initEvents and touchStore, as well as the extra stuff in useEditMode apart from the invocation of the new hook.

There's still one effect in useRevertCorrection, plus the selectionchange handler that is load-bearing for the caret corrections. That seems to be the only signal that iOS moved the caret. And selection.set is the only way to put the caret back after it was moved.

There are still a bunch of new event handlers to catch all of the events that end the revert window, rather than the arbitrary CARET_REVERT_TIMEOUT timer from #5665.

Next Steps

I could look into moving selection.set plus the dispatch out into useEditMode and see if they can be shared with the existing caret management code. That could result in simplification, or it could end up being more complicated.

requestAnimationFrame is still in there but I'm going to dig into whether it's necessary in the new version.

FYI I'm still seeing incorrect caret placement with this PR, though it appears slightly different than the original issue. Here the caret is set at the end of "eternally" after tapping at the end of "Existentialism". I'm not sure if this would be considered to share the same root cause or if this PR is meant to have a more limited scope.

I'll double check now that it's all living in the #4220 branch. That corrects a wider range of caret revert behavior than this PR. Since it's based around proximity to the caret, setting it at the end of "eternally" probably wouldn't be fixed by #6002. iOS behavior shouldn't be reverting the caret from more than about 40px away.

I tried to figure out if the 44px dead zone was connected to this, but Copilot didn't think that constant was likely to be used anywhere else. It would probably be in closed-source UIKit code rather than WebKit.

I'm going to put the original issue #5981 on hold since we have higher priority issues. I would still be interested in hearing any other thoughts you have on this. Thank you!

You're welcome! Let me know if #6002 seems like it's still too complex.

This branch was successfully deployed

1 active deployment
Preview — 10a4e723 Deployed Oct 27, 2026 by github-actions[bot]
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.

[iOS] Caret: a tap close to the caret moves it and then back on iOS 27

3 participants