From 29e75de051f2dfb32e896b359e605e869f9db04e Mon Sep 17 00:00:00 2001 From: "finnur@chromium.org" Date: Fri, 27 Feb 2009 22:02:15 +0000 Subject: Fix 8200: Pressing Esc should cancel dialogs, not commit. When we create the OK & Cancel dialog buttons in DialogClientView, we were registering OK as a handler for Escape, even though the dialog had a Cancel button. This is because we register for Esc if the member variable |cancel_ button_| is NULL. Problem is, that member isn't created until later on in that function. We should be checking the |buttons| flags instead. TEST=Open the Clear Browsing Data dialog, press Tab twice, press Esc and make sure that the dialog closes and nothing is cleared (from whatever checkbox was selected). BUG=8200 Review URL: http://codereview.chromium.org/28267 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@10631 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/views/dialog_client_view.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'chrome') diff --git a/chrome/views/dialog_client_view.cc b/chrome/views/dialog_client_view.cc index 58618b2..c32b17c 100644 --- a/chrome/views/dialog_client_view.cc +++ b/chrome/views/dialog_client_view.cc @@ -67,7 +67,7 @@ class DialogButton : public NativeButton { Window* owner_; const DialogDelegate::DialogButton type_; - DISALLOW_EVIL_CONSTRUCTORS(DialogButton); + DISALLOW_COPY_AND_ASSIGN(DialogButton); }; } // namespace @@ -114,7 +114,7 @@ void DialogClientView::ShowDialogButtons() { ok_button_->SetGroup(kButtonGroup); if (is_default_button) default_button_ = ok_button_; - if (!cancel_button_) + if (!(buttons & DialogDelegate::DIALOGBUTTON_CANCEL)) ok_button_->AddAccelerator(Accelerator(VK_ESCAPE, false, false, false)); AddChildView(ok_button_); } @@ -273,7 +273,7 @@ void DialogClientView::ViewHierarchyChanged(bool is_add, View* parent, // Listen for focus change events so we can update the default button. DCHECK(focus_manager); // bug #1291225: crash reports seem to indicate it // can be NULL. - if (focus_manager) + if (focus_manager) focus_manager->AddFocusChangeListener(this); // The "extra view" must be created and installed after the contents view -- cgit v1.1