From f7359809aea977080efbbdcc91b6136d4a0c9de6 Mon Sep 17 00:00:00 2001 From: "csharp@chromium.org" Date: Fri, 27 Apr 2012 17:01:06 +0000 Subject: [PATCH] New Autofill UI properly changes Colour. Fixes a bug where the new Autofill UI would change the colour of an input element to yellow when showing a preview in it and fail to change the colour back once the preview was cleared. BUG=51644 TEST=When using the new Autofill UI, elements returns to their original colour when the preview ends. Review URL: http://codereview.chromium.org/10020055 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@134286 0039d316-1c4b-4281-b951-d872f2087c98 --- .../browser/autofill/autofill_external_delegate.cc | 8 ++++--- .../autofill_external_delegate_unittest.cc | 25 ++++++++++++++++++++++ chrome/browser/autofill/autofill_popup_view.cc | 2 +- chrome/renderer/autofill/autofill_agent.cc | 2 -- 4 files changed, 31 insertions(+), 6 deletions(-) diff --git a/chrome/browser/autofill/autofill_external_delegate.cc b/chrome/browser/autofill/autofill_external_delegate.cc index 98ec1b43a367..6f1f713d2646 100644 --- a/chrome/browser/autofill/autofill_external_delegate.cc +++ b/chrome/browser/autofill/autofill_external_delegate.cc @@ -37,14 +37,16 @@ AutofillExternalDelegate::AutofillExternalDelegate( void AutofillExternalDelegate::SelectAutofillSuggestionAtIndex(int unique_id, int list_index) { - if (unique_id == WebAutofillClient::MenuItemIDPasswordEntry) - return; - if (list_index == suggestions_options_index_ || list_index == suggestions_clear_index_ || unique_id == WebAutofillClient::MenuItemIDWarningMessage) return; + ClearPreviewedForm(); + + if (unique_id == WebAutofillClient::MenuItemIDPasswordEntry) + return; + FillAutofillFormData(unique_id, true); } diff --git a/chrome/browser/autofill/autofill_external_delegate_unittest.cc b/chrome/browser/autofill/autofill_external_delegate_unittest.cc index 1f1c918ea924..dabdaf8ff9e6 100644 --- a/chrome/browser/autofill/autofill_external_delegate_unittest.cc +++ b/chrome/browser/autofill/autofill_external_delegate_unittest.cc @@ -14,6 +14,7 @@ #include "content/test/test_browser_thread.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +#include "third_party/WebKit/Source/WebKit/chromium/public/WebAutofillClient.h" #include "ui/gfx/rect.h" #include "webkit/forms/form_data.h" #include "webkit/forms/form_field.h" @@ -22,6 +23,7 @@ using content::BrowserThread; using testing::_; using webkit::forms::FormData; using webkit::forms::FormField; +using WebKit::WebAutofillClient; namespace { @@ -45,6 +47,8 @@ class MockAutofillExternalDelegate : public TestAutofillExternalDelegate { const webkit::forms::FormField& field, const gfx::Rect& bounds)); + MOCK_METHOD0(ClearPreviewedForm, void()); + MOCK_METHOD0(HideAutofillPopup, void()); private: @@ -123,6 +127,9 @@ TEST_F(AutofillExternalDelegateUnitTest, TestExternalDelegateVirtualCalls) { EXPECT_CALL(*external_delegate_, HideAutofillPopup()); + // Called by DidAutofillSuggestions, add expectation to remove warning. + EXPECT_CALL(*autofill_manager_, OnFillAutofillFormData(_, _, _, _)); + // This should trigger a call to hide the popup since // we've selected an option. external_delegate_->DidAcceptAutofillSuggestions(autofill_item[0], @@ -140,3 +147,21 @@ TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateInvalidUniqueId) { EXPECT_CALL(*autofill_manager_, OnFillAutofillFormData(_, _, _, _)).Times(0); external_delegate_->DidAcceptAutofillSuggestions(string16(), -1, 0); } + +// Test that the ClearPreview IPC is only sent the form was being previewed +// (i.e. it isn't autofilling a password). +TEST_F(AutofillExternalDelegateUnitTest, ExternalDelegateClearPreviewedForm) { + // Called by SelectAutofillSuggestionAtIndex, add expectation to remove + // warning. + EXPECT_CALL(*autofill_manager_, OnFillAutofillFormData(_, _, _, _)); + + // Ensure selecting a new password entries or Autofill entries will + // cause any previews to get cleared. + EXPECT_CALL(*external_delegate_, ClearPreviewedForm()).Times(1); + external_delegate_->SelectAutofillSuggestionAtIndex( + WebAutofillClient::MenuItemIDPasswordEntry, + 0); + + EXPECT_CALL(*external_delegate_, ClearPreviewedForm()).Times(1); + external_delegate_->SelectAutofillSuggestionAtIndex(1, 0); +} diff --git a/chrome/browser/autofill/autofill_popup_view.cc b/chrome/browser/autofill/autofill_popup_view.cc index 657fc4b7640d..17e94d548328 100644 --- a/chrome/browser/autofill/autofill_popup_view.cc +++ b/chrome/browser/autofill/autofill_popup_view.cc @@ -76,7 +76,7 @@ void AutofillPopupView::SetSelectedLine(int selected_line) { if (selected_line_ != kNoSelection) { external_delegate_->SelectAutofillSuggestionAtIndex( autofill_unique_ids_[selected_line_], - selected_line); + selected_line_); } } diff --git a/chrome/renderer/autofill/autofill_agent.cc b/chrome/renderer/autofill/autofill_agent.cc index 0b28d452e6dd..73baa1f81f02 100644 --- a/chrome/renderer/autofill/autofill_agent.cc +++ b/chrome/renderer/autofill/autofill_agent.cc @@ -440,8 +440,6 @@ void AutofillAgent::OnFormDataFilled(int query_id, base::TimeTicks::Now())); break; case AUTOFILL_PREVIEW: - didClearAutofillSelection(element_); - PreviewForm(form, element_); Send(new AutofillHostMsg_DidPreviewAutofillFormData(routing_id())); break; -- 2.11.4.GIT