Strip control characters from omnibox input without losing attributes.

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

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/location_bar/autocomplete_text_field_editor.h"
 
 #include "base/string_util.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"  // IDC_*
 #include "chrome/browser/ui/browser_list.h"
@@ skipping 257 matching lines @@
 }
 
 // (URLDropTarget protocol)
 - (BOOL)performDragOperation:(id<NSDraggingInfo>)sender {
   return [dropHandler_ performDragOperation:sender];
 }
 
 // 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
   // the input contained a mixture of characters and the string ended
   // up not empty.
   [super insertText:aString];
 }
 
 - (void)setMarkedText:(id)aString selectedRange:(NSRange)selRange {
   if (![self hasMarkedText]) {
@@ skipping 122 matching lines @@
 - (void)mouseDown:(NSEvent*)theEvent {
   // Close the popup before processing the event.
   AutocompleteTextFieldObserver* observer = [self observer];
   if (observer)
     observer->ClosePopup();
 
   [super mouseDown:theEvent];
 }
 
 @end