Improved the phone number editing during registration

- Fixed a crash where an offset wrapped around when deleting the opening bracket
- Backspacing now skips over formatting characters
- Cursor position is maintained more accurately when reformatting
- Added a few utility methods
- Also fixed a test not having "test" as a prefix, causing it not to run
//FREEBIE

Signal/src/util/PhoneNumberUtil.h

@@ -6,4 +6,10 @@
 + (NSString *)countryNameFromCountryCode:(NSString *)code;
 + (NSArray *)countryCodesForSearchTerm:(NSString *)searchTerm;
 + (NSString*) normalizePhoneNumber:(NSString *) number;
+
++(NSUInteger) translateCursorPosition:(NSUInteger)offset
+                                 from:(NSString*)source
+                                   to:(NSString*)target
+                    stickingRightward:(bool)preferHigh;
+
 @end

Signal/src/util/PhoneNumberUtil.m

@@ -1,7 +1,7 @@
 #import "PhoneNumberUtil.h"
 #import "ContactsManager.h"
-#import "FunctionalUtil.h"
 #import "NBPhoneNumberUtil.h"
+#import "Util.h"
 
 @implementation PhoneNumberUtil
 
@@ -36,4 +36,64 @@
     return [NBPhoneNumberUtil.sharedInstance normalizePhoneNumber:number];
 }
 
++(NSUInteger) translateCursorPosition:(NSUInteger)offset
+                                 from:(NSString*)source
+                                   to:(NSString*)target
+                    stickingRightward:(bool)preferHigh {
+    require(source != nil);
+    require(target != nil);
+    require(offset <= source.length);
+    
+    NSUInteger n = source.length;
+    NSUInteger m = target.length;
+    
+    int moves[n+1][m+1];
+    {
+        // Wagner-Fischer algorithm for computing edit distance, with a tweaks:
+        // - Tracks best moves at each location, to allow reconstruction of edit path
+        // - Does not allow substitutions
+        // - Over-values digits relative to other characters, so they're "harder" to delete or insert
+        const int DIGIT_VALUE = 10;
+        NSUInteger scores[n+1][m+1];
+        moves[0][0] = 0; // (match) move up and left
+        scores[0][0] = 0;
+        for (NSUInteger i = 1; i <= n; i++) {
+            scores[i][0] = i;
+            moves[i][0] = -1; // (deletion) move left
+        }
+        for (NSUInteger j = 1; j <= m; j++) {
+            scores[0][j] = j;
+            moves[0][j] = +1; // (insertion) move up
+        }
+        
+        NSCharacterSet* digits = NSCharacterSet.decimalDigitCharacterSet;
+        for (NSUInteger i = 1; i <= n; i++) {
+            unichar c1 = [source characterAtIndex:i-1];
+            bool isDigit1 = [digits characterIsMember:c1];
+            for (NSUInteger j = 1; j <= m; j++) {
+                unichar c2 = [target characterAtIndex:j-1];
+                bool isDigit2 = [digits characterIsMember:c2];
+                if (c1 == c2) {
+                    scores[i][j] = scores[i-1][j-1];
+                    moves[i][j] = 0; // move up-and-left
+                } else {
+                    NSUInteger del = scores[i-1][j] + (isDigit1 ? DIGIT_VALUE : 1);
+                    NSUInteger ins = scores[i][j-1] + (isDigit2 ? DIGIT_VALUE : 1);
+                    bool isDel = del < ins;
+                    scores[i][j] = isDel ? del : ins;
+                    moves[i][j] = isDel ? -1 : +1;
+                }
+            }
+        }
+    }
+    
+    // Backtrack to find desired corresponding offset
+    for (NSUInteger i = n, j = m; ; i -= 1) {
+        if (i == offset && preferHigh) return j; // early exit
+        while (moves[i][j] == +1) j -= 1; // zip upward
+        if (i == offset) return j; // late exit
+        if (moves[i][j] == 0) j -= 1;
+    }
+}
+
 @end

Signal/src/util/StringUtil.h

@@ -1,6 +1,7 @@
 #import <Foundation/Foundation.h>
 
 @interface NSString (Util)
+
 /// The utf-8 encoding of the string's text.
 -(NSData*) encodedAsUtf8;
 /// The ascii encoding of the string's text.
@@ -17,5 +18,8 @@
 -(NSData*) decodedAsBase64Data;
 -(NSNumber*) tryParseAsDecimalNumber;
 -(NSNumber*) tryParseAsUnsignedInteger;
+-(NSString*) removeAllCharactersIn:(NSCharacterSet*)characterSet;
+-(NSString*) digitsOnly;
+-(NSString*) withCharactersInRange:(NSRange)range replacedBy:(NSString*)substring;
 
 @end

Signal/src/util/StringUtil.m

@@ -140,5 +140,17 @@
     NSNumber* value = [self tryParseAsDecimalNumber];
     return value.hasUnsignedIntegerValue ? value : nil;
 }
+-(NSString*) removeAllCharactersIn:(NSCharacterSet*)characterSet {
+    require(characterSet != nil);
+    return [[self componentsSeparatedByCharactersInSet:characterSet] componentsJoinedByString:@""];
+}
+-(NSString*) digitsOnly {
+    return [self removeAllCharactersIn:[NSCharacterSet.decimalDigitCharacterSet invertedSet]];
+}
+-(NSString*) withCharactersInRange:(NSRange)range replacedBy:(NSString*)substring {
+    NSMutableString* result = self.mutableCopy;
+    [result replaceCharactersInRange:range withString:substring];
+    return result;
+}
 
 @end

Signal/src/view controllers/RegisterViewController.m

@@ -27,7 +27,6 @@
 #define IPHONE_BLUE [UIColor colorWithRed:22 green:173 blue:214 alpha:1]
 
 @interface RegisterViewController () {
-    NSMutableString *_enteredPhoneNumber;
     NSTimer* countdownTimer;
     NSDate *timeoutDate;
 }
@@ -51,7 +50,6 @@
 
     [self initializeKeyboardHandlers];
     [self setPlaceholderTextColor:[UIColor lightGrayColor]];
-    _enteredPhoneNumber = [NSMutableString string];
 }
 
 + (RegisterViewController*)registerViewController {
@@ -360,7 +358,16 @@
                        forCountry:(NSString *)country {
     _countryCodeLabel.text = code;
     _countryNameLabel.text = country;
-    [self updatePhoneNumberFieldWithString:code cursorposition:_enteredPhoneNumber.length];
+    
+    // Reformat phone number
+    NSString* digits = _phoneNumberTextField.text.digitsOnly;
+    NSString* reformattedNumber = [PhoneNumber bestEffortFormatPartialUserSpecifiedTextToLookLikeAPhoneNumber:digits
+                                                                               withSpecifiedCountryCodeString:_countryCodeLabel.text];
+    _phoneNumberTextField.text = reformattedNumber;
+    UITextPosition *pos = _phoneNumberTextField.endOfDocument;
+    [_phoneNumberTextField setSelectedTextRange:[_phoneNumberTextField textRangeFromPosition:pos toPosition:pos]];
+    
+    // Done choosing country
     [vc dismissViewControllerAnimated:YES completion:nil];
 }
 
@@ -370,61 +377,41 @@
 
 #pragma mark - UITextFieldDelegate
 
-- (NSUInteger) calculateLocationOffset:(NSUInteger)location {
+-(BOOL)textField:(UITextField *)textField shouldChangeCharactersInRange:(NSRange)range replacementString:(NSString *)string {
-    NSUInteger offset = 0, phonenumberposition = 0;
+    NSString* textBeforeChange = textField.text;
-    for (NSUInteger i = 0; i < location; i++) {
+
-        if ([_phoneNumberTextField.text characterAtIndex:i] != [_enteredPhoneNumber characterAtIndex:phonenumberposition]) {
+    // backspacing should skip over formatting characters
-            offset++;
+    bool isBackspace = string.length == 0 && range.length == 1;
-        } else {
+    if (isBackspace) {
-            phonenumberposition++;
+        NSString* digits = _phoneNumberTextField.text.digitsOnly;
+        NSUInteger correspondingDeletePosition = [PhoneNumberUtil translateCursorPosition:range.location+range.length
+                                                                                     from:textBeforeChange
+                                                                                       to:digits
+                                                                        stickingRightward:true];
+        if (correspondingDeletePosition > 0) {
+            textBeforeChange = digits;
+            range = NSMakeRange(correspondingDeletePosition - 1, 1);
         }
     }
-    return offset;
+    
-}
+    // make the proposed change
-
+    NSString* textAfterChange = [textBeforeChange withCharactersInRange:range replacedBy:string];
-- (BOOL)textField:(UITextField *)textField shouldChangeCharactersInRange:(NSRange)range
+    NSUInteger cursorPositionAfterChange = range.location + string.length;
-                                                       replacementString:(NSString *)string {
+
-    NSUInteger offset = [self calculateLocationOffset:range.location];
+    // reformat the phone number, trying to keep the cursor beside the inserted or deleted digit
-    unichar currentPoschar = 0;
+    bool isJustDeletion = string.length == 0;
-    range.location -= offset;
+    NSString* textAfterReformat = [PhoneNumber bestEffortFormatPartialUserSpecifiedTextToLookLikeAPhoneNumber:textAfterChange.digitsOnly
-    BOOL handleBackspace = range.length == 1;
+                                                                               withSpecifiedCountryCodeString:_countryCodeLabel.text];
-    if (handleBackspace) {
+    NSUInteger cursorPositionAfterReformat = [PhoneNumberUtil translateCursorPosition:cursorPositionAfterChange
-        if ((range.location + offset) <  _phoneNumberTextField.text.length) {
+                                                                                 from:textAfterChange
-            currentPoschar = [_phoneNumberTextField.text characterAtIndex:(range.location + offset)];
+                                                                                   to:textAfterReformat
-        }
+                                                                    stickingRightward:isJustDeletion];
-        if ((currentPoschar < '0' || currentPoschar > '9') && currentPoschar != 0) {
+    _phoneNumberTextField.text = textAfterReformat;
-            range.location--;
+    UITextPosition *pos = [_phoneNumberTextField positionFromPosition:_phoneNumberTextField.beginningOfDocument
-        }
+                                                               offset:(NSInteger)cursorPositionAfterReformat];
-        NSRange backspaceRange = NSMakeRange(range.location, 1);
+    [_phoneNumberTextField setSelectedTextRange:[_phoneNumberTextField textRangeFromPosition:pos toPosition:pos]];
-        [_enteredPhoneNumber replaceCharactersInRange:backspaceRange withString:string];
+
-        range.location++;
+    return NO; // inform our caller that we took care of performing the change
-    } else {
-        NSString* sanitizedString = [[string componentsSeparatedByCharactersInSet:[[NSCharacterSet decimalDigitCharacterSet ] invertedSet]] componentsJoinedByString:@""];
-        NSMutableString* mutablePhoneNumber = [NSMutableString stringWithString:_enteredPhoneNumber];
-        [mutablePhoneNumber insertString:sanitizedString atIndex:range.location];
-        _enteredPhoneNumber = mutablePhoneNumber;
-        if ((range.location + offset + 1) <  _phoneNumberTextField.text.length) {
-            currentPoschar = [_phoneNumberTextField.text characterAtIndex:(range.location + offset + 1)];
-        }
-        if ((currentPoschar < '0' || currentPoschar > '9') && currentPoschar != 0) {
-            range.location++;
-        }
-    }
-
-    [self updatePhoneNumberFieldWithString:_enteredPhoneNumber cursorposition:range.location+offset];
-    return NO;
-}
-
--(void) updatePhoneNumberFieldWithString:(NSString*)input
-                          cursorposition:(NSUInteger)cursorpos {
-    NSString* result = [PhoneNumber bestEffortFormatPartialUserSpecifiedTextToLookLikeAPhoneNumber:_enteredPhoneNumber
-                                                                    withSpecifiedCountryCodeString:_countryCodeLabel.text];
-    cursorpos += result.length - _phoneNumberTextField.text.length;
-    _phoneNumberTextField.text = result;
-    UITextPosition *position = [_phoneNumberTextField positionFromPosition:[_phoneNumberTextField beginningOfDocument]
-                                                                    offset:(NSInteger)cursorpos];
-    [_phoneNumberTextField setSelectedTextRange:[_phoneNumberTextField textRangeFromPosition:position
-                                                                                  toPosition:position]];
 }
 
 @end

Signal/test/phone/PhoneNumberTest.m

@@ -1,6 +1,7 @@
 #import <XCTest/XCTest.h>
 #import "TestUtil.h"
 #import "PhoneNumber.h"
+#import "PhoneNumberUtil.h"
 
 @interface PhoneNumberTest : XCTestCase
 
@@ -15,4 +16,64 @@
     test([[[PhoneNumber tryParsePhoneNumberFromText:@"1-902-５５５-5555" fromRegion:@"US"] toE164] isEqualToString:@"+19025555555"]);
 }
 
+-(void) testTranslateCursorPosition {
+    testThrows([PhoneNumberUtil translateCursorPosition:0 from:nil to:@"" stickingRightward:true]);
+    testThrows([PhoneNumberUtil translateCursorPosition:0 from:@"" to:nil stickingRightward:true]);
+    testThrows([PhoneNumberUtil translateCursorPosition:1 from:@"" to:@"" stickingRightward:true]);
+    
+    test([PhoneNumberUtil translateCursorPosition:0 from:@"" to:@"" stickingRightward:true] == 0);
+
+    test([PhoneNumberUtil translateCursorPosition:0 from:@"12" to:@"1" stickingRightward:true] == 0);
+    test([PhoneNumberUtil translateCursorPosition:1 from:@"12" to:@"1" stickingRightward:true] == 1);
+    test([PhoneNumberUtil translateCursorPosition:2 from:@"12" to:@"1" stickingRightward:true] == 1);
+    
+    test([PhoneNumberUtil translateCursorPosition:0 from:@"1" to:@"12" stickingRightward:false] == 0);
+    test([PhoneNumberUtil translateCursorPosition:0 from:@"1" to:@"12" stickingRightward:true] == 0);
+    test([PhoneNumberUtil translateCursorPosition:1 from:@"1" to:@"12" stickingRightward:false] == 1);
+    test([PhoneNumberUtil translateCursorPosition:1 from:@"1" to:@"12" stickingRightward:true] == 2);
+    
+    test([PhoneNumberUtil translateCursorPosition:0 from:@"12" to:@"132" stickingRightward:false] == 0);
+    test([PhoneNumberUtil translateCursorPosition:0 from:@"12" to:@"132" stickingRightward:true] == 0);
+    test([PhoneNumberUtil translateCursorPosition:1 from:@"12" to:@"132" stickingRightward:false] == 1);
+    test([PhoneNumberUtil translateCursorPosition:1 from:@"12" to:@"132" stickingRightward:true] == 2);
+    test([PhoneNumberUtil translateCursorPosition:2 from:@"12" to:@"132" stickingRightward:false] == 3);
+    test([PhoneNumberUtil translateCursorPosition:2 from:@"12" to:@"132" stickingRightward:true] == 3);
+
+    test([PhoneNumberUtil translateCursorPosition:0 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:true] == 0);
+    test([PhoneNumberUtil translateCursorPosition:1 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:true] == 1);
+    test([PhoneNumberUtil translateCursorPosition:2 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:true] == 2);
+    test([PhoneNumberUtil translateCursorPosition:3 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:true] == 3);
+    test([PhoneNumberUtil translateCursorPosition:4 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:true] == 3);
+    test([PhoneNumberUtil translateCursorPosition:5 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:true] == 3);
+    test([PhoneNumberUtil translateCursorPosition:6 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:true] == 6);
+    test([PhoneNumberUtil translateCursorPosition:7 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:true] == 7);
+    test([PhoneNumberUtil translateCursorPosition:8 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:true] == 8);
+    test([PhoneNumberUtil translateCursorPosition:9 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:true] == 8);
+    
+    test([PhoneNumberUtil translateCursorPosition:0 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:false] == 0);
+    test([PhoneNumberUtil translateCursorPosition:1 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:false] == 1);
+    test([PhoneNumberUtil translateCursorPosition:2 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:false] == 2);
+    test([PhoneNumberUtil translateCursorPosition:3 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:false] == 3);
+    test([PhoneNumberUtil translateCursorPosition:4 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:false] == 3);
+    test([PhoneNumberUtil translateCursorPosition:5 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:false] == 3);
+    test([PhoneNumberUtil translateCursorPosition:6 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:false] == 4);
+    test([PhoneNumberUtil translateCursorPosition:7 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:false] == 7);
+    test([PhoneNumberUtil translateCursorPosition:8 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:false] == 8);
+    test([PhoneNumberUtil translateCursorPosition:9 from:@"(55) 123-4567" to:@"(551) 234-567" stickingRightward:false] == 8);
+    
+    test([PhoneNumberUtil translateCursorPosition:0 from:@"(5551) 234-567" to:@"(555) 123-4567" stickingRightward:true] == 0);
+    test([PhoneNumberUtil translateCursorPosition:1 from:@"(5551) 234-567" to:@"(555) 123-4567" stickingRightward:true] == 1);
+    test([PhoneNumberUtil translateCursorPosition:2 from:@"(5551) 234-567" to:@"(555) 123-4567" stickingRightward:true] == 2);
+    test([PhoneNumberUtil translateCursorPosition:3 from:@"(5551) 234-567" to:@"(555) 123-4567" stickingRightward:true] == 3);
+    test([PhoneNumberUtil translateCursorPosition:4 from:@"(5551) 234-567" to:@"(555) 123-4567" stickingRightward:true] == 6);
+    test([PhoneNumberUtil translateCursorPosition:5 from:@"(5551) 234-567" to:@"(555) 123-4567" stickingRightward:true] == 7);
+
+    test([PhoneNumberUtil translateCursorPosition:0 from:@"(5551) 234-567" to:@"(555) 123-4567" stickingRightward:false] == 0);
+    test([PhoneNumberUtil translateCursorPosition:1 from:@"(5551) 234-567" to:@"(555) 123-4567" stickingRightward:false] == 1);
+    test([PhoneNumberUtil translateCursorPosition:2 from:@"(5551) 234-567" to:@"(555) 123-4567" stickingRightward:false] == 2);
+    test([PhoneNumberUtil translateCursorPosition:3 from:@"(5551) 234-567" to:@"(555) 123-4567" stickingRightward:false] == 3);
+    test([PhoneNumberUtil translateCursorPosition:4 from:@"(5551) 234-567" to:@"(555) 123-4567" stickingRightward:false] == 4);
+    test([PhoneNumberUtil translateCursorPosition:5 from:@"(5551) 234-567" to:@"(555) 123-4567" stickingRightward:false] == 7);
+}
+
 @end

Signal/test/util/UtilTest.m

@@ -378,7 +378,7 @@
     test(!@(-pow(2, 64)).hasLongLongValue);
     test(!(@0.5).hasLongLongValue);
 }
--(void) tryParseAsUnsignedInteger {
+-(void) testTryParseAsUnsignedInteger {
     test([@"" tryParseAsUnsignedInteger] == nil);
     test([@"88ffhih" tryParseAsUnsignedInteger] == nil);
     test([@"0xA" tryParseAsUnsignedInteger] == nil);
@@ -391,6 +391,47 @@
     test([[@"0" tryParseAsUnsignedInteger] isEqual:@0]);
     test([[@"1" tryParseAsUnsignedInteger] isEqual:@1]);
     test([[@"25" tryParseAsUnsignedInteger] isEqual:@25]);
-    test([[@"4294967296" tryParseAsUnsignedInteger] isEqual:@4294967296]);
+    test([[(@NSUIntegerMax).description tryParseAsUnsignedInteger] isEqual:@NSUIntegerMax]);
+    if (NSUIntegerMax == 4294967295) {
+        test([@"4294967296" tryParseAsUnsignedInteger] == nil);
+    }
+}
+-(void) testRemoveAllCharactersIn {
+    testThrows([@"" removeAllCharactersIn:nil]);
+
+    test([[@"" removeAllCharactersIn:NSCharacterSet.letterCharacterSet] isEqual:@""]);
+    test([[@"1" removeAllCharactersIn:NSCharacterSet.letterCharacterSet] isEqual:@"1"]);
+    test([[@"a" removeAllCharactersIn:NSCharacterSet.letterCharacterSet] isEqual:@""]);
+    test([[@"A" removeAllCharactersIn:NSCharacterSet.letterCharacterSet] isEqual:@""]);
+    test([[@"abc123%^&" removeAllCharactersIn:NSCharacterSet.letterCharacterSet] isEqual:@"123%^&"]);
+
+    test([[@"" removeAllCharactersIn:NSCharacterSet.decimalDigitCharacterSet] isEqual:@""]);
+    test([[@"1" removeAllCharactersIn:NSCharacterSet.decimalDigitCharacterSet] isEqual:@""]);
+    test([[@"a" removeAllCharactersIn:NSCharacterSet.decimalDigitCharacterSet] isEqual:@"a"]);
+    test([[@"A" removeAllCharactersIn:NSCharacterSet.decimalDigitCharacterSet] isEqual:@"A"]);
+    test([[@"abc123%^&" removeAllCharactersIn:NSCharacterSet.decimalDigitCharacterSet] isEqual:@"abc%^&"]);
+}
+-(void) testDigitsOnly {
+    test([@"".digitsOnly isEqual:@""]);
+    test([@"1".digitsOnly isEqual:@"1"]);
+    test([@"a".digitsOnly isEqual:@""]);
+    test([@"(555) 235-7111".digitsOnly isEqual:@"5552357111"]);
+}
+-(void) testWithCharactersInRangeReplacedBy {
+    testThrows([@"" withCharactersInRange:NSMakeRange(0, 0) replacedBy:nil]);
+    testThrows([@"" withCharactersInRange:NSMakeRange(0, 1) replacedBy:@""]);
+    testThrows([@"" withCharactersInRange:NSMakeRange(1, 0) replacedBy:@""]);
+    testThrows([@"" withCharactersInRange:NSMakeRange(1, 1) replacedBy:@""]);
+    testThrows([@"abc" withCharactersInRange:NSMakeRange(4, 0) replacedBy:@""]);
+    testThrows([@"abc" withCharactersInRange:NSMakeRange(3, 1) replacedBy:@""]);
+    testThrows([@"abc" withCharactersInRange:NSMakeRange(4, NSUIntegerMax) replacedBy:@""]);
+    
+    test([[@"" withCharactersInRange:NSMakeRange(0, 0) replacedBy:@""] isEqual:@""]);
+    test([[@"" withCharactersInRange:NSMakeRange(0, 0) replacedBy:@"abc"] isEqual:@"abc"]);
+    test([[@"abc" withCharactersInRange:NSMakeRange(0, 0) replacedBy:@"123"] isEqual:@"123abc"]);
+    test([[@"abc" withCharactersInRange:NSMakeRange(3, 0) replacedBy:@"123"] isEqual:@"abc123"]);
+    test([[@"abc" withCharactersInRange:NSMakeRange(2, 0) replacedBy:@"123"] isEqual:@"ab123c"]);
+    test([[@"abcdef" withCharactersInRange:NSMakeRange(1, 2) replacedBy:@"1234"] isEqual:@"a1234def"]);
 }
+
 @end