330666 - Switch to use native NSSearchField widget in toolbar and search sheet

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Håkan Waara]

Since we now go to 10.3, we can start using the native NSSearchField widget. For one it would mean less code to maintain (bugs like bug 329673).

Another reason is for example some software that would have an easier time integrating with Camino if we did this natively (although he seems to have succeeded now):

http://www.flickr.com/photos/xlife/113206139

Comment 1

[Håkan Waara]

Another good reason to do away with our custom class is that it's not accessible, like the standard one is.

Comment 2

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

Privacy prefPane search fields are in bug 357560, so this is just for the toolbar and the Bookmarks Manager.

Comment 3

[Stuart Morgan]

And now the Bookmark Manager is bug 358620.

Comment 4

[Stuart Morgan]

Taking.  It looks like we'll need a subclass of NSSearchFieldCell to do the hint text, so this will have to wait for the SDK.

Comment 5

[Simon Fraser [no longer active]]

Isn't the hint text just the placeholderString?

Comment 6

[Stuart Morgan]

Whoops, yeah.  I'll take another look.

Comment 8

[Stuart Morgan]

The bigger issue turned out to be the custom drag-and-drop support (possible among other things); we'll probably want to hold off until after 1.1 to avoid accidently regressing some aspect of its behavior this late in the game.

Comment 10

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

Mass un-setting milestone per 1.6 roadmap.

Filter on RemoveRedonkulousBuglist to remove bugspam.

Developers: if you have a patch in hand for one of these bugs, you may pull the bug back to 1.6 *at that point*.

Comment 11

[Stuart Morgan]

Attachment: fix

Replaces the toolbar/sheet search field with a small NSSearchField subclass. All our custom behavior (control character stripping, drag-and-drop support, etc.) should continue to work. This is a trunk version; the project is likely to rot anyway so I'll wait to do a branch project patch until it's ready to land.

Once this and the privacy pane switch land, I'll rip out SearchTextField and all the associated classes and images in another bug.

Comment 12

[Stuart Morgan]

Attachment: New nib

Comment 13

[Håkan Waara]

Comment on attachment 275293 [details] [diff] [review]
fix

Nice cleanup!

Seems like you missed to post the diff of WebSearchField.* ... If you want, you can post it as a separate diff.

I only have one nit in this diff: please don't use |aFoo| variable names unless they are arguments - it can be very confusing. I know that the previous code already did this, but since you're refactoring it...

Comment 14

[Stuart Morgan]

Attachment: v2

> Seems like you missed to post the diff of WebSearchField.*

Doh! Fixed.

> please don't use |aFoo| variable names unless they are arguments

Agreed and fixed.

Comment 15

[Håkan Waara]

Comment on attachment 277054 [details] [diff] [review]
v2

>+// Changes the current search engine setting to the one names |engineName|.
>+- (void)setCurrentSearchEngine:(NSString*)engineName

Typo: "names"?

>+  NSMenu* engineMenu = [[[NSMenu alloc] initWithTitle:@"Search Engines"] autorelease];

This will have to be localized, so shouldn't you use NSLocalizedString?

Comment 16

[Stuart Morgan]

(In reply to comment #15)
> >+  NSMenu* engineMenu = [[[NSMenu alloc] initWithTitle:@"Search Engines"] autorelease];
> 
> This will have to be localized, so shouldn't you use NSLocalizedString?

The name of the menu doesn't matter, and isn't user-visible.

Comment 17

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 277054 [details] [diff] [review]
v2

sr=pink

Comment 18

[Stuart Morgan]

Landed on trunk and MOZILLA_1_8_BRANCH. Yay!

Comment 19

[Stuart Morgan]

The nib was missing a delegate connection, so a bunch of the resize behavior was broken. I've checked in a fixed nib.