Strip control characters from omnibox input without losing attributes.

chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm

Index: chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
diff --git a/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm b/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
index 817cabcd185d68c59f28e106ffd1d988f13ae828..2646f9ccebf47e32704f8d64c599387b0eb40064 100644
--- a/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
+++ b/chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.mm
@@ -275,23 +275,29 @@ BOOL ThePasteboardIsTooDamnBig() {
 // Prevent control characters from being entered into the Omnibox.
 // This is invoked for keyboard entry, not for pasting.
 - (void)insertText:(id)aString {
-  // This method is documented as received either |NSString| or
-  // |NSAttributedString|.  The autocomplete code will restyle the
-  // results in any case, so simplify by always using |NSString|.
-  if ([aString isKindOfClass:[NSAttributedString class]])
-    aString = [aString string];
-
   // Repeatedly remove control characters.  The loop will only ever
-  // execute at allwhen the user enters control characters (using
+  // execute at all when the user enters control characters (using
   // Ctrl-Alt- or Ctrl-Q).  Making this generally efficient would
   // probably be a loss, since the input always seems to be a single
   // character.
-  NSRange range = [aString rangeOfCharacterFromSet:forbiddenCharacters_];
-  while (range.location != NSNotFound) {
-    aString = [aString stringByReplacingCharactersInRange:range withString:@""];
-    range = [aString rangeOfCharacterFromSet:forbiddenCharacters_];
+  if ([aString isKindOfClass:[NSAttributedString class]]) {
+    NSRange range =
+        [[aString string] rangeOfCharacterFromSet:forbiddenCharacters_];
+    while (range.location != NSNotFound) {
+      aString = [[aString mutableCopy] autorelease];

[Mark Mentovai, 2011/08/12 19:49:24]

Technically you only need to make the copy the first time. Premature
optimization? Maybe.

[Avi (use Gerrit), 2011/08/12 19:55:16]

True, but...

99.99999% of the time there are no control characters, and I don't believe there
is a way to put in two control characters in one input event.

[Mark Mentovai, 2011/08/12 19:57:02]

Avi wrote:
Then why is there a loop at all?

I wasn’t saying “make the copy even if you don’t need it,” by the way, I meant
to do:

  bool madeCopy = false;
  while (condition) {
    if (!madeCopy)
      aString = [[aString mutableCopy] autorelease];
    // transform aString and recompute input to condition
  }

Not important either way, just clarifying.

+      [aString deleteCharactersInRange:range];
+      range = [[aString string] rangeOfCharacterFromSet:forbiddenCharacters_];
+    }
+    DCHECK_EQ(range.length, 0U);
+  } else {
+    NSRange range = [aString rangeOfCharacterFromSet:forbiddenCharacters_];
+    while (range.location != NSNotFound) {
+      aString =
+          [aString stringByReplacingCharactersInRange:range withString:@""];
+      range = [aString rangeOfCharacterFromSet:forbiddenCharacters_];
+    }
+    DCHECK_EQ(range.length, 0U);
   }
-  DCHECK_EQ(range.length, 0U);
 
   // NOTE: If |aString| is empty, this intentionally replaces the
   // selection with empty.  This seems consistent with the case where