Skip to content

Commit

Permalink
Removed duplicate key registration bug
Browse files Browse the repository at this point in the history
Summary:
@public

I was using UIKeyCommand as a key in a dictionary, but it seems iOS wasn't treating identical commands as equal, so it was possible to register the same key command twice, resulting in the command triggering the action multiple times.

I've now created a container object for the key commands, and not relying on undocumented hashing behavior of UIKeyCommand for deduplication any more.

Test Plan: Reload bridge multiple times, then check that the number of registered keys in the command set inside RCTKeyCommands doesn't keep increasing.
  • Loading branch information
nicklockwood committed Jun 19, 2015
1 parent caffd60 commit 634cdfb
Showing 1 changed file with 72 additions and 29 deletions.
101 changes: 72 additions & 29 deletions React/Base/RCTKeyCommands.m
Original file line number Diff line number Diff line change
Expand Up @@ -13,27 +13,74 @@

#import "RCTUtils.h"

@interface RCTKeyCommand : NSObject <NSCopying>

@property (nonatomic, strong) UIKeyCommand *keyCommand;
@property (nonatomic, copy) void (^block)(UIKeyCommand *);

@end

@implementation RCTKeyCommand

- (instancetype)initWithKeyCommand:(UIKeyCommand *)keyCommand
block:(void (^)(UIKeyCommand *))block
{
if ((self = [super init])) {
_keyCommand = keyCommand;
_block = block ?: ^(__unused UIKeyCommand *cmd) {};
}
return self;
}

RCT_NOT_IMPLEMENTED(-init)

- (id)copyWithZone:(__unused NSZone *)zone
{
return self;
}

- (NSUInteger)hash
{
return _keyCommand.input.hash ^ _keyCommand.modifierFlags;
}

- (BOOL)isEqual:(RCTKeyCommand *)object
{
if (![object isKindOfClass:[RCTKeyCommand class]]) {
return NO;
}
return [self matchesInput:object.keyCommand.input
flags:object.keyCommand.modifierFlags];
}

- (BOOL)matchesInput:(NSString *)input flags:(UIKeyModifierFlags)flags
{
return [_keyCommand.input isEqual:input] && _keyCommand.modifierFlags == flags;
}

@end

@interface RCTKeyCommands ()

@property (nonatomic, strong) NSMutableDictionary *commandBindings;
@property (nonatomic, strong) NSMutableSet *commands;

- (void)RCT_handleKeyCommand:(UIKeyCommand *)key;
- (BOOL)RCT_handleKeyCommand:(UIKeyCommand *)key;

@end

@implementation UIApplication (RCTKeyCommands)

- (NSArray *)RCT_keyCommands
{
NSDictionary *commandBindings = [RCTKeyCommands sharedInstance].commandBindings;
return [[self RCT_keyCommands] arrayByAddingObjectsFromArray:[commandBindings allKeys]];
NSSet *commands = [RCTKeyCommands sharedInstance].commands;
return [[self RCT_keyCommands] arrayByAddingObjectsFromArray:
[[commands valueForKeyPath:@"keyCommand"] allObjects]];
}

- (BOOL)RCT_sendAction:(SEL)action to:(id)target from:(id)sender forEvent:(UIEvent *)event
{
if (action == @selector(RCT_handleKeyCommand:)) {
[[RCTKeyCommands sharedInstance] RCT_handleKeyCommand:sender];
return YES;
return [[RCTKeyCommands sharedInstance] RCT_handleKeyCommand:sender];
}
return [self RCT_sendAction:action to:target from:sender forEvent:event];
}
Expand All @@ -49,8 +96,6 @@ + (void)initialize
RCTSwapInstanceMethods([UIApplication class], @selector(sendAction:to:from:forEvent:), @selector(RCT_sendAction:to:from:forEvent:));
}

static RCTKeyCommands *RKKeyCommandsSharedInstance = nil;

+ (instancetype)sharedInstance
{
static RCTKeyCommands *sharedInstance;
Expand All @@ -65,25 +110,11 @@ + (instancetype)sharedInstance
- (instancetype)init
{
if ((self = [super init])) {
_commandBindings = [[NSMutableDictionary alloc] init];
_commands = [[NSMutableSet alloc] init];
}
return self;
}

- (void)RCT_handleKeyCommand:(UIKeyCommand *)key
{
// NOTE: We should just be able to do commandBindings[key] here, but curiously, the
// lookup seems to return nil sometimes, even if the key is found in the dictionary.
// To fix this, we use a linear search, since there won't be many keys anyway

[_commandBindings enumerateKeysAndObjectsUsingBlock:
^(UIKeyCommand *k, void (^block)(UIKeyCommand *), __unused BOOL *stop) {
if ([key.input isEqualToString:k.input] && key.modifierFlags == k.modifierFlags) {
block(key);
}
}];
}

- (void)registerKeyCommandWithInput:(NSString *)input
modifierFlags:(UIKeyModifierFlags)flags
action:(void (^)(UIKeyCommand *))block
Expand All @@ -105,17 +136,29 @@ - (void)registerKeyCommandWithInput:(NSString *)input
modifierFlags:flags
action:@selector(RCT_handleKeyCommand:)];

_commandBindings[command] = block ?: ^(__unused UIKeyCommand *cmd) {};
[_commands addObject:[[RCTKeyCommand alloc] initWithKeyCommand:command block:block]];
}

- (BOOL)RCT_handleKeyCommand:(UIKeyCommand *)key
{
for (RCTKeyCommand *command in [RCTKeyCommands sharedInstance].commands) {
if ([command.keyCommand.input isEqualToString:key.input] &&
command.keyCommand.modifierFlags == key.modifierFlags) {
command.block(key);
return YES;
}
}
return NO;
}

- (void)unregisterKeyCommandWithInput:(NSString *)input
modifierFlags:(UIKeyModifierFlags)flags
{
RCTAssertMainThread();

for (UIKeyCommand *key in [_commandBindings allKeys]) {
if ([key.input isEqualToString:input] && key.modifierFlags == flags) {
[_commandBindings removeObjectForKey:key];
for (RCTKeyCommand *command in _commands.allObjects) {
if ([command matchesInput:input flags:flags]) {
[_commands removeObject:command];
break;
}
}
Expand All @@ -126,8 +169,8 @@ - (BOOL)isKeyCommandRegisteredForInput:(NSString *)input
{
RCTAssertMainThread();

for (UIKeyCommand *key in [_commandBindings allKeys]) {
if ([key.input isEqualToString:input] && key.modifierFlags == flags) {
for (RCTKeyCommand *command in _commands) {
if ([command matchesInput:input flags:flags]) {
return YES;
}
}
Expand Down

0 comments on commit 634cdfb

Please sign in to comment.