Skip to content

Commit c89f5fd

Browse files
vincentriemerfacebook-github-bot
authored andcommitted
Rework button/buttons handling & add a new test checking for move events on chorded mouse button presses
Summary: Changelog: [iOS][Internal] - Rework button/buttons handling on pointer events & add a new test checking for move events on chorded mouse button presses This is a larger diff that largely stems from implementing a new pointer event platform test about chorded mouse button presses (adapted from https://github.com/web-platform-tests/wpt/blob/master/pointerevents/pointerevent_pointermove_on_chorded_mouse_button.html). In order to implement this test I added a new method, `step`, to an async test instance to allow you to run ad-hoc assertions labeled by the async test (like it's done in the original web platform test). Once implementing the test I also discovered my current logic for handling the `button` and `buttons` properties was insufficient so I reworked it to handle more/most cases by making the `button` property instead determined by the change in the `buttons` property. This largely works but unfortunately due to what seems to be a bug in UIKit, chorded pointer "up" events receive the wrong button mask values so it's currently impossible to get this test fully passing on iOS. I've since submitted a bug report to Apple but my hopes aren't high that it will be focused on since it's such a niche issue. Reviewed By: lunaleaps Differential Revision: D38951159 fbshipit-source-id: 426b7cf7930ed8329151382fafee487493e568de
1 parent b098215 commit c89f5fd

File tree

5 files changed

+274
-93
lines changed

5 files changed

+274
-93
lines changed

React/Fabric/RCTSurfaceTouchHandler.mm

Lines changed: 30 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -174,21 +174,36 @@ static CGFloat RadsToDegrees(CGFloat rads)
174174

175175
static int ButtonMaskToButtons(UIEventButtonMask buttonMask)
176176
{
177+
int buttonsMaskResult = 0;
177178
if (@available(iOS 13.4, *)) {
178-
return (((buttonMask & UIEventButtonMaskPrimary) > 0) ? 1 : 0) |
179-
(((buttonMask & UIEventButtonMaskSecondary) > 0) ? 2 : 0);
179+
if ((buttonMask & UIEventButtonMaskPrimary) != 0) {
180+
buttonsMaskResult |= 1;
181+
}
182+
if ((buttonMask & UIEventButtonMaskSecondary) != 0) {
183+
buttonsMaskResult |= 2;
184+
}
185+
// undocumented mask value which represents the "auxiliary button" (i.e. middle mouse button)
186+
if ((buttonMask & 0x4) != 0) {
187+
buttonsMaskResult |= 4;
188+
}
180189
}
181-
return 0;
190+
return buttonsMaskResult;
182191
}
183192

184-
static int ButtonMaskToButton(UIEventButtonMask buttonMask)
193+
static int ButtonMaskDiffToButton(UIEventButtonMask prevButtonMask, UIEventButtonMask curButtonMask)
185194
{
186195
if (@available(iOS 13.4, *)) {
187-
if ((buttonMask & UIEventButtonMaskSecondary) > 0) {
196+
if ((prevButtonMask & UIEventButtonMaskPrimary) != (curButtonMask & UIEventButtonMaskPrimary)) {
197+
return 0;
198+
}
199+
if ((prevButtonMask & 0x4) != (curButtonMask & 0x4)) {
200+
return 1;
201+
}
202+
if ((prevButtonMask & UIEventButtonMaskSecondary) != (curButtonMask & UIEventButtonMaskSecondary)) {
188203
return 2;
189204
}
190205
}
191-
return 0;
206+
return -1;
192207
}
193208

194209
static void UpdateActiveTouchWithUITouch(
@@ -220,9 +235,15 @@ static void UpdateActiveTouchWithUITouch(
220235
activeTouch.altitudeAngle = uiTouch.altitudeAngle;
221236
activeTouch.azimuthAngle = [uiTouch azimuthAngleInView:nil];
222237
if (@available(iOS 13.4, *)) {
223-
activeTouch.buttonMask = uiEvent.buttonMask;
238+
UIEventButtonMask nextButtonMask = 0;
239+
if (uiTouch.phase != UITouchPhaseEnded) {
240+
nextButtonMask = uiTouch.type == UITouchTypeIndirectPointer ? uiEvent.buttonMask : 1;
241+
}
242+
activeTouch.button = ButtonMaskDiffToButton(activeTouch.buttonMask, nextButtonMask);
243+
activeTouch.buttonMask = nextButtonMask;
224244
activeTouch.modifierFlags = uiEvent.modifierFlags;
225245
} else {
246+
activeTouch.button = 0;
226247
activeTouch.buttonMask = 0;
227248
activeTouch.modifierFlags = 0;
228249
}
@@ -245,7 +266,6 @@ static void UpdateActiveTouchWithUITouch(
245266
}
246267
componentView = componentView.superview;
247268
}
248-
249269
UpdateActiveTouchWithUITouch(activeTouch, uiTouch, uiEvent, rootComponentView, rootViewOriginOffset);
250270
return activeTouch;
251271
}
@@ -344,29 +364,11 @@ static PointerEvent CreatePointerEventFromActiveTouch(ActiveTouch activeTouch, R
344364

345365
event.detail = 0;
346366

347-
event.button = -1;
348-
if (eventType == RCTTouchEventTypeTouchStart || eventType == RCTTouchEventTypeTouchEnd) {
349-
event.button = activeTouch.button;
350-
}
351-
352-
event.buttons = 1;
353-
if (@available(iOS 13.4, *)) {
354-
if (activeTouch.touchType == UITouchTypeIndirectPointer) {
355-
// Indirect pointers are the only situations where buttonMask is "accurate"
356-
// so we override the assumed "left click" button value when those type of
357-
// events are recieved
358-
event.buttons = ButtonMaskToButtons(activeTouch.buttonMask);
359-
}
360-
}
367+
event.button = activeTouch.button;
368+
event.buttons = ButtonMaskToButtons(activeTouch.buttonMask);
361369

362370
UpdatePointerEventModifierFlags(event, activeTouch.modifierFlags);
363371

364-
// UIEvent's button mask for touch end events still marks the button as down
365-
// so this ensures it's set to 0 as per the pointer event spec
366-
if (eventType == RCTTouchEventTypeTouchEnd) {
367-
event.buttons = 0;
368-
}
369-
370372
event.tangentialPressure = 0.0;
371373
event.twist = 0;
372374
event.isPrimary = activeTouch.isPrimary;
@@ -557,15 +559,12 @@ - (void)_registerTouches:(NSSet<UITouch *> *)touches withEvent:(UIEvent *)event
557559
}
558560
break;
559561
}
560-
561-
activeTouch.button = ButtonMaskToButton(event.buttonMask);
562562
} else {
563563
activeTouch.touch.identifier = _identifierPool.dequeue();
564564
if (_primaryTouchPointerId == -1) {
565565
_primaryTouchPointerId = activeTouch.touch.identifier;
566566
activeTouch.isPrimary = true;
567567
}
568-
activeTouch.button = 0;
569568
}
570569

571570
// If the pointer has not been marked as hovering over views before the touch started, we register

packages/rn-tester/js/examples/Experimental/PlatformTest/RNTesterPlatformTestTypes.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export type PlatformTestResult = $ReadOnly<{|
4040
export type PlatformTestContext = $ReadOnly<{
4141
assert_true(a: boolean, description: string): void,
4242
assert_equals(a: any, b: any, description: string): void,
43+
assert_not_equals(a: any, b: any, description: string): void,
4344
assert_greater_than_equal(a: number, b: number, description: string): void,
4445
assert_less_than_equal(a: number, b: number, description: string): void,
4546
}>;
@@ -48,6 +49,7 @@ export type PlatformTestCase = (context: PlatformTestContext) => void;
4849

4950
export type AsyncPlatformTest = $ReadOnly<{|
5051
done(): void,
52+
step(testcase: PlatformTestCase): void,
5153
|}>;
5254

5355
export type SyncTestOptions = $ReadOnly<{|

packages/rn-tester/js/examples/Experimental/PlatformTest/usePlatformTestHarness.js

Lines changed: 95 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,21 @@ function constructAsyncTestHook(
3535
$ReadOnly<{[string]: AsyncTestStatus}>,
3636
) => $ReadOnly<{[string]: AsyncTestStatus}>,
3737
) => void,
38+
runTestCase: (
39+
testCase: PlatformTestCase,
40+
) => Array<PlatformTestAssertionResult>,
3841
) {
3942
return (description: string, timeoutMs?: number = 10000) => {
43+
const assertionsRef = useRef([]);
44+
4045
const timeoutIDRef = useRef<TimeoutID | null>(null);
4146

4247
const timeoutHandler = useCallback(() => {
4348
timeoutIDRef.current = null;
4449
addTestResult({
4550
name: description,
4651
assertions: [
52+
...assertionsRef.current,
4753
{
4854
passing: false,
4955
name: 'async_timeout',
@@ -82,13 +88,16 @@ function constructAsyncTestHook(
8288
addTestResult({
8389
name: description,
8490
assertions: [
91+
...assertionsRef.current,
8592
{
8693
passing: true,
8794
name: 'async_test',
8895
description: 'async test should be completed',
8996
},
9097
],
91-
status: 'PASS',
98+
status: didAllAssertionsPass(assertionsRef.current)
99+
? 'PASS'
100+
: 'FAIL',
92101
error: null,
93102
});
94103
return {...prev, [description]: 'COMPLETED'};
@@ -97,6 +106,11 @@ function constructAsyncTestHook(
97106
});
98107
}, [description]);
99108

109+
const stepHandler = useCallback(testCase => {
110+
const stepAssertions = runTestCase(testCase);
111+
assertionsRef.current.push(...stepAssertions);
112+
}, []);
113+
100114
// test registration
101115
useEffect(() => {
102116
updateAsyncTestStatuses(prev => {
@@ -110,8 +124,9 @@ function constructAsyncTestHook(
110124
return useMemo(
111125
() => ({
112126
done: completionHandler,
127+
step: stepHandler,
113128
}),
114-
[completionHandler],
129+
[completionHandler, stepHandler],
115130
);
116131
};
117132
}
@@ -174,6 +189,74 @@ export default function usePlatformTestHarness(): PlatformTestHarnessHookResult
174189
setTestElementKey(k => k + 1);
175190
}, []);
176191

192+
const runTestCase = useCallback((testCase: PlatformTestCase) => {
193+
const assertionResults: Array<PlatformTestAssertionResult> = [];
194+
195+
const baseAssert = (
196+
assertionName: string,
197+
testConditionResult: boolean,
198+
description: string,
199+
failureMessage: string,
200+
) => {
201+
if (testConditionResult) {
202+
assertionResults.push({
203+
passing: true,
204+
name: assertionName,
205+
description,
206+
});
207+
} else {
208+
assertionResults.push({
209+
passing: false,
210+
name: assertionName,
211+
description,
212+
failureMessage,
213+
});
214+
}
215+
};
216+
217+
const context: PlatformTestContext = {
218+
assert_true: (cond: boolean, desc: string) =>
219+
baseAssert(
220+
'assert_true',
221+
cond,
222+
desc,
223+
"expected 'true' but recieved 'false'",
224+
),
225+
assert_equals: (a: any, b: any, desc: string) =>
226+
baseAssert(
227+
'assert_equal',
228+
a === b,
229+
desc,
230+
`expected ${a} to equal ${b}`,
231+
),
232+
assert_not_equals: (a: any, b: any, desc: string) =>
233+
baseAssert(
234+
'assert_not_equals',
235+
a !== b,
236+
desc,
237+
`expected ${a} not to equal ${b}`,
238+
),
239+
assert_greater_than_equal: (a: number, b: number, desc: string) =>
240+
baseAssert(
241+
'assert_greater_than_equal',
242+
a >= b,
243+
desc,
244+
`expected ${a} to be greater than or equal to ${b}`,
245+
),
246+
assert_less_than_equal: (a: number, b: number, desc: string) =>
247+
baseAssert(
248+
'assert_less_than_equal',
249+
a <= b,
250+
desc,
251+
`expected ${a} to be less than or equal to ${b}`,
252+
),
253+
};
254+
255+
testCase(context);
256+
257+
return assertionResults;
258+
}, []);
259+
177260
const testFunction: PlatformTestHarness['test'] = useCallback(
178261
(
179262
testCase: PlatformTestCase,
@@ -192,63 +275,8 @@ export default function usePlatformTestHarness(): PlatformTestHarnessHookResult
192275
return;
193276
}
194277

195-
const assertionResults: Array<PlatformTestAssertionResult> = [];
196-
197-
const baseAssert = (
198-
assertionName: string,
199-
testConditionResult: boolean,
200-
description: string,
201-
failureMessage: string,
202-
) => {
203-
if (testConditionResult) {
204-
assertionResults.push({
205-
passing: true,
206-
name: assertionName,
207-
description,
208-
});
209-
} else {
210-
assertionResults.push({
211-
passing: false,
212-
name: assertionName,
213-
description,
214-
failureMessage,
215-
});
216-
}
217-
};
218-
219-
const context: PlatformTestContext = {
220-
assert_true: (cond: boolean, desc: string) =>
221-
baseAssert(
222-
'assert_true',
223-
cond,
224-
desc,
225-
"expected 'true' but recieved 'false'",
226-
),
227-
assert_equals: (a: any, b: any, desc: string) =>
228-
baseAssert(
229-
'assert_equal',
230-
a === b,
231-
desc,
232-
`expected ${a} to equal ${b}`,
233-
),
234-
assert_greater_than_equal: (a: number, b: number, desc: string) =>
235-
baseAssert(
236-
'assert_greater_than_equal',
237-
a >= b,
238-
desc,
239-
`expected ${a} to be greater than or equal to ${b}`,
240-
),
241-
assert_less_than_equal: (a: number, b: number, desc: string) =>
242-
baseAssert(
243-
'assert_less_than_equal',
244-
a <= b,
245-
desc,
246-
`expected ${a} to be less than or equal to ${b}`,
247-
),
248-
};
249-
250278
try {
251-
testCase(context);
279+
const assertionResults = runTestCase(testCase);
252280
addTestResult({
253281
name,
254282
status: didAllAssertionsPass(assertionResults) ? 'PASS' : 'FAIL',
@@ -259,17 +287,22 @@ export default function usePlatformTestHarness(): PlatformTestHarnessHookResult
259287
addTestResult({
260288
name,
261289
status: 'ERROR',
262-
assertions: assertionResults,
290+
assertions: [],
263291
error,
264292
});
265293
}
266294
},
267-
[addTestResult],
295+
[addTestResult, runTestCase],
268296
);
269297

270298
const asyncTestHook: PlatformTestHarness['useAsyncTest'] = useMemo(
271-
() => constructAsyncTestHook(addTestResult, updateAsyncTestStatuses),
272-
[addTestResult],
299+
() =>
300+
constructAsyncTestHook(
301+
addTestResult,
302+
updateAsyncTestStatuses,
303+
runTestCase,
304+
),
305+
[addTestResult, runTestCase],
273306
);
274307

275308
const numPendingAsyncTests = useMemo(() => {

0 commit comments

Comments
 (0)