Commit 15be5845 authored by Mohamed Amir Yosef's avatar Mohamed Amir Yosef Committed by Commit Bot

[Passwords] Change Bubble Title for Save<->Update Password Transitions

Details are in crbug.com/1090740

screencast pre-Butter:
https://screencast.googleplex.com/cast/NTUyNjczMjc1NDkxMTIzMnw3MjBkYjI2Mi05Ng

screencast after-Butter:
https://screencast.googleplex.com/cast/NTcyNTgzODU4MDM4Mzc0NHwxNjMzOGFmZC0yNw

Bug: 1044038,1090740
Change-Id: I202ca32992bea93b04d378203fbe140cdf29126b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2228883
Commit-Queue: Mohamed Amir Yosef <mamir@chromium.org>
Reviewed-by: default avatarVasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775754}
parent a4420186
...@@ -133,16 +133,6 @@ SaveUpdateBubbleController::SaveUpdateBubbleController( ...@@ -133,16 +133,6 @@ SaveUpdateBubbleController::SaveUpdateBubbleController(
enable_editing_ = delegate_->GetCredentialSource() != enable_editing_ = delegate_->GetCredentialSource() !=
password_manager::metrics_util::CredentialSourceType:: password_manager::metrics_util::CredentialSourceType::
kCredentialManagementAPI; kCredentialManagementAPI;
// Compute the title.
PasswordTitleType type =
state_ == password_manager::ui::PENDING_PASSWORD_UPDATE_STATE
? PasswordTitleType::UPDATE_PASSWORD
: (pending_password_.federation_origin.opaque()
? PasswordTitleType::SAVE_PASSWORD
: PasswordTitleType::SAVE_ACCOUNT);
title_ = GetSavePasswordDialogTitleText(GetWebContents()->GetVisibleURL(),
origin_, type);
} }
SaveUpdateBubbleController::~SaveUpdateBubbleController() { SaveUpdateBubbleController::~SaveUpdateBubbleController() {
...@@ -213,7 +203,6 @@ bool SaveUpdateBubbleController::ReplaceToShowPromotionIfNeeded() { ...@@ -213,7 +203,6 @@ bool SaveUpdateBubbleController::ReplaceToShowPromotionIfNeeded() {
if (password_bubble_experiment::ShouldShowChromeSignInPasswordPromo( if (password_bubble_experiment::ShouldShowChromeSignInPasswordPromo(
prefs, sync_service)) { prefs, sync_service)) {
ReportInteractions(); ReportInteractions();
title_ = l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_SYNC_PROMO_TITLE);
state_ = password_manager::ui::CHROME_SIGN_IN_PROMO_STATE; state_ = password_manager::ui::CHROME_SIGN_IN_PROMO_STATE;
int show_count = prefs->GetInteger( int show_count = prefs->GetInteger(
password_manager::prefs::kNumberSignInPasswordPromoShown); password_manager::prefs::kNumberSignInPasswordPromoShown);
...@@ -234,7 +223,16 @@ bool SaveUpdateBubbleController::RevealPasswords() { ...@@ -234,7 +223,16 @@ bool SaveUpdateBubbleController::RevealPasswords() {
} }
base::string16 SaveUpdateBubbleController::GetTitle() const { base::string16 SaveUpdateBubbleController::GetTitle() const {
return title_; if (state_ == password_manager::ui::CHROME_SIGN_IN_PROMO_STATE)
return l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_SYNC_PROMO_TITLE);
PasswordTitleType type = IsCurrentStateUpdate()
? PasswordTitleType::UPDATE_PASSWORD
: (pending_password_.federation_origin.opaque()
? PasswordTitleType::SAVE_PASSWORD
: PasswordTitleType::SAVE_ACCOUNT);
return GetSavePasswordDialogTitleText(GetWebContents()->GetVisibleURL(),
origin_, type);
} }
void SaveUpdateBubbleController::ReportInteractions() { void SaveUpdateBubbleController::ReportInteractions() {
......
...@@ -95,7 +95,6 @@ class SaveUpdateBubbleController : public PasswordBubbleControllerBase { ...@@ -95,7 +95,6 @@ class SaveUpdateBubbleController : public PasswordBubbleControllerBase {
// Origin of the page from where this bubble was triggered. // Origin of the page from where this bubble was triggered.
url::Origin origin_; url::Origin origin_;
password_manager::ui::State state_; password_manager::ui::State state_;
base::string16 title_;
autofill::PasswordForm pending_password_; autofill::PasswordForm pending_password_;
std::vector<autofill::PasswordForm> local_credentials_; std::vector<autofill::PasswordForm> local_credentials_;
password_manager::InteractionsStats interaction_stats_; password_manager::InteractionsStats interaction_stats_;
......
...@@ -139,16 +139,6 @@ SaveUpdateWithAccountStoreBubbleController:: ...@@ -139,16 +139,6 @@ SaveUpdateWithAccountStoreBubbleController::
enable_editing_ = delegate_->GetCredentialSource() != enable_editing_ = delegate_->GetCredentialSource() !=
password_manager::metrics_util::CredentialSourceType:: password_manager::metrics_util::CredentialSourceType::
kCredentialManagementAPI; kCredentialManagementAPI;
// Compute the title.
PasswordTitleType type =
state_ == password_manager::ui::PENDING_PASSWORD_UPDATE_STATE
? PasswordTitleType::UPDATE_PASSWORD
: (pending_password_.federation_origin.opaque()
? PasswordTitleType::SAVE_PASSWORD
: PasswordTitleType::SAVE_ACCOUNT);
title_ = GetSavePasswordDialogTitleText(GetWebContents()->GetVisibleURL(),
origin_, type);
} }
SaveUpdateWithAccountStoreBubbleController:: SaveUpdateWithAccountStoreBubbleController::
...@@ -283,6 +273,16 @@ SaveUpdateWithAccountStoreBubbleController::GetPrimaryAccountAvatar( ...@@ -283,6 +273,16 @@ SaveUpdateWithAccountStoreBubbleController::GetPrimaryAccountAvatar(
icon_size_dip, profiles::SHAPE_CIRCLE)); icon_size_dip, profiles::SHAPE_CIRCLE));
} }
base::string16 SaveUpdateWithAccountStoreBubbleController::GetTitle() const {
PasswordTitleType type = IsCurrentStateUpdate()
? PasswordTitleType::UPDATE_PASSWORD
: (pending_password_.federation_origin.opaque()
? PasswordTitleType::SAVE_PASSWORD
: PasswordTitleType::SAVE_ACCOUNT);
return GetSavePasswordDialogTitleText(GetWebContents()->GetVisibleURL(),
origin_, type);
}
void SaveUpdateWithAccountStoreBubbleController::ReportInteractions() { void SaveUpdateWithAccountStoreBubbleController::ReportInteractions() {
DCHECK(state_ == password_manager::ui::PENDING_PASSWORD_UPDATE_STATE || DCHECK(state_ == password_manager::ui::PENDING_PASSWORD_UPDATE_STATE ||
state_ == password_manager::ui::PENDING_PASSWORD_STATE); state_ == password_manager::ui::PENDING_PASSWORD_STATE);
...@@ -326,7 +326,3 @@ void SaveUpdateWithAccountStoreBubbleController::ReportInteractions() { ...@@ -326,7 +326,3 @@ void SaveUpdateWithAccountStoreBubbleController::ReportInteractions() {
if (metrics_recorder_) if (metrics_recorder_)
metrics_recorder_->RecordUIDismissalReason(dismissal_reason_); metrics_recorder_->RecordUIDismissalReason(dismissal_reason_);
} }
base::string16 SaveUpdateWithAccountStoreBubbleController::GetTitle() const {
return title_;
}
...@@ -80,6 +80,9 @@ class SaveUpdateWithAccountStoreBubbleController ...@@ -80,6 +80,9 @@ class SaveUpdateWithAccountStoreBubbleController
// account is signed in. // account is signed in.
ui::ImageModel GetPrimaryAccountAvatar(int icon_size_dip); ui::ImageModel GetPrimaryAccountAvatar(int icon_size_dip);
// PasswordBubbleControllerBase methods:
base::string16 GetTitle() const override;
password_manager::ui::State state() const { return state_; } password_manager::ui::State state() const { return state_; }
const autofill::PasswordForm& pending_password() const { const autofill::PasswordForm& pending_password() const {
...@@ -106,13 +109,11 @@ class SaveUpdateWithAccountStoreBubbleController ...@@ -106,13 +109,11 @@ class SaveUpdateWithAccountStoreBubbleController
private: private:
// PasswordBubbleControllerBase methods: // PasswordBubbleControllerBase methods:
base::string16 GetTitle() const override;
void ReportInteractions() override; void ReportInteractions() override;
// Origin of the page from where this bubble was triggered. // Origin of the page from where this bubble was triggered.
url::Origin origin_; url::Origin origin_;
password_manager::ui::State state_; password_manager::ui::State state_;
base::string16 title_;
autofill::PasswordForm pending_password_; autofill::PasswordForm pending_password_;
std::vector<autofill::PasswordForm> existing_credentials_; std::vector<autofill::PasswordForm> existing_credentials_;
password_manager::InteractionsStats interaction_stats_; password_manager::InteractionsStats interaction_stats_;
......
...@@ -43,6 +43,11 @@ ...@@ -43,6 +43,11 @@
namespace { namespace {
// Controls whether we should update the bubble title when transitioning
// between Save and Update states.
const base::Feature kUpdatePasswordSaveUpdateBubbleTitle{
"UpdatePasswordSaveUpdateBubbleTitle", base::FEATURE_ENABLED_BY_DEFAULT};
enum PasswordSaveUpdateViewColumnSetType { enum PasswordSaveUpdateViewColumnSetType {
// | | (LEADING, FILL) | | (FILL, FILL) | | // | | (LEADING, FILL) | | (FILL, FILL) | |
// Used for the username/password line of the bubble, for the pending view. // Used for the username/password line of the bubble, for the pending view.
...@@ -311,7 +316,7 @@ PasswordSaveUpdateView::PasswordSaveUpdateView( ...@@ -311,7 +316,7 @@ PasswordSaveUpdateView::PasswordSaveUpdateView(
SetFootnoteView(CreateFooterView()); SetFootnoteView(CreateFooterView());
SetCancelCallback(base::BindOnce(&PasswordSaveUpdateView::OnDialogCancelled, SetCancelCallback(base::BindOnce(&PasswordSaveUpdateView::OnDialogCancelled,
base::Unretained(this))); base::Unretained(this)));
UpdateDialogButtons(); UpdateBubbleUIElements();
} }
views::View* PasswordSaveUpdateView::GetUsernameTextfieldForTest() const { views::View* PasswordSaveUpdateView::GetUsernameTextfieldForTest() const {
...@@ -364,7 +369,7 @@ void PasswordSaveUpdateView::OnContentChanged( ...@@ -364,7 +369,7 @@ void PasswordSaveUpdateView::OnContentChanged(
if (is_update_state_before != controller_.IsCurrentStateUpdate() || if (is_update_state_before != controller_.IsCurrentStateUpdate() ||
is_ok_button_enabled_before != is_ok_button_enabled_before !=
IsDialogButtonEnabled(ui::DIALOG_BUTTON_OK)) { IsDialogButtonEnabled(ui::DIALOG_BUTTON_OK)) {
UpdateDialogButtons(); UpdateBubbleUIElements();
DialogModelChanged(); DialogModelChanged();
// TODO(ellyjones): This should not be necessary; DialogModelChanged() // TODO(ellyjones): This should not be necessary; DialogModelChanged()
// implies a re-layout of the dialog. // implies a re-layout of the dialog.
...@@ -467,14 +472,14 @@ void PasswordSaveUpdateView::ReplaceWithPromo() { ...@@ -467,14 +472,14 @@ void PasswordSaveUpdateView::ReplaceWithPromo() {
} }
GetWidget()->UpdateWindowIcon(); GetWidget()->UpdateWindowIcon();
SetTitle(controller_.GetTitle()); SetTitle(controller_.GetTitle());
UpdateDialogButtons(); UpdateBubbleUIElements();
DialogModelChanged(); DialogModelChanged();
SizeToContents(); SizeToContents();
#endif // defined(OS_CHROMEOS) #endif // defined(OS_CHROMEOS)
} }
void PasswordSaveUpdateView::UpdateDialogButtons() { void PasswordSaveUpdateView::UpdateBubbleUIElements() {
if (sign_in_promo_) { if (sign_in_promo_) {
SetButtons(ui::DIALOG_BUTTON_NONE); SetButtons(ui::DIALOG_BUTTON_NONE);
return; return;
...@@ -490,6 +495,9 @@ void PasswordSaveUpdateView::UpdateDialogButtons() { ...@@ -490,6 +495,9 @@ void PasswordSaveUpdateView::UpdateDialogButtons() {
l10n_util::GetStringUTF16( l10n_util::GetStringUTF16(
is_update_bubble_ ? IDS_PASSWORD_MANAGER_CANCEL_BUTTON is_update_bubble_ ? IDS_PASSWORD_MANAGER_CANCEL_BUTTON
: IDS_PASSWORD_MANAGER_BUBBLE_BLACKLIST_BUTTON)); : IDS_PASSWORD_MANAGER_BUBBLE_BLACKLIST_BUTTON));
if (base::FeatureList::IsEnabled(kUpdatePasswordSaveUpdateBubbleTitle))
SetTitle(controller_.GetTitle());
} }
std::unique_ptr<views::View> PasswordSaveUpdateView::CreateFooterView() { std::unique_ptr<views::View> PasswordSaveUpdateView::CreateFooterView() {
......
...@@ -65,7 +65,7 @@ class PasswordSaveUpdateView : public PasswordBubbleViewBase, ...@@ -65,7 +65,7 @@ class PasswordSaveUpdateView : public PasswordBubbleViewBase,
void TogglePasswordVisibility(); void TogglePasswordVisibility();
void UpdateUsernameAndPasswordInModel(); void UpdateUsernameAndPasswordInModel();
void ReplaceWithPromo(); void ReplaceWithPromo();
void UpdateDialogButtons(); void UpdateBubbleUIElements();
std::unique_ptr<views::View> CreateFooterView(); std::unique_ptr<views::View> CreateFooterView();
void OnDialogCancelled(); void OnDialogCancelled();
......
...@@ -447,7 +447,7 @@ PasswordSaveUpdateWithAccountStoreView::PasswordSaveUpdateWithAccountStoreView( ...@@ -447,7 +447,7 @@ PasswordSaveUpdateWithAccountStoreView::PasswordSaveUpdateWithAccountStoreView(
} }
SetFootnoteView(CreateFooterView()); SetFootnoteView(CreateFooterView());
UpdateDialogButtonsAndAccountPickerVisiblity(); UpdateBubbleUIElements();
} }
PasswordSaveUpdateWithAccountStoreView:: PasswordSaveUpdateWithAccountStoreView::
...@@ -487,7 +487,7 @@ void PasswordSaveUpdateWithAccountStoreView::OnContentChanged( ...@@ -487,7 +487,7 @@ void PasswordSaveUpdateWithAccountStoreView::OnContentChanged(
if (is_update_state_before != controller_.IsCurrentStateUpdate() || if (is_update_state_before != controller_.IsCurrentStateUpdate() ||
is_ok_button_enabled_before != is_ok_button_enabled_before !=
IsDialogButtonEnabled(ui::DIALOG_BUTTON_OK)) { IsDialogButtonEnabled(ui::DIALOG_BUTTON_OK)) {
UpdateDialogButtonsAndAccountPickerVisiblity(); UpdateBubbleUIElements();
DialogModelChanged(); DialogModelChanged();
} }
} }
...@@ -570,8 +570,7 @@ void PasswordSaveUpdateWithAccountStoreView:: ...@@ -570,8 +570,7 @@ void PasswordSaveUpdateWithAccountStoreView::
std::move(new_password)); std::move(new_password));
} }
void PasswordSaveUpdateWithAccountStoreView:: void PasswordSaveUpdateWithAccountStoreView::UpdateBubbleUIElements() {
UpdateDialogButtonsAndAccountPickerVisiblity() {
SetButtons((ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL)); SetButtons((ui::DIALOG_BUTTON_OK | ui::DIALOG_BUTTON_CANCEL));
SetButtonLabel( SetButtonLabel(
ui::DIALOG_BUTTON_OK, ui::DIALOG_BUTTON_OK,
...@@ -583,6 +582,9 @@ void PasswordSaveUpdateWithAccountStoreView:: ...@@ -583,6 +582,9 @@ void PasswordSaveUpdateWithAccountStoreView::
l10n_util::GetStringUTF16( l10n_util::GetStringUTF16(
is_update_bubble_ ? IDS_PASSWORD_MANAGER_CANCEL_BUTTON is_update_bubble_ ? IDS_PASSWORD_MANAGER_CANCEL_BUTTON
: IDS_PASSWORD_MANAGER_BUBBLE_BLACKLIST_BUTTON)); : IDS_PASSWORD_MANAGER_BUBBLE_BLACKLIST_BUTTON));
SetTitle(controller_.GetTitle());
// Nothing to do if the bubble isn't visible yet. // Nothing to do if the bubble isn't visible yet.
if (!GetWidget()) if (!GetWidget())
return; return;
......
...@@ -72,7 +72,7 @@ class PasswordSaveUpdateWithAccountStoreView ...@@ -72,7 +72,7 @@ class PasswordSaveUpdateWithAccountStoreView
void TogglePasswordVisibility(); void TogglePasswordVisibility();
void UpdateUsernameAndPasswordInModel(); void UpdateUsernameAndPasswordInModel();
void UpdateDialogButtonsAndAccountPickerVisiblity(); void UpdateBubbleUIElements();
std::unique_ptr<views::View> CreateFooterView(); std::unique_ptr<views::View> CreateFooterView();
SaveUpdateWithAccountStoreBubbleController controller_; SaveUpdateWithAccountStoreBubbleController controller_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment