fix(pointer): label a touch/pen contact as the primary button so JS press handling fires#16338
Open
FaithfulAudio wants to merge 1 commit into
Open
fix(pointer): label a touch/pen contact as the primary button so JS press handling fires#16338FaithfulAudio wants to merge 1 commit into
FaithfulAudio wants to merge 1 commit into
Conversation
onPointerPressed maps ActiveTouch.button exclusively from PointerUpdateKind, a mouse-only concept. A touch or pen contact matches no case, falls through to default: button = -1, and the derived W3C buttons bitmask becomes 0. The pointerdown delivered to JS therefore claims no button is pressed, so pointer-event-driven press handling discards finger contacts while identical mouse clicks work. Per W3C pointer-events a touch/pen contact IS the primary button: button 0, buttons 1. Set that after the switch when the mouse mapping left it negative. main counterpart of the button-labeling change in microsoft#16333 (0.83-stable), tracking microsoft#16332. Ports only that change: main already covers the tag == -1 release leak and the stale-pointer-reuse leak via microsoft#16048 (dispatching a synthesized touch Cancel), and cancels capture loss per pointer. microsoft#16333's cancel-all loop, IsPrimary purge and stale-touch backstop are deliberately not ported - they do not exist on main and their necessity there has not been assessed.
Contributor
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
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.
fix(pointer): label a touch/pen contact as the primary button so JS press handling fires
Tracks #16332. This is the
maincounterpart of the primary-button labeling change in #16333 (which targets0.83-stable).Problem
On real touch hardware, finger taps on
Pressable/TouchableOpacityare ignored by pointer-event-driven press machinery, while identical mouse clicks on the same element work.Root cause
In
vnext/Microsoft.ReactNative/Fabric/Composition/CompositionEventHandler.cpp,onPointerPressedmapsActiveTouch.buttonexclusively fromPointerUpdateKind— a mouse-only concept:A touch or pen contact has no mouse
PointerUpdateKind, so it falls through todefault:and getsbutton = -1; the derived W3Cbuttonsbitmask then becomes0. The dispatchedpointerdowntells JS "no button is pressed", so press logic driven by pointer events discards finger contacts. Per W3C pointer-events, a touch/pen contact is the primary button:button 0,buttons 1.Fix
After the existing switch: if the device is not a mouse and
activeTouch.buttonfell through to a negative value, setactiveTouch.button = 0(yieldingbuttons = 1while the contact is down). Mouse behaviour is unchanged, and the< 0guard leaves untouched any non-mouse contact that somehow carried a real button mapping.Baseline:
main@c69cf55f67f9b03f467502dac1007ac2d9ebe209. One hunk, +10 lines, no header or API change.Scope — what this PR deliberately does not port
mainalready addresses the other problems #16333 covers, by different means, so this PR ports only the button-labeling change:tag == -1release leak and the stale-pointer-reuse leak were fixed onmainby Fix touch event handling, improve reliability, and optimize performance #16048 (Fix touch event handling, improve reliability, and optimize performance, merged 2026-04-24). Notemaindispatches a synthesized touch Cancel for these, not a touch End.mainalso cancels capture loss per-pointer and hasonPointerRoutedAway.#16333 additionally contains a cancel-all-active-touches loop in
onPointerCaptureLost, anIsPrimaryself-heal purge, and a stale-touch time backstop. None of those exist onmain, and I have not assessed whether they are still needed there —main's at-source cleanups may subsume them. I have left them out rather than port hardening whose necessity I cannot demonstrate on this branch. Happy to do that assessment as a follow-up if it would be useful.Validation
This exact logic — identical condition and assignment, the only textual difference being that this PR spells the enum via the
Composition::Input::aliasmainalready uses in this function — is running in production as part of #16333 on RNW 0.83.2: compiled intoMicrosoft.ReactNativefrom source (UseExperimentalNuget=false), x64 Release, new-arch app (Facilitron FIT), on a physical Surface-class touch tablet. There, finger taps on press targets went from ignored to firing, matching mouse clicks. Validated with realPT_TOUCHinjection (InitializeTouchInjection/InjectTouchInput), not simulated mouse input — the mouse path does not exercise the touch pipeline at all.Diff verified against the pinned base: exactly one hunk, +10 lines, LF endings preserved, and the file's current tip is byte-identical to that base (no drift).
Not verified — please weigh before merging:
/azp runwould be appreciated.mainwith this change, nor exercised it on a device onmain. The behavioural evidence above comes from the 0.83.2 app.Composition::Input::alias used elsewhere in the function; the condition line is 117 columns, within this repo'sColumnLimit: 120) — by inspection, not by running the tools.main's surrounding code is materially identical to the 0.83 code this was validated on rests on readingmain'sCompositionEventHandler.cppat the baseline commit (same switch, samedefault: button = -1), not on differential testing onmain.Caveats for reviewers:
type: prereleaseto match the convention inmain's pending change files; happy to adjust.Microsoft Reviewers: Open in CodeFlow