diff options
8 files changed, 338 insertions, 349 deletions
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h index c8d0bfb..f08d37f 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h +++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h @@ -7,26 +7,16 @@ #import <Cocoa/Cocoa.h> -#include "base/basictypes.h" -#include "base/scoped_nsobject.h" #include "base/scoped_ptr.h" -#include "chrome/browser/autocomplete/autocomplete.h" #include "chrome/browser/autocomplete/autocomplete_edit_view.h" -#include "chrome/browser/toolbar_model.h" #include "chrome/browser/cocoa/autocomplete_text_field.h" -#include "chrome/common/page_transition_types.h" -#include "grit/generated_resources.h" -#include "webkit/glue/window_open_disposition.h" class AutocompleteEditController; -class AutocompleteEditModel; -@class AutocompleteFieldDelegate; class AutocompletePopupViewMac; class BubblePositioner; class Clipboard; class CommandUpdater; class Profile; -class TabContents; class ToolbarModel; // Implements AutocompleteEditView on an AutocompleteTextField. @@ -94,43 +84,11 @@ class AutocompleteEditViewMac : public AutocompleteEditView, virtual int GetPasteActionStringId(); virtual void OnPasteAndGo(); virtual void OnFrameChanged(); - - // Helper functions for use from AutocompleteEditHelper Objective-C - // class. - - // Returns true if |popup_view_| is open. - bool IsPopupOpen() const; - - // Trivial wrappers forwarding to |model_| methods. - void OnEscapeKeyPressed(); - void OnUpOrDownKeyPressed(bool up, bool by_page); - - // Called when editing begins in the field, and before the results - // of any editing are communicated to |model_|. - void OnWillBeginEditing(); - - // Called when editing ends in the field. - void OnDidEndEditing(); - - // Called when the window |field_| is in loses key to clean up - // visual state (such as closing the popup). - void OnDidResignKey(); - - // Checks if a keyword search is possible and forwards to |model_| - // if so. Returns true if the tab should be eaten. - bool OnTabPressed(); - - // Called when the user hits backspace in |field_|. Checks whether - // keyword search is being terminated. Returns true if the - // backspace should be intercepted (not forwarded on to the standard - // machinery). - bool OnBackspacePressed(); - - // Forward to same method in |popup_view_| model. Used when - // Shift-Delete is pressed, to delete items from the popup. - void TryDeletingCurrentItem(); - - void AcceptInput(WindowOpenDisposition disposition, bool for_drop); + virtual void OnDidResignKey(); // Closes the popup. + virtual void OnDidBeginEditing(); + virtual void OnDidChange(); + virtual void OnDidEndEditing(); + virtual bool OnDoCommandBySelector(SEL cmd); // Helper for LocationBarViewMac. Selects all in |field_|. void FocusLocation(); @@ -140,6 +98,12 @@ class AutocompleteEditViewMac : public AutocompleteEditView, static std::wstring GetClipboardText(Clipboard* clipboard); private: + // Called when the user hits backspace in |field_|. Checks whether + // keyword search is being terminated. Returns true if the + // backspace should be intercepted (not forwarded on to the standard + // machinery). + bool OnBackspacePressed(); + // Returns the field's currently selected range. Only valid if the // field has focus. NSRange GetSelectedRange() const; @@ -178,9 +142,6 @@ class AutocompleteEditViewMac : public AutocompleteEditView, AutocompleteTextField* field_; // owned by tab controller - // Objective-C object to bridge field_ delegate calls to C++. - scoped_nsobject<AutocompleteFieldDelegate> edit_helper_; - // Selection at the point where the user started using the // arrows to move around in the popup. NSRange saved_temporary_selection_; diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm index a8a15c7..8f3c58a 100644 --- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm +++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm @@ -7,10 +7,7 @@ #include <Carbon/Carbon.h> // kVK_Return #include "app/clipboard/clipboard.h" -#include "app/gfx/font.h" -#include "app/l10n_util_mac.h" #include "app/resource_bundle.h" -#import "base/cocoa_protocols_mac.h" #include "base/string_util.h" #include "base/sys_string_conversions.h" #include "chrome/browser/autocomplete/autocomplete_edit.h" @@ -19,6 +16,7 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/cocoa/event_utils.h" #include "chrome/browser/tab_contents/tab_contents.h" +#include "chrome/browser/toolbar_model.h" #include "grit/generated_resources.h" // Focus-handling between |field_| and |model_| is a bit subtle. @@ -124,21 +122,6 @@ NSRange ComponentToNSRange(const url_parse::Component& component) { } // namespace -// Thin Obj-C bridge class that is the delegate of the omnibox field. -// It intercepts various control delegate methods and vectors them to -// the edit view. - -// TODO(shess): Consider moving more of this code off to -// AutocompleteTextFieldObserver. - -@interface AutocompleteFieldDelegate : NSObject<NSTextFieldDelegate> { - @private - AutocompleteEditViewMac* edit_view_; // weak, owns us. -} -- initWithEditView:(AutocompleteEditViewMac*)view; -- (void)windowDidResignKey:(NSNotification*)notification; -@end - // TODO(shess): AutocompletePopupViewMac doesn't really need an // NSTextField. It wants to know where the position the popup, what // font to use, and it also needs to be able to attach the popup to @@ -156,26 +139,16 @@ AutocompleteEditViewMac::AutocompleteEditViewMac( controller_(controller), toolbar_model_(toolbar_model), command_updater_(command_updater), - field_(field), - edit_helper_([[AutocompleteFieldDelegate alloc] initWithEditView:this]) { + field_(field) { DCHECK(controller); DCHECK(toolbar_model); DCHECK(profile); DCHECK(command_updater); DCHECK(field); - [field_ setDelegate:edit_helper_.get()]; [field_ setObserver:this]; // Needed so that editing doesn't lose the styling. [field_ setAllowsEditingTextAttributes:YES]; - - // Track the window's key status for signalling focus changes to - // |model_|. - NSNotificationCenter* nc = [NSNotificationCenter defaultCenter]; - [nc addObserver:edit_helper_ - selector:@selector(windowDidResignKey:) - name:NSWindowDidResignKeyNotification - object:[field_ window]]; } AutocompleteEditViewMac::~AutocompleteEditViewMac() { @@ -188,9 +161,7 @@ AutocompleteEditViewMac::~AutocompleteEditViewMac() { popup_view_.reset(); model_.reset(); - // Disconnect field_ from edit_helper_ so that we don't get calls - // after destruction. - [field_ setDelegate:nil]; + // Disconnect from |field_|, it outlives this object. [field_ setObserver:NULL]; } @@ -561,22 +532,7 @@ gfx::NativeView AutocompleteEditViewMac::GetNativeView() const { return field_; } -void AutocompleteEditViewMac::OnUpOrDownKeyPressed(bool up, bool by_page) { - // We should only arrive here when the field is focussed. - DCHECK(IsFirstResponder()); - - const int count = by_page ? model_->result().size() : 1; - model_->OnUpOrDownKeyPressed(up ? -count : count); -} - -void AutocompleteEditViewMac::OnEscapeKeyPressed() { - // We should only arrive here when the field is focussed. - DCHECK(IsFirstResponder()); - - model_->OnEscapeKeyPressed(); -} - -void AutocompleteEditViewMac::OnWillBeginEditing() { +void AutocompleteEditViewMac::OnDidBeginEditing() { // We should only arrive here when the field is focussed. DCHECK([field_ currentEditor]); @@ -588,6 +544,14 @@ void AutocompleteEditViewMac::OnWillBeginEditing() { OnBeforePossibleChange(); } +void AutocompleteEditViewMac::OnDidChange() { + // Figure out what changed and notify the model_. + OnAfterPossibleChange(); + + // Then capture the new state. + OnBeforePossibleChange(); +} + void AutocompleteEditViewMac::OnDidEndEditing() { ClosePopup(); @@ -595,6 +559,108 @@ void AutocompleteEditViewMac::OnDidEndEditing() { model_->OnKillFocus(); } +bool AutocompleteEditViewMac::OnDoCommandBySelector(SEL cmd) { + // We should only arrive here when the field is focussed. + DCHECK(IsFirstResponder()); + + // Don't intercept up/down-arrow if the popup isn't open. + if (popup_view_->IsOpen()) { + if (cmd == @selector(moveDown:)) { + model_->OnUpOrDownKeyPressed(1); + return true; + } + + if (cmd == @selector(moveUp:)) { + model_->OnUpOrDownKeyPressed(-1); + return true; + } + } + + if (cmd == @selector(scrollPageDown:)) { + model_->OnUpOrDownKeyPressed(model_->result().size()); + return true; + } + + if (cmd == @selector(scrollPageUp:)) { + model_->OnUpOrDownKeyPressed(-model_->result().size()); + return true; + } + + if (cmd == @selector(cancelOperation:)) { + model_->OnEscapeKeyPressed(); + return true; + } + + if (cmd == @selector(insertTab:)) { + if (model_->is_keyword_hint() && !model_->keyword().empty()) { + model_->AcceptKeyword(); + return true; + } + } + + // TODO(shess): Option-tab, would normally insert a literal tab + // character. Consider combining with -insertTab: + if (cmd == @selector(insertTabIgnoringFieldEditor:)) { + return true; + } + + // |-noop:| is sent when the user presses Cmd+Return. Override the no-op + // behavior with the proper WindowOpenDisposition. + NSEvent* event = [NSApp currentEvent]; + if (cmd == @selector(insertNewline:) || + (cmd == @selector(noop:) && [event keyCode] == kVK_Return)) { + WindowOpenDisposition disposition = + event_utils::WindowOpenDispositionFromNSEvent(event); + model_->AcceptInput(disposition, false); + // Opening a URL in a background tab should also revert the omnibox contents + // to their original state. We cannot do a blanket revert in OpenURL() + // because middle-clicks also open in a new background tab, but those should + // not revert the omnibox text. + RevertAll(); + return true; + } + + // Option-Return + if (cmd == @selector(insertNewlineIgnoringFieldEditor:)) { + model_->AcceptInput(NEW_FOREGROUND_TAB, false); + return true; + } + + // When the user does Control-Enter, the existing content has "www." + // prepended and ".com" appended. |model_| should already have + // received notification when the Control key was depressed, but it + // is safe to tell it twice. + if (cmd == @selector(insertLineBreak:)) { + OnControlKeyChanged(true); + WindowOpenDisposition disposition = + event_utils::WindowOpenDispositionFromNSEvent([NSApp currentEvent]); + model_->AcceptInput(disposition, false); + return true; + } + + if (cmd == @selector(deleteBackward:)) { + if (OnBackspacePressed()) { + return true; + } + } + + if (cmd == @selector(deleteForward:)) { + const NSUInteger modifiers = [[NSApp currentEvent] modifierFlags]; + if ((modifiers & NSShiftKeyMask) != 0) { + if (popup_view_->IsOpen()) { + popup_view_->GetModel()->TryDeletingCurrentItem(); + return true; + } + } + } + + // Capture the state before the operation changes the content. + // TODO(shess): Determine if this is always redundent WRT the call + // in -controlTextDidChange:. + OnBeforePossibleChange(); + return false; +} + void AutocompleteEditViewMac::OnDidResignKey() { ClosePopup(); } @@ -665,14 +731,6 @@ void AutocompleteEditViewMac::OnFrameChanged() { controller_->OnChanged(); } -bool AutocompleteEditViewMac::OnTabPressed() { - if (model_->is_keyword_hint() && !model_->keyword().empty()) { - model_->AcceptKeyword(); - return true; - } - return false; -} - bool AutocompleteEditViewMac::OnBackspacePressed() { // Don't intercept if not in keyword search mode. if (model_->is_keyword_hint() || model_->keyword().empty()) { @@ -692,23 +750,10 @@ bool AutocompleteEditViewMac::OnBackspacePressed() { return true; } -bool AutocompleteEditViewMac::IsPopupOpen() const { - return popup_view_->IsOpen(); -} - -void AutocompleteEditViewMac::TryDeletingCurrentItem() { - popup_view_->GetModel()->TryDeletingCurrentItem(); -} - void AutocompleteEditViewMac::OnControlKeyChanged(bool pressed) { model_->OnControlKeyChanged(pressed); } -void AutocompleteEditViewMac::AcceptInput( - WindowOpenDisposition disposition, bool for_drop) { - model_->AcceptInput(disposition, for_drop); -} - void AutocompleteEditViewMac::FocusLocation() { [[field_ window] makeFirstResponder:field_]; DCHECK_EQ([field_ currentEditor], [[field_ window] firstResponder]); @@ -755,154 +800,3 @@ std::wstring AutocompleteEditViewMac::GetClipboardText(Clipboard* clipboard) { return std::wstring(); } - -@implementation AutocompleteFieldDelegate - -- initWithEditView:(AutocompleteEditViewMac*)view { - self = [super init]; - if (self) { - edit_view_ = view; - } - return self; -} - -- (void)dealloc { - [[NSNotificationCenter defaultCenter] removeObserver:self]; - [super dealloc]; -} - -- (BOOL)control:(NSControl*)control - textView:(NSTextView*)textView doCommandBySelector:(SEL)cmd { - // Don't intercept up/down-arrow if the popup isn't open. - if (edit_view_->IsPopupOpen()) { - if (cmd == @selector(moveDown:)) { - edit_view_->OnUpOrDownKeyPressed(false, false); - return YES; - } - - if (cmd == @selector(moveUp:)) { - edit_view_->OnUpOrDownKeyPressed(true, false); - return YES; - } - } - - if (cmd == @selector(scrollPageDown:)) { - edit_view_->OnUpOrDownKeyPressed(false, true); - return YES; - } - - if (cmd == @selector(scrollPageUp:)) { - edit_view_->OnUpOrDownKeyPressed(true, true); - return YES; - } - - if (cmd == @selector(cancelOperation:)) { - edit_view_->OnEscapeKeyPressed(); - return YES; - } - - if (cmd == @selector(insertTab:)) { - if (edit_view_->OnTabPressed()) { - return YES; - } - } - - // TODO(shess): Option-tab, would normally insert a literal tab - // character. Consider combining with -insertTab: - if (cmd == @selector(insertTabIgnoringFieldEditor:)) { - return YES; - } - - // |-noop:| is sent when the user presses Cmd+Return. Override the no-op - // behavior with the proper WindowOpenDisposition. - NSEvent* event = [NSApp currentEvent]; - if (cmd == @selector(insertNewline:) || - (cmd == @selector(noop:) && [event keyCode] == kVK_Return)) { - WindowOpenDisposition disposition = - event_utils::WindowOpenDispositionFromNSEvent(event); - edit_view_->AcceptInput(disposition, false); - // Opening a URL in a background tab should also revert the omnibox contents - // to their original state. We cannot do a blanket revert in OpenURL() - // because middle-clicks also open in a new background tab, but those should - // not revert the omnibox text. - edit_view_->RevertAll(); - return YES; - } - - // Option-Return - if (cmd == @selector(insertNewlineIgnoringFieldEditor:)) { - edit_view_->AcceptInput(NEW_FOREGROUND_TAB, false); - return YES; - } - - // When the user does Control-Enter, the existing content has "www." - // prepended and ".com" appended. |model_| should already have - // received notification when the Control key was depressed, but it - // is safe to tell it twice. - if (cmd == @selector(insertLineBreak:)) { - edit_view_->OnControlKeyChanged(true); - WindowOpenDisposition disposition = - event_utils::WindowOpenDispositionFromNSEvent([NSApp currentEvent]); - edit_view_->AcceptInput(disposition, false); - return YES; - } - - if (cmd == @selector(deleteBackward:)) { - if (edit_view_->OnBackspacePressed()) { - return YES; - } - } - - if (cmd == @selector(deleteForward:)) { - const NSUInteger modifiers = [[NSApp currentEvent] modifierFlags]; - if ((modifiers & NSShiftKeyMask) != 0) { - if (edit_view_->IsPopupOpen()) { - edit_view_->TryDeletingCurrentItem(); - return YES; - } - } - } - - // Capture the state before the operation changes the content. - // TODO(shess): Determine if this is always redundent WRT the call - // in -controlTextDidChange:. - edit_view_->OnBeforePossibleChange(); - return NO; -} - -- (void)controlTextDidBeginEditing:(NSNotification*)aNotification { - // After the user runs the Print or Page Layout panels, this - // notification starts coming at inappropriate times. Ignore the - // notification when the field does not have a field editor. See - // http://crbug.com/19116 for additional info. - NSTextField* field = static_cast<NSTextField*>([aNotification object]); - if ([field isKindOfClass:[NSTextField class]] && ![field currentEditor]) { - return; - } - - edit_view_->OnWillBeginEditing(); -} - -- (void)controlTextDidChange:(NSNotification*)aNotification { - // Figure out what changed and notify the model_. - edit_view_->OnAfterPossibleChange(); - - // Then capture the new state. - edit_view_->OnBeforePossibleChange(); -} - -- (BOOL)control:(NSControl*)control textShouldEndEditing:(NSText*)fieldEditor { - edit_view_->OnDidEndEditing(); - - return YES; - - // TODO(shess): Figure out where the selection belongs. On GTK, - // it's set to the start of the text. -} - -// Signal that we've lost focus when the window resigns key. -- (void)windowDidResignKey:(NSNotification*)notification { - edit_view_->OnDidResignKey(); -} - -@end diff --git a/chrome/browser/cocoa/autocomplete_text_field.h b/chrome/browser/cocoa/autocomplete_text_field.h index 7c21527..7336441 100644 --- a/chrome/browser/cocoa/autocomplete_text_field.h +++ b/chrome/browser/cocoa/autocomplete_text_field.h @@ -48,11 +48,31 @@ class AutocompleteTextFieldObserver { virtual int GetPasteActionStringId() = 0; // Called when the user initiates a "paste and go" or "paste and - // search" into |field_|. + // search" into the field. virtual void OnPasteAndGo() = 0; // Called when the field's frame changes. virtual void OnFrameChanged() = 0; + + // Called when the window containing the field loses focus. + virtual void OnDidResignKey() = 0; + + // Called when the user begins editing the field, for every edit, + // and when the user is done editing the field. + virtual void OnDidBeginEditing() = 0; + virtual void OnDidChange() = 0; + virtual void OnDidEndEditing() = 0; + + // NSResponder translates certain keyboard actions into selectors + // passed to -doCommandBySelector:. The selector is forwarded here, + // return true if |cmd| is handled, false if the caller should + // handle it. + // TODO(shess): For now, I think having the code which makes these + // decisions closer to the other autocomplete code is worthwhile, + // since it calls a wide variety of methods which otherwise aren't + // clearly relevent to expose here. But consider pulling more of + // the AutocompleteEditViewMac calls up to here. + virtual bool OnDoCommandBySelector(SEL cmd) = 0; }; @interface AutocompleteTextField : StyledTextField { diff --git a/chrome/browser/cocoa/autocomplete_text_field.mm b/chrome/browser/cocoa/autocomplete_text_field.mm index 1fb8aa5..d0b61dd 100644 --- a/chrome/browser/cocoa/autocomplete_text_field.mm +++ b/chrome/browser/cocoa/autocomplete_text_field.mm @@ -15,13 +15,20 @@ return [AutocompleteTextFieldCell class]; } +- (void)dealloc { + [[NSNotificationCenter defaultCenter] removeObserver:self]; + [super dealloc]; +} + - (void)awakeFromNib { DCHECK([[self cell] isKindOfClass:[AutocompleteTextFieldCell class]]); } - (void)flagsChanged:(NSEvent*)theEvent { - bool controlFlag = ([theEvent modifierFlags]&NSControlKeyMask) != 0; - observer_->OnControlKeyChanged(controlFlag); + if (observer_) { + const bool controlFlag = ([theEvent modifierFlags]&NSControlKeyMask) != 0; + observer_->OnControlKeyChanged(controlFlag); + } } - (AutocompleteTextFieldCell*)autocompleteTextFieldCell { @@ -170,4 +177,65 @@ [undoManager_ removeAllActions]; } +// NOTE(shess): http://crbug.com/19116 describes a weird bug which +// happens when the user runs a Print panel on Leopard. After that, +// spurious -controlTextDidBeginEditing notifications are sent when an +// NSTextField is firstResponder, even though -currentEditor on that +// field returns nil. That notification caused significant problems +// in AutocompleteEditViewMac. -textDidBeginEditing: was NOT being +// sent in those cases, so this approach doesn't have the problem. +- (void)textDidBeginEditing:(NSNotification*)aNotification { + [super textDidBeginEditing:aNotification]; + if (observer_) { + observer_->OnDidBeginEditing(); + } +} + +- (void)textDidChange:(NSNotification *)aNotification { + [super textDidChange:aNotification]; + if (observer_) { + observer_->OnDidChange(); + } +} + +- (void)textDidEndEditing:(NSNotification *)aNotification { + [super textDidEndEditing:aNotification]; + if (observer_) { + observer_->OnDidEndEditing(); + } +} + +- (BOOL)textView:(NSTextView*)textView doCommandBySelector:(SEL)cmd { + if (observer_ && observer_->OnDoCommandBySelector(cmd)) { + return YES; + } + return [super textView:textView doCommandBySelector:cmd]; +} + +- (void)windowDidResignKey:(NSNotification*)notification { + DCHECK_EQ([self window], [notification object]); + if (observer_) { + observer_->OnDidResignKey(); + } +} + +- (void)viewWillMoveToWindow:(NSWindow*)newWindow { + if ([self window]) { + NSNotificationCenter* nc = [NSNotificationCenter defaultCenter]; + [nc removeObserver:self + name:NSWindowDidResignKeyNotification + object:[self window]]; + } +} + +- (void)viewDidMoveToWindow { + if ([self window]) { + NSNotificationCenter* nc = [NSNotificationCenter defaultCenter]; + [nc addObserver:self + selector:@selector(windowDidResignKey:) + name:NSWindowDidResignKeyNotification + object:[self window]]; + } +} + @end diff --git a/chrome/browser/cocoa/autocomplete_text_field_editor_unittest.mm b/chrome/browser/cocoa/autocomplete_text_field_editor_unittest.mm index a4eb9b9..27545bb 100644 --- a/chrome/browser/cocoa/autocomplete_text_field_editor_unittest.mm +++ b/chrome/browser/cocoa/autocomplete_text_field_editor_unittest.mm @@ -15,6 +15,7 @@ #include "testing/platform_test.h" using ::testing::Return; +using ::testing::StrictMock; namespace { @@ -53,7 +54,6 @@ class AutocompleteTextFieldEditorTest : public CocoaTest { [[AutocompleteTextField alloc] initWithFrame:frame]); field_ = field.get(); [field_ setStringValue:@"Testing"]; - [field_ setObserver:&field_observer_]; [[test_window() contentView] addSubview:field_]; // Arrange for |field_| to get the right field editor. @@ -80,7 +80,6 @@ class AutocompleteTextFieldEditorTest : public CocoaTest { AutocompleteTextFieldEditor* editor_; AutocompleteTextField* field_; - MockAutocompleteTextFieldObserver field_observer_; scoped_nsobject<AutocompleteTextFieldWindowTestDelegate> window_delegate_; private: @@ -89,6 +88,27 @@ class AutocompleteTextFieldEditorTest : public CocoaTest { TEST_VIEW(AutocompleteTextFieldEditorTest, field_); +// Base class for testing AutocompleteTextFieldObserver messages. +class AutocompleteTextFieldEditorObserverTest + : public AutocompleteTextFieldEditorTest { + public: + virtual void SetUp() { + AutocompleteTextFieldEditorTest::SetUp(); + [field_ setObserver:&field_observer_]; + } + + virtual void TearDown() { + // Clear the observer so that we don't show output for + // uninteresting messages to the mock (for instance, if |field_| has + // focus at the end of the test). + [field_ setObserver:NULL]; + + AutocompleteTextFieldEditorTest::TearDown(); + } + + StrictMock<MockAutocompleteTextFieldObserver> field_observer_; +}; + // Test that the field editor is linked in correctly. TEST_F(AutocompleteTextFieldEditorTest, FirstResponder) { EXPECT_EQ(editor_, [field_ currentEditor]); @@ -134,19 +154,19 @@ TEST_F(AutocompleteTextFieldEditorTest, Display) { } // Test that -paste: is correctly delegated to the observer. -TEST_F(AutocompleteTextFieldEditorTest, Paste) { +TEST_F(AutocompleteTextFieldEditorObserverTest, Paste) { EXPECT_CALL(field_observer_, OnPaste()); [editor_ paste:nil]; } // Test that -pasteAndGo: is correctly delegated to the observer. -TEST_F(AutocompleteTextFieldEditorTest, PasteAndGo) { +TEST_F(AutocompleteTextFieldEditorObserverTest, PasteAndGo) { EXPECT_CALL(field_observer_, OnPasteAndGo()); [editor_ pasteAndGo:nil]; } // Test that the menu is constructed correctly when CanPasteAndGo(). -TEST_F(AutocompleteTextFieldEditorTest, CanPasteAndGoMenu) { +TEST_F(AutocompleteTextFieldEditorObserverTest, CanPasteAndGoMenu) { EXPECT_CALL(field_observer_, CanPasteAndGo()) .WillOnce(Return(true)); EXPECT_CALL(field_observer_, GetPasteActionStringId()) @@ -169,7 +189,7 @@ TEST_F(AutocompleteTextFieldEditorTest, CanPasteAndGoMenu) { } // Test that the menu is constructed correctly when !CanPasteAndGo(). -TEST_F(AutocompleteTextFieldEditorTest, CannotPasteAndGoMenu) { +TEST_F(AutocompleteTextFieldEditorObserverTest, CannotPasteAndGoMenu) { EXPECT_CALL(field_observer_, CanPasteAndGo()) .WillOnce(Return(false)); @@ -190,7 +210,7 @@ TEST_F(AutocompleteTextFieldEditorTest, CannotPasteAndGoMenu) { // Test that the menu is constructed correctly when field isn't // editable. -TEST_F(AutocompleteTextFieldEditorTest, CanPasteAndGoMenuNotEditable) { +TEST_F(AutocompleteTextFieldEditorObserverTest, CanPasteAndGoMenuNotEditable) { [field_ setEditable:NO]; [editor_ setEditable:NO]; diff --git a/chrome/browser/cocoa/autocomplete_text_field_unittest.mm b/chrome/browser/cocoa/autocomplete_text_field_unittest.mm index 928d005..afbc474 100644 --- a/chrome/browser/cocoa/autocomplete_text_field_unittest.mm +++ b/chrome/browser/cocoa/autocomplete_text_field_unittest.mm @@ -14,18 +14,10 @@ #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/platform_test.h" -#import "third_party/ocmock/OCMock/OCMock.h" using ::testing::InSequence; - -// OCMock wants to mock a concrete class or protocol. This should -// provide a correct protocol for newer versions of the SDK, while -// providing something mockable for older versions. - -@protocol MockTextEditingDelegate<NSControlTextEditingDelegate> -- (void)controlTextDidBeginEditing:(NSNotification*)aNotification; -- (BOOL)control:(NSControl*)control textShouldEndEditing:(NSText*)fieldEditor; -@end +using ::testing::Return; +using ::testing::StrictMock; namespace { // Mock a SecurityImageView. @@ -78,7 +70,6 @@ class AutocompleteTextFieldTest : public CocoaTest { [[AutocompleteTextField alloc] initWithFrame:frame]); field_ = field.get(); [field_ setStringValue:@"Test test"]; - [field_ setObserver:&field_observer_]; [[test_window() contentView] addSubview:field_]; window_delegate_.reset( @@ -113,12 +104,31 @@ class AutocompleteTextFieldTest : public CocoaTest { } AutocompleteTextField* field_; - MockAutocompleteTextFieldObserver field_observer_; scoped_nsobject<AutocompleteTextFieldWindowTestDelegate> window_delegate_; }; TEST_VIEW(AutocompleteTextFieldTest, field_); +// Base class for testing AutocompleteTextFieldObserver messages. +class AutocompleteTextFieldObserverTest : public AutocompleteTextFieldTest { + public: + virtual void SetUp() { + AutocompleteTextFieldTest::SetUp(); + [field_ setObserver:&field_observer_]; + } + + virtual void TearDown() { + // Clear the observer so that we don't show output for + // uninteresting messages to the mock (for instance, if |field_| has + // focus at the end of the test). + [field_ setObserver:NULL]; + + AutocompleteTextFieldTest::TearDown(); + } + + StrictMock<MockAutocompleteTextFieldObserver> field_observer_; +}; + // Test that we have the right cell class. TEST_F(AutocompleteTextFieldTest, CellClass) { EXPECT_TRUE([[field_ cell] isKindOfClass:[AutocompleteTextFieldCell class]]); @@ -209,7 +219,7 @@ TEST_F(AutocompleteTextFieldTest, Display) { [field_ display]; } -TEST_F(AutocompleteTextFieldTest, FlagsChanged) { +TEST_F(AutocompleteTextFieldObserverTest, FlagsChanged) { InSequence dummy; // Call mock in exactly the order specified. // Test without Control key down, but some other modifier down. @@ -224,7 +234,7 @@ TEST_F(AutocompleteTextFieldTest, FlagsChanged) { // This test is here rather than in the editor's tests because the // field catches -flagsChanged: because it's on the responder chain, // the field editor doesn't implement it. -TEST_F(AutocompleteTextFieldTest, FieldEditorFlagsChanged) { +TEST_F(AutocompleteTextFieldObserverTest, FieldEditorFlagsChanged) { [test_window() makePretendKeyWindowAndSetFirstResponder:field_]; NSResponder* firstResponder = [[field_ window] firstResponder]; EXPECT_EQ(firstResponder, [field_ currentEditor]); @@ -241,7 +251,7 @@ TEST_F(AutocompleteTextFieldTest, FieldEditorFlagsChanged) { } // Frame size changes are propagated to |observer_|. -TEST_F(AutocompleteTextFieldTest, FrameChanged) { +TEST_F(AutocompleteTextFieldObserverTest, FrameChanged) { EXPECT_CALL(field_observer_, OnFrameChanged()); NSRect frame = [field_ frame]; frame.size.width += 10.0; @@ -345,60 +355,23 @@ TEST_F(AutocompleteTextFieldTest, ResetFieldEditorKeywordHint) { } // Test that resetting the field editor bounds does not cause untoward -// messages to the field's delegate. -TEST_F(AutocompleteTextFieldTest, ResetFieldEditorBlocksEndEditing) { - // First, test that -makeFirstResponder: sends - // -controlTextDidBeginEditing: and -control:textShouldEndEditing at - // the expected times. - { - id mockDelegate = - [OCMockObject mockForProtocol:@protocol(MockTextEditingDelegate)]; - - [field_ setDelegate:mockDelegate]; - - // Becoming first responder doesn't begin editing. - [test_window() makePretendKeyWindowAndSetFirstResponder:field_]; - NSTextView* editor = static_cast<NSTextView*>([field_ currentEditor]); - EXPECT_TRUE(nil != editor); - [mockDelegate verify]; - - // This should begin editing. - [[mockDelegate expect] controlTextDidBeginEditing:OCMOCK_ANY]; - [editor shouldChangeTextInRange:NSMakeRange(0, 0) replacementString:@""]; - [mockDelegate verify]; - - // Changing first responder ends editing. - BOOL yes = YES; - [[[mockDelegate expect] andReturnValue:OCMOCK_VALUE(yes)] - control:OCMOCK_ANY textShouldEndEditing:OCMOCK_ANY]; - [test_window() makePretendKeyWindowAndSetFirstResponder:field_]; - [mockDelegate verify]; - - [field_ setDelegate:nil]; - } - - // Test that -resetFieldEditorFrameIfNeeded manages to rearrange the - // editor without ending editing. - { - id mockDelegate = - [OCMockObject mockForProtocol:@protocol(MockTextEditingDelegate)]; - - [field_ setDelegate:mockDelegate]; - - // Start editing. - [[mockDelegate expect] controlTextDidBeginEditing:OCMOCK_ANY]; - NSTextView* editor = static_cast<NSTextView*>([field_ currentEditor]); - [editor shouldChangeTextInRange:NSMakeRange(0, 0) replacementString:@""]; - [mockDelegate verify]; +// messages to the field's observer. +TEST_F(AutocompleteTextFieldObserverTest, ResetFieldEditorContinuesEditing) { + // Becoming first responder doesn't begin editing. + [test_window() makePretendKeyWindowAndSetFirstResponder:field_]; + NSTextView* editor = static_cast<NSTextView*>([field_ currentEditor]); + EXPECT_TRUE(nil != editor); - // No more messages to mockDelegate. - AutocompleteTextFieldCell* cell = [field_ autocompleteTextFieldCell]; - [cell setSearchHintString:@"Type to search" availableWidth:kWidth]; - [field_ resetFieldEditorFrameIfNeeded]; - [mockDelegate verify]; + // This should begin editing and indicate a change. + EXPECT_CALL(field_observer_, OnDidBeginEditing()); + EXPECT_CALL(field_observer_, OnDidChange()); + [editor shouldChangeTextInRange:NSMakeRange(0, 0) replacementString:@""]; + [editor didChangeText]; - [field_ setDelegate:nil]; - } + // No messages to |field_observer_| when resetting the frame. + AutocompleteTextFieldCell* cell = [field_ autocompleteTextFieldCell]; + [cell setSearchHintString:@"Type to search" availableWidth:kWidth]; + [field_ resetFieldEditorFrameIfNeeded]; } // Clicking in the search hint should put the caret in the rightmost @@ -561,7 +534,7 @@ TEST_F(AutocompleteTextFieldTest, TripleClickSelectsAll) { EXPECT_EQ(selectedRange.length, [[field_ stringValue] length]); } -TEST_F(AutocompleteTextFieldTest, SecurityIconMouseDown) { +TEST_F(AutocompleteTextFieldObserverTest, SecurityIconMouseDown) { AutocompleteTextFieldCell* cell = [field_ autocompleteTextFieldCell]; MockSecurityImageView security_image_view(NULL, NULL); @@ -670,4 +643,52 @@ TEST_F(AutocompleteTextFieldTest, EditorGetsCorrectUndoManager) { EXPECT_TRUE(editor); EXPECT_EQ([field_ undoManagerForTextView:editor], [editor undoManager]); } + +TEST_F(AutocompleteTextFieldObserverTest, SendsEditingMessages) { + // Becoming first responder doesn't begin editing. + [test_window() makePretendKeyWindowAndSetFirstResponder:field_]; + NSTextView* editor = static_cast<NSTextView*>([field_ currentEditor]); + EXPECT_TRUE(nil != editor); + + // This should begin editing and indicate a change. + EXPECT_CALL(field_observer_, OnDidBeginEditing()); + EXPECT_CALL(field_observer_, OnDidChange()); + [editor shouldChangeTextInRange:NSMakeRange(0, 0) replacementString:@""]; + [editor didChangeText]; + + // Further changes don't send the begin message. + EXPECT_CALL(field_observer_, OnDidChange()); + [editor shouldChangeTextInRange:NSMakeRange(0, 0) replacementString:@""]; + [editor didChangeText]; + + // -doCommandBySelector: should forward to observer via |field_|. + // TODO(shess): Test with a fake arrow-key event? + const SEL cmd = @selector(moveDown:); + EXPECT_CALL(field_observer_, OnDoCommandBySelector(cmd)) + .WillOnce(Return(true)); + [editor doCommandBySelector:cmd]; + + // Finished with the changes. + EXPECT_CALL(field_observer_, OnDidEndEditing()); + [test_window() clearPretendKeyWindowAndFirstResponder]; +} + +// Test that the resign-key notification is forwarded right, and that +// the notification is registered and unregistered when the view moves +// in and out of the window. +// TODO(shess): Should this test the key window for realz? That would +// be really annoying to whoever is running the tests. +TEST_F(AutocompleteTextFieldObserverTest, SendsOnResignKey) { + EXPECT_CALL(field_observer_, OnDidResignKey()); + [test_window() resignKeyWindow]; + + scoped_nsobject<AutocompleteTextField> pin([field_ retain]); + [field_ removeFromSuperview]; + [test_window() resignKeyWindow]; + + [[test_window() contentView] addSubview:field_]; + EXPECT_CALL(field_observer_, OnDidResignKey()); + [test_window() resignKeyWindow]; +} + } // namespace diff --git a/chrome/browser/cocoa/autocomplete_text_field_unittest_helper.h b/chrome/browser/cocoa/autocomplete_text_field_unittest_helper.h index 301cf34..b055086 100644 --- a/chrome/browser/cocoa/autocomplete_text_field_unittest_helper.h +++ b/chrome/browser/cocoa/autocomplete_text_field_unittest_helper.h @@ -40,6 +40,11 @@ class MockAutocompleteTextFieldObserver : public AutocompleteTextFieldObserver { MOCK_METHOD0(GetPasteActionStringId, int()); MOCK_METHOD0(OnPasteAndGo, void()); MOCK_METHOD0(OnFrameChanged, void()); + MOCK_METHOD0(OnDidResignKey, void()); + MOCK_METHOD0(OnDidBeginEditing, void()); + MOCK_METHOD0(OnDidChange, void()); + MOCK_METHOD0(OnDidEndEditing, void()); + MOCK_METHOD1(OnDoCommandBySelector, bool(SEL cmd)); }; } // namespace diff --git a/chrome/browser/cocoa/location_bar_view_mac.mm b/chrome/browser/cocoa/location_bar_view_mac.mm index 4698e99..5390d20 100644 --- a/chrome/browser/cocoa/location_bar_view_mac.mm +++ b/chrome/browser/cocoa/location_bar_view_mac.mm @@ -121,7 +121,7 @@ void LocationBarViewMac::AcceptInput() { void LocationBarViewMac::AcceptInputWithDisposition( WindowOpenDisposition disposition) { - edit_view_->AcceptInput(disposition, false); + edit_view_->model()->AcceptInput(disposition, false); } void LocationBarViewMac::FocusLocation() { |