diff options
author | shess@chromium.org <shess@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-12-04 20:50:47 +0000 |
---|---|---|
committer | shess@chromium.org <shess@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-12-04 20:50:47 +0000 |
commit | fe0ecbe4c6b2a4b7422c65be1967fcb019f49342 (patch) | |
tree | eb9b5ceb8640801f0b80b433da331dad4a2f40b5 /chrome/browser | |
parent | 6cd3a5ffeb3b3707bee1b95374e03762d29a2711 (diff) | |
download | chromium_src-fe0ecbe4c6b2a4b7422c65be1967fcb019f49342.zip chromium_src-fe0ecbe4c6b2a4b7422c65be1967fcb019f49342.tar.gz chromium_src-fe0ecbe4c6b2a4b7422c65be1967fcb019f49342.tar.bz2 |
[Mac] Route edit-state messages via observer rather than delegate.
AutocompleteTextField routes messages to AutocompleteEditViewMac via a
Cocoa-style delegate and also a C++ observer class. The Cocoa-style
delegate has been gradually being pruned down in favor of the
observer, this CL completes the transformation.
The noted bugs occur because some bit of Cocoa code was sending
spurious delegate notifications. Since this code no longer depends on
those notifications, the spurious notifications are no longer a
problem for this case.
BUG=19116, 17803
TEST=Login to gmail, select a message, j, k, and other keys should work (and not send you to the omnibox). Bring up the Print panel and cancel. Bring up a new tab and close it. gmail's accelerator keys should still work right (not send focus to omnibox).
Review URL: http://codereview.chromium.org/391025
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@33853 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
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() { |