358472 - Search field should not allow control characters

Status: RESOLVED FIXED

(Camino Graveyard :: Toolbars & Menus, defect)

Description

[Ben Willmore]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a1) Gecko/20061027 Camino/1.2+
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a1) Gecko/20061027 Camino/1.2+

The Google search field does not deal well with control characters.  For example, select several lines of text from a web page.  Cmd-C, Cmd-V copy-paste them into the google search field.  The text is displaced downwards a few pixels, the highlight looks odd, and it is non-obvious that there are multiple lines of text in the field.

Pressing option-enter when typing into the field is similarly confusing.

I cannot currently think of a reason why control characters would be allowed in this field. Hence:

Index: browser/SearchTextField.m
===================================================================
RCS file: /cvsroot/mozilla/camino/src/browser/SearchTextField.m,v
retrieving revision 1.16
diff -u -d -b -w -u -8 -r1.16 SearchTextField.m
--- browser/SearchTextField.m   23 Mar 2006 23:08:23 -0000      1.16
+++ browser/SearchTextField.m   28 Oct 2006 05:50:11 -0000
@@ -144,16 +144,17 @@
 
 - (void)setHasPopUpButton:(BOOL)aBoolean
 {
   [[self cell] setHasPopUpButton:aBoolean];
 }
 
 - (void)textDidChange:(NSNotification *)aNotification
 {
+  [self setStringValue:[[self stringValue] stringByRemovingCharactersInSet:[NSCharacterSet controlCharacterSet]]];
   [[self cell] searchSubmittedFromView:self];
   [super textDidChange: aNotification];
 }
 
 //
 // -setStringValue:
 //
 // Override to display the clear (x) when setting the search bar's text


Reproducible: Always

Comment 1

[Ben Willmore]

Attachment: patch without erroneous line breaks

Comment 2

[Chris Lawson (gone)]

I agree, and the patch looks reasonable. r=me.

Comment 3

[Chris Lawson (gone)]

Fixing summary/OS, targeting for 1.1.

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 243878 [details] [diff] [review]
patch without erroneous line breaks

I think Stuart can probably sr this one ;)

Comment 5

[Stuart Morgan]

Is there any reason to do this instead of using a formatter on the field to prevent control character input?  That would have the advantage of not going away and needing to be reimplemented when we rip out the fake search fields.

Comment 6

[Stuart Morgan]

Comment on attachment 243878 [details] [diff] [review]
patch without erroneous line breaks

I guess since it's done there's no real reason not to check it in, so sr=me.  I just hate to see fixes going into a class that we are already planning to remove.

Comment 7

[froodian (Ian Leue)]

Checked in on 1.8branch and trunk.