Skip to content

Commit

Permalink
Fix ghost leave/out events firing due to view recycling
Browse files Browse the repository at this point in the history
Summary:
Changelog: [iOS][Internal] - Fix ghost pointer leave/out events firing due to view recycling on iOS

While implementing the properties on the PointerEvent object on iOS I noticed that in certain specific scenarios I was seeing pointerLeave events being fired seemingly without corresponding pointerEvent events and even more strangely, when the pointer wasn't even close to the view in question.

After a lot of research I discovered that this was caused by an incompatibility between my strategy of keeping track/identifying views which are being hovered and RN's handling of creating/deleting views on iOS. See on iOS RN has the `RCTComponentViewRegistry` class which manages the creation & deletion of UIViews and adds an optimization of recycling views instead of outright deleting them.

This is causing issues with my tracking of which views are hovered because I compare the view's object references which, while accurate towards confirming equality of an underlying UIView — isn't accurate in confirming the equality of views from react's perspective.

This diff addresses this issue by adding a simple wrapper class that can be used around the UIViews which stores the view's react ID at initialization time ensuring it doesn't get updated even if the underlying view's react id is. As an additional precaution the wrapper class will also not return the view it's wrapping if their react tags do not match.

Reviewed By: lunaleaps

Differential Revision: D38546628

fbshipit-source-id: 8b732d52da0e61a5447001e8940e4439f49c6baf
  • Loading branch information
vincentriemer authored and facebook-github-bot committed Aug 10, 2022
1 parent 0149229 commit e532f86
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 23 deletions.
53 changes: 31 additions & 22 deletions React/Fabric/RCTSurfaceTouchHandler.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#import "RCTSurfaceTouchHandler.h"

#import <React/RCTReactTaggedView.h>
#import <React/RCTUtils.h>
#import <React/RCTViewComponentView.h>
#import <UIKit/UIGestureRecognizerSubclass.h>
Expand Down Expand Up @@ -237,12 +238,12 @@ static void UpdateActiveTouchWithUITouch(
return nil;
}

static NSOrderedSet *GetTouchableViewsInPathToRoot(UIView *componentView)
static NSOrderedSet<RCTReactTaggedView *> *GetTouchableViewsInPathToRoot(UIView *componentView)
{
NSMutableOrderedSet *results = [NSMutableOrderedSet orderedSet];
do {
if ([componentView respondsToSelector:@selector(touchEventEmitterAtPoint:)]) {
[results addObject:componentView];
[results addObject:[RCTReactTaggedView wrap:componentView]];
}
componentView = componentView.superview;
} while (componentView);
Expand Down Expand Up @@ -378,9 +379,10 @@ static BOOL AnyTouchesChanged(NSSet<UITouch *> *touches)
return NO;
}

static BOOL IsViewListeningToEvent(UIView *view, ViewEvents::Offset eventType)
static BOOL IsViewListeningToEvent(RCTReactTaggedView *taggedView, ViewEvents::Offset eventType)
{
if ([view.class conformsToProtocol:@protocol(RCTComponentViewProtocol)]) {
UIView *view = taggedView.view;
if (view && [view.class conformsToProtocol:@protocol(RCTComponentViewProtocol)]) {
auto props = ((id<RCTComponentViewProtocol>)view).props;
if (SharedViewProps viewProps = std::dynamic_pointer_cast<ViewProps const>(props)) {
return viewProps->events[eventType];
Expand All @@ -389,10 +391,10 @@ static BOOL IsViewListeningToEvent(UIView *view, ViewEvents::Offset eventType)
return NO;
}

static BOOL IsAnyViewInPathListeningToEvent(NSOrderedSet<UIView *> *viewPath, ViewEvents::Offset eventType)
static BOOL IsAnyViewInPathListeningToEvent(NSOrderedSet<RCTReactTaggedView *> *viewPath, ViewEvents::Offset eventType)
{
for (UIView *view in viewPath) {
if (IsViewListeningToEvent(view, eventType)) {
for (RCTReactTaggedView *taggedView in viewPath) {
if (IsViewListeningToEvent(taggedView, eventType)) {
return YES;
}
}
Expand Down Expand Up @@ -427,7 +429,7 @@ @implementation RCTSurfaceTouchHandler {
IdentifierPool<11> _identifierPool;

UIHoverGestureRecognizer *_hoverRecognizer API_AVAILABLE(ios(13.0));
NSOrderedSet *_currentlyHoveredViews;
NSOrderedSet<RCTReactTaggedView *> *_currentlyHoveredViews;

int _primaryTouchPointerId;
}
Expand Down Expand Up @@ -780,7 +782,10 @@ - (void)hovering:(UIHoverGestureRecognizer *)recognizer API_AVAILABLE(ios(13.0))

UIView *targetView = [listenerView hitTest:clientLocation withEvent:nil];
targetView = FindClosestFabricManagedTouchableView(targetView);
UIView *prevTargetView = [_currentlyHoveredViews firstObject];

RCTReactTaggedView *targetTaggedView = [RCTReactTaggedView wrap:targetView];
RCTReactTaggedView *prevTargetTaggedView = [_currentlyHoveredViews firstObject];
UIView *prevTargetView = prevTargetTaggedView.view;

CGPoint offsetLocation = [recognizer locationInView:targetView];

Expand All @@ -791,12 +796,12 @@ - (void)hovering:(UIHoverGestureRecognizer *)recognizer API_AVAILABLE(ios(13.0))
modifierFlags = 0;
}

NSOrderedSet *eventPathViews = GetTouchableViewsInPathToRoot(targetView);
NSOrderedSet<RCTReactTaggedView *> *eventPathViews = GetTouchableViewsInPathToRoot(targetView);

BOOL hasMoveListenerInEventPath = NO;

// Over
if (prevTargetView != targetView) {
if (prevTargetTaggedView.tag != targetTaggedView.tag) {
BOOL shouldEmitOverEvent = IsAnyViewInPathListeningToEvent(eventPathViews, ViewEvents::Offset::PointerOver);
SharedTouchEventEmitter eventEmitter = GetTouchEmitterFromView(targetView, [recognizer locationInView:targetView]);
if (shouldEmitOverEvent && eventEmitter != nil) {
Expand All @@ -815,11 +820,13 @@ - (void)hovering:(UIHoverGestureRecognizer *)recognizer API_AVAILABLE(ios(13.0))
// of events we send to JS
BOOL hasParentEnterListener = NO;

for (UIView *componentView in [eventPathViews reverseObjectEnumerator]) {
BOOL shouldEmitEvent =
hasParentEnterListener || IsViewListeningToEvent(componentView, ViewEvents::Offset::PointerEnter);
for (RCTReactTaggedView *taggedView in [eventPathViews reverseObjectEnumerator]) {
UIView *componentView = taggedView.view;

BOOL shouldEmitEvent = componentView != nil &&
(hasParentEnterListener || IsViewListeningToEvent(taggedView, ViewEvents::Offset::PointerEnter));

if (shouldEmitEvent && ![_currentlyHoveredViews containsObject:componentView]) {
if (shouldEmitEvent && ![_currentlyHoveredViews containsObject:taggedView]) {
SharedTouchEventEmitter eventEmitter =
GetTouchEmitterFromView(componentView, [recognizer locationInView:componentView]);
if (eventEmitter != nil) {
Expand All @@ -833,7 +840,7 @@ - (void)hovering:(UIHoverGestureRecognizer *)recognizer API_AVAILABLE(ios(13.0))
hasParentEnterListener = YES;
}

if (!hasMoveListenerInEventPath && IsViewListeningToEvent(componentView, ViewEvents::Offset::PointerMove)) {
if (!hasMoveListenerInEventPath && IsViewListeningToEvent(taggedView, ViewEvents::Offset::PointerMove)) {
hasMoveListenerInEventPath = YES;
}
}
Expand All @@ -849,7 +856,7 @@ - (void)hovering:(UIHoverGestureRecognizer *)recognizer API_AVAILABLE(ios(13.0))
}

// Out
if (prevTargetView != targetView) {
if (prevTargetView != nil && prevTargetTaggedView.tag != targetTaggedView.tag) {
BOOL shouldEmitOutEvent = IsAnyViewInPathListeningToEvent(_currentlyHoveredViews, ViewEvents::Offset::PointerOut);
SharedTouchEventEmitter eventEmitter =
GetTouchEmitterFromView(prevTargetView, [recognizer locationInView:prevTargetView]);
Expand All @@ -866,14 +873,16 @@ - (void)hovering:(UIHoverGestureRecognizer *)recognizer API_AVAILABLE(ios(13.0))
// we also need to efficiently keep track of if a view has a parent which is listening to the leave events,
// so we first iterate from the root to the target, collecting the views which need events fired for, of which
// we reverse iterate (now from target to root), actually emitting the events.
NSMutableOrderedSet *viewsToEmitLeaveEventsTo = [NSMutableOrderedSet orderedSet];
NSMutableOrderedSet<UIView *> *viewsToEmitLeaveEventsTo = [NSMutableOrderedSet orderedSet];

BOOL hasParentLeaveListener = NO;
for (UIView *componentView in [_currentlyHoveredViews reverseObjectEnumerator]) {
BOOL shouldEmitEvent =
hasParentLeaveListener || IsViewListeningToEvent(componentView, ViewEvents::Offset::PointerLeave);
for (RCTReactTaggedView *taggedView in [_currentlyHoveredViews reverseObjectEnumerator]) {
UIView *componentView = taggedView.view;

BOOL shouldEmitEvent = componentView != nil &&
(hasParentLeaveListener || IsViewListeningToEvent(taggedView, ViewEvents::Offset::PointerLeave));

if (shouldEmitEvent && ![eventPathViews containsObject:componentView]) {
if (shouldEmitEvent && ![eventPathViews containsObject:taggedView]) {
[viewsToEmitLeaveEventsTo addObject:componentView];
}

Expand Down
34 changes: 34 additions & 0 deletions React/Fabric/Utils/RCTReactTaggedView.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#import <UIKit/UIKit.h>

NS_ASSUME_NONNULL_BEGIN

/**
* Lightweight wrapper class around a UIView with a react tag which registers a
* constant react tag at initialization time for a stable hash and provides the
* udnerlying view to a caller if that underlying view's react tag has not
* changed from the one provided at initalization time (i.e. recycled).
*/
@interface RCTReactTaggedView : NSObject {
UIView *_view;
NSInteger _tag;
}

+ (RCTReactTaggedView *)wrap:(UIView *)view;

- (instancetype)initWithView:(UIView *)view;
- (nullable UIView *)view;
- (NSInteger)tag;

- (BOOL)isEqual:(id)other;
- (NSUInteger)hash;

@end

NS_ASSUME_NONNULL_END
55 changes: 55 additions & 0 deletions React/Fabric/Utils/RCTReactTaggedView.mm
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright (c) Meta Platforms, Inc. and affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

#import "RCTReactTaggedView.h"

@implementation RCTReactTaggedView

+ (RCTReactTaggedView *)wrap:(UIView *)view
{
return [[RCTReactTaggedView alloc] initWithView:view];
}

- (instancetype)initWithView:(UIView *)view
{
if (self = [super init]) {
_view = view;
_tag = view.tag;
}
return self;
}

- (nullable UIView *)view
{
if (_view.tag == _tag) {
return _view;
}
return nil;
}

- (NSInteger)tag
{
return _tag;
}

- (BOOL)isEqual:(id)other
{
if (other == self) {
return YES;
}
if (!other || ![other isKindOfClass:[self class]]) {
return NO;
}
return _tag == [other tag];
}

- (NSUInteger)hash
{
return _tag;
}

@end
2 changes: 1 addition & 1 deletion packages/rn-tester/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,7 @@ SPEC CHECKSUMS:
React-bridging: cc10a051eff1f03306a1d7659593d8aac3242bc3
React-callinvoker: 5f16202ad4e45f0607b1fae0f6955a8f7c87eef1
React-Codegen: 5adf19af97eb37a7d441c040521191e446255086
React-Core: 0cfb25c65d4dcb856b1807fe44a1ebe5e7ec9749
React-Core: ce4282fb714ffbe444b84d296d1728eaee4d0e9f
React-CoreModules: 675170bccf156da3a3348e04e2036ce401b2010d
React-cxxreact: 7276467c246302fedf598cc40d7003896ddb20ba
React-Fabric: abfd61dc5498ce165634af85d65fcc42b82b5bf4
Expand Down

0 comments on commit e532f86

Please sign in to comment.