Skip to content

Commit

Permalink
Rework button/buttons handling & add a new test checking for move eve…
Browse files Browse the repository at this point in the history
…nts 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
  • Loading branch information
vincentriemer authored and facebook-github-bot committed Sep 1, 2022
1 parent b098215 commit c89f5fd
Show file tree
Hide file tree
Showing 5 changed files with 274 additions and 93 deletions.
61 changes: 30 additions & 31 deletions React/Fabric/RCTSurfaceTouchHandler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -174,21 +174,36 @@ static CGFloat RadsToDegrees(CGFloat rads)

static int ButtonMaskToButtons(UIEventButtonMask buttonMask)
{
int buttonsMaskResult = 0;
if (@available(iOS 13.4, *)) {
return (((buttonMask & UIEventButtonMaskPrimary) > 0) ? 1 : 0) |
(((buttonMask & UIEventButtonMaskSecondary) > 0) ? 2 : 0);
if ((buttonMask & UIEventButtonMaskPrimary) != 0) {
buttonsMaskResult |= 1;
}
if ((buttonMask & UIEventButtonMaskSecondary) != 0) {
buttonsMaskResult |= 2;
}
// undocumented mask value which represents the "auxiliary button" (i.e. middle mouse button)
if ((buttonMask & 0x4) != 0) {
buttonsMaskResult |= 4;
}
}
return 0;
return buttonsMaskResult;
}

static int ButtonMaskToButton(UIEventButtonMask buttonMask)
static int ButtonMaskDiffToButton(UIEventButtonMask prevButtonMask, UIEventButtonMask curButtonMask)
{
if (@available(iOS 13.4, *)) {
if ((buttonMask & UIEventButtonMaskSecondary) > 0) {
if ((prevButtonMask & UIEventButtonMaskPrimary) != (curButtonMask & UIEventButtonMaskPrimary)) {
return 0;
}
if ((prevButtonMask & 0x4) != (curButtonMask & 0x4)) {
return 1;
}
if ((prevButtonMask & UIEventButtonMaskSecondary) != (curButtonMask & UIEventButtonMaskSecondary)) {
return 2;
}
}
return 0;
return -1;
}

static void UpdateActiveTouchWithUITouch(
Expand Down Expand Up @@ -220,9 +235,15 @@ static void UpdateActiveTouchWithUITouch(
activeTouch.altitudeAngle = uiTouch.altitudeAngle;
activeTouch.azimuthAngle = [uiTouch azimuthAngleInView:nil];
if (@available(iOS 13.4, *)) {
activeTouch.buttonMask = uiEvent.buttonMask;
UIEventButtonMask nextButtonMask = 0;
if (uiTouch.phase != UITouchPhaseEnded) {
nextButtonMask = uiTouch.type == UITouchTypeIndirectPointer ? uiEvent.buttonMask : 1;
}
activeTouch.button = ButtonMaskDiffToButton(activeTouch.buttonMask, nextButtonMask);
activeTouch.buttonMask = nextButtonMask;
activeTouch.modifierFlags = uiEvent.modifierFlags;
} else {
activeTouch.button = 0;
activeTouch.buttonMask = 0;
activeTouch.modifierFlags = 0;
}
Expand All @@ -245,7 +266,6 @@ static void UpdateActiveTouchWithUITouch(
}
componentView = componentView.superview;
}

UpdateActiveTouchWithUITouch(activeTouch, uiTouch, uiEvent, rootComponentView, rootViewOriginOffset);
return activeTouch;
}
Expand Down Expand Up @@ -344,29 +364,11 @@ static PointerEvent CreatePointerEventFromActiveTouch(ActiveTouch activeTouch, R

event.detail = 0;

event.button = -1;
if (eventType == RCTTouchEventTypeTouchStart || eventType == RCTTouchEventTypeTouchEnd) {
event.button = activeTouch.button;
}

event.buttons = 1;
if (@available(iOS 13.4, *)) {
if (activeTouch.touchType == UITouchTypeIndirectPointer) {
// Indirect pointers are the only situations where buttonMask is "accurate"
// so we override the assumed "left click" button value when those type of
// events are recieved
event.buttons = ButtonMaskToButtons(activeTouch.buttonMask);
}
}
event.button = activeTouch.button;
event.buttons = ButtonMaskToButtons(activeTouch.buttonMask);

UpdatePointerEventModifierFlags(event, activeTouch.modifierFlags);

// UIEvent's button mask for touch end events still marks the button as down
// so this ensures it's set to 0 as per the pointer event spec
if (eventType == RCTTouchEventTypeTouchEnd) {
event.buttons = 0;
}

event.tangentialPressure = 0.0;
event.twist = 0;
event.isPrimary = activeTouch.isPrimary;
Expand Down Expand Up @@ -557,15 +559,12 @@ - (void)_registerTouches:(NSSet<UITouch *> *)touches withEvent:(UIEvent *)event
}
break;
}

activeTouch.button = ButtonMaskToButton(event.buttonMask);
} else {
activeTouch.touch.identifier = _identifierPool.dequeue();
if (_primaryTouchPointerId == -1) {
_primaryTouchPointerId = activeTouch.touch.identifier;
activeTouch.isPrimary = true;
}
activeTouch.button = 0;
}

// If the pointer has not been marked as hovering over views before the touch started, we register
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export type PlatformTestResult = $ReadOnly<{|
export type PlatformTestContext = $ReadOnly<{
assert_true(a: boolean, description: string): void,
assert_equals(a: any, b: any, description: string): void,
assert_not_equals(a: any, b: any, description: string): void,
assert_greater_than_equal(a: number, b: number, description: string): void,
assert_less_than_equal(a: number, b: number, description: string): void,
}>;
Expand All @@ -48,6 +49,7 @@ export type PlatformTestCase = (context: PlatformTestContext) => void;

export type AsyncPlatformTest = $ReadOnly<{|
done(): void,
step(testcase: PlatformTestCase): void,
|}>;

export type SyncTestOptions = $ReadOnly<{|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,15 +35,21 @@ function constructAsyncTestHook(
$ReadOnly<{[string]: AsyncTestStatus}>,
) => $ReadOnly<{[string]: AsyncTestStatus}>,
) => void,
runTestCase: (
testCase: PlatformTestCase,
) => Array<PlatformTestAssertionResult>,
) {
return (description: string, timeoutMs?: number = 10000) => {
const assertionsRef = useRef([]);

const timeoutIDRef = useRef<TimeoutID | null>(null);

const timeoutHandler = useCallback(() => {
timeoutIDRef.current = null;
addTestResult({
name: description,
assertions: [
...assertionsRef.current,
{
passing: false,
name: 'async_timeout',
Expand Down Expand Up @@ -82,13 +88,16 @@ function constructAsyncTestHook(
addTestResult({
name: description,
assertions: [
...assertionsRef.current,
{
passing: true,
name: 'async_test',
description: 'async test should be completed',
},
],
status: 'PASS',
status: didAllAssertionsPass(assertionsRef.current)
? 'PASS'
: 'FAIL',
error: null,
});
return {...prev, [description]: 'COMPLETED'};
Expand All @@ -97,6 +106,11 @@ function constructAsyncTestHook(
});
}, [description]);

const stepHandler = useCallback(testCase => {
const stepAssertions = runTestCase(testCase);
assertionsRef.current.push(...stepAssertions);
}, []);

// test registration
useEffect(() => {
updateAsyncTestStatuses(prev => {
Expand All @@ -110,8 +124,9 @@ function constructAsyncTestHook(
return useMemo(
() => ({
done: completionHandler,
step: stepHandler,
}),
[completionHandler],
[completionHandler, stepHandler],
);
};
}
Expand Down Expand Up @@ -174,6 +189,74 @@ export default function usePlatformTestHarness(): PlatformTestHarnessHookResult
setTestElementKey(k => k + 1);
}, []);

const runTestCase = useCallback((testCase: PlatformTestCase) => {
const assertionResults: Array<PlatformTestAssertionResult> = [];

const baseAssert = (
assertionName: string,
testConditionResult: boolean,
description: string,
failureMessage: string,
) => {
if (testConditionResult) {
assertionResults.push({
passing: true,
name: assertionName,
description,
});
} else {
assertionResults.push({
passing: false,
name: assertionName,
description,
failureMessage,
});
}
};

const context: PlatformTestContext = {
assert_true: (cond: boolean, desc: string) =>
baseAssert(
'assert_true',
cond,
desc,
"expected 'true' but recieved 'false'",
),
assert_equals: (a: any, b: any, desc: string) =>
baseAssert(
'assert_equal',
a === b,
desc,
`expected ${a} to equal ${b}`,
),
assert_not_equals: (a: any, b: any, desc: string) =>
baseAssert(
'assert_not_equals',
a !== b,
desc,
`expected ${a} not to equal ${b}`,
),
assert_greater_than_equal: (a: number, b: number, desc: string) =>
baseAssert(
'assert_greater_than_equal',
a >= b,
desc,
`expected ${a} to be greater than or equal to ${b}`,
),
assert_less_than_equal: (a: number, b: number, desc: string) =>
baseAssert(
'assert_less_than_equal',
a <= b,
desc,
`expected ${a} to be less than or equal to ${b}`,
),
};

testCase(context);

return assertionResults;
}, []);

const testFunction: PlatformTestHarness['test'] = useCallback(
(
testCase: PlatformTestCase,
Expand All @@ -192,63 +275,8 @@ export default function usePlatformTestHarness(): PlatformTestHarnessHookResult
return;
}

const assertionResults: Array<PlatformTestAssertionResult> = [];

const baseAssert = (
assertionName: string,
testConditionResult: boolean,
description: string,
failureMessage: string,
) => {
if (testConditionResult) {
assertionResults.push({
passing: true,
name: assertionName,
description,
});
} else {
assertionResults.push({
passing: false,
name: assertionName,
description,
failureMessage,
});
}
};

const context: PlatformTestContext = {
assert_true: (cond: boolean, desc: string) =>
baseAssert(
'assert_true',
cond,
desc,
"expected 'true' but recieved 'false'",
),
assert_equals: (a: any, b: any, desc: string) =>
baseAssert(
'assert_equal',
a === b,
desc,
`expected ${a} to equal ${b}`,
),
assert_greater_than_equal: (a: number, b: number, desc: string) =>
baseAssert(
'assert_greater_than_equal',
a >= b,
desc,
`expected ${a} to be greater than or equal to ${b}`,
),
assert_less_than_equal: (a: number, b: number, desc: string) =>
baseAssert(
'assert_less_than_equal',
a <= b,
desc,
`expected ${a} to be less than or equal to ${b}`,
),
};

try {
testCase(context);
const assertionResults = runTestCase(testCase);
addTestResult({
name,
status: didAllAssertionsPass(assertionResults) ? 'PASS' : 'FAIL',
Expand All @@ -259,17 +287,22 @@ export default function usePlatformTestHarness(): PlatformTestHarnessHookResult
addTestResult({
name,
status: 'ERROR',
assertions: assertionResults,
assertions: [],
error,
});
}
},
[addTestResult],
[addTestResult, runTestCase],
);

const asyncTestHook: PlatformTestHarness['useAsyncTest'] = useMemo(
() => constructAsyncTestHook(addTestResult, updateAsyncTestStatuses),
[addTestResult],
() =>
constructAsyncTestHook(
addTestResult,
updateAsyncTestStatuses,
runTestCase,
),
[addTestResult, runTestCase],
);

const numPendingAsyncTests = useMemo(() => {
Expand Down
Loading

0 comments on commit c89f5fd

Please sign in to comment.