Commit af417440 authored by Pavel Yatsuk's avatar Pavel Yatsuk Committed by Chromium LUCI CQ

[Messages] Adjust Save Password message description for signed in users

This CL adjusts Save Password message title and description to reflect
feedback from privacy team:
http://doc/1FxSQbCL-zqUsTJaEodEvZ7D8ENxE3lzg5_e7ZEsu8Uk#heading=h.vgwmyj43xgsb

This CL adds strings for both options. Only one is currently used. Once
we make a decision I'll clean up unused option.

BUG=1166841
R=twellington@chromium.org

Change-Id: I762c6dbc5e99f9c5fee574321be2750d5d61509f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2630447Reviewed-by: default avatarTheresa  <twellington@chromium.org>
Commit-Queue: Pavel Yatsuk <pavely@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843807}
parent 6e6ed678
...@@ -4937,11 +4937,11 @@ Keep your key file in a safe place. You will need it to create new versions of y ...@@ -4937,11 +4937,11 @@ Keep your key file in a safe place. You will need it to create new versions of y
<message name="IDS_UPDATE_PASSWORD_DIFFERENT_DOMAINS_TITLE" desc="The title of the update password bubble when the submitted form origin isn't equal to saved credentials origin."> <message name="IDS_UPDATE_PASSWORD_DIFFERENT_DOMAINS_TITLE" desc="The title of the update password bubble when the submitted form origin isn't equal to saved credentials origin.">
Update password for <ph name="ORIGIN">$1<ex>example.com</ex></ph>? Update password for <ph name="ORIGIN">$1<ex>example.com</ex></ph>?
</message> </message>
<message name="IDS_SAVE_PASSWORD_TO_GOOGLE" desc="The title of the save password bubble when the user syncs passswors to thir Google account."> <message name="IDS_SAVE_PASSWORD_SIGNED_IN_MESSAGE_DESCRIPTION_GOOGLE_ACCOUNT" desc="The description of save password message when the user is signed in.">
Save password to your Google Account? <ph name="USERNAME">$1<ex>Username</ex></ph> <ph name="MASKED_PASSWORD">$2<ex>*****</ex></ph> will be saved to your Google Account, <ph name="ACCOUNT">$3<ex>user@gmail.com</ex></ph>
</message> </message>
<message name="IDS_SAVE_ACCOUNT_TO_GOOGLE" desc="The title of the save password bubble when a federated credential can be saved to the user's Google account."> <message name="IDS_SAVE_PASSWORD_SIGNED_IN_MESSAGE_DESCRIPTION" desc="The alternative description of save password message when the user is signed in.">
Save username to your Google Account? <ph name="USERNAME">$1<ex>Username</ex></ph> <ph name="MASKED_PASSWORD">$2<ex>*****</ex></ph> will be saved to <ph name="ACCOUNT">$3<ex>user@gmail.com</ex></ph>
</message> </message>
<message name="IDS_SAVE_PASSWORD_FOOTER" desc="The footer text of the bubble that offers user to save a password to Chrome."> <message name="IDS_SAVE_PASSWORD_FOOTER" desc="The footer text of the bubble that offers user to save a password to Chrome.">
Passwords are saved in your Google Account so you can use them on any device Passwords are saved in your Google Account so you can use them on any device
......
d82dbe45f91b7573060266d89e8d5d7a91923458
\ No newline at end of file
96800ef4fe9e6e5d01d64263d50de3e76b40e39a
\ No newline at end of file
b53a8f296fc56d2e618e646ced5095d95857fefc
\ No newline at end of file
...@@ -45,4 +45,19 @@ base::Optional<AccountInfo> GetAccountInfoForPasswordInfobars(Profile* profile, ...@@ -45,4 +45,19 @@ base::Optional<AccountInfo> GetAccountInfoForPasswordInfobars(Profile* profile,
return should_show_account_footer ? account_info : base::nullopt; return should_show_account_footer ? account_info : base::nullopt;
} }
base::Optional<AccountInfo> GetAccountInfoForPasswordMessages(Profile* profile,
bool is_syncing) {
DCHECK(profile);
if (!is_syncing)
return base::nullopt;
signin::IdentityManager* identity_manager =
IdentityManagerFactory::GetForProfile(profile);
CoreAccountId account_id =
identity_manager->GetPrimaryAccountId(signin::ConsentLevel::kSync);
return identity_manager
->FindExtendedAccountInfoForAccountWithRefreshTokenByAccountId(
account_id);
}
} // namespace password_manager } // namespace password_manager
...@@ -14,6 +14,9 @@ namespace password_manager { ...@@ -14,6 +14,9 @@ namespace password_manager {
base::Optional<AccountInfo> GetAccountInfoForPasswordInfobars(Profile* profile, base::Optional<AccountInfo> GetAccountInfoForPasswordInfobars(Profile* profile,
bool is_syncing); bool is_syncing);
base::Optional<AccountInfo> GetAccountInfoForPasswordMessages(Profile* profile,
bool is_syncing);
} // namespace password_manager } // namespace password_manager
#endif // CHROME_BROWSER_PASSWORD_MANAGER_ANDROID_PASSWORD_INFOBAR_UTILS_H_ #endif // CHROME_BROWSER_PASSWORD_MANAGER_ANDROID_PASSWORD_INFOBAR_UTILS_H_
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "chrome/browser/android/android_theme_resources.h" #include "chrome/browser/android/android_theme_resources.h"
#include "chrome/browser/android/resource_mapper.h" #include "chrome/browser/android/resource_mapper.h"
#include "chrome/browser/password_manager/android/password_infobar_utils.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/sync/profile_sync_service_factory.h" #include "chrome/browser/sync/profile_sync_service_factory.h"
#include "chrome/grit/generated_resources.h" #include "chrome/grit/generated_resources.h"
...@@ -31,18 +32,21 @@ void SavePasswordMessageDelegate::DisplaySavePasswordPrompt( ...@@ -31,18 +32,21 @@ void SavePasswordMessageDelegate::DisplaySavePasswordPrompt(
DismissSavePasswordPrompt(); DismissSavePasswordPrompt();
DCHECK(message_ == nullptr); DCHECK(message_ == nullptr);
Profile* profile =
Profile::FromBrowserContext(web_contents->GetBrowserContext());
// is_saving_google_account indicates whether the user is syncing // is_saving_google_account indicates whether the user is syncing
// passwords to their Google Account. // passwords to their Google Account.
const bool is_saving_google_account = const bool is_saving_google_account =
password_bubble_experiment::IsSmartLockUser( password_bubble_experiment::IsSmartLockUser(
ProfileSyncServiceFactory::GetForProfile( ProfileSyncServiceFactory::GetForProfile(profile));
Profile::FromBrowserContext(web_contents->GetBrowserContext())));
base::Optional<AccountInfo> account_info =
password_manager::GetAccountInfoForPasswordMessages(
profile, is_saving_google_account);
// All the DisplaySavePasswordPrompt parameters are passed to CreateMessage to // All the DisplaySavePasswordPrompt parameters are passed to CreateMessage to
// avoid a call to MessageDispatcherBridge::EnqueueMessage from test while // avoid a call to MessageDispatcherBridge::EnqueueMessage from test while
// still providing decent test coverage. // still providing decent test coverage.
CreateMessage(web_contents, std::move(form_to_save), CreateMessage(web_contents, std::move(form_to_save), account_info);
is_saving_google_account);
messages::MessageDispatcherBridge::EnqueueMessage(message_.get(), messages::MessageDispatcherBridge::EnqueueMessage(message_.get(),
web_contents_); web_contents_);
...@@ -58,7 +62,7 @@ void SavePasswordMessageDelegate::DismissSavePasswordPrompt() { ...@@ -58,7 +62,7 @@ void SavePasswordMessageDelegate::DismissSavePasswordPrompt() {
void SavePasswordMessageDelegate::CreateMessage( void SavePasswordMessageDelegate::CreateMessage(
content::WebContents* web_contents, content::WebContents* web_contents,
std::unique_ptr<password_manager::PasswordFormManagerForUI> form_to_save, std::unique_ptr<password_manager::PasswordFormManagerForUI> form_to_save,
bool is_saving_google_account) { base::Optional<AccountInfo> account_info) {
ui_dismissal_reason_ = password_manager::metrics_util::NO_DIRECT_INTERACTION; ui_dismissal_reason_ = password_manager::metrics_util::NO_DIRECT_INTERACTION;
web_contents_ = web_contents; web_contents_ = web_contents;
form_to_save_ = std::move(form_to_save); form_to_save_ = std::move(form_to_save);
...@@ -75,20 +79,26 @@ void SavePasswordMessageDelegate::CreateMessage( ...@@ -75,20 +79,26 @@ void SavePasswordMessageDelegate::CreateMessage(
const password_manager::PasswordForm& pending_credentials = const password_manager::PasswordForm& pending_credentials =
form_to_save_->GetPendingCredentials(); form_to_save_->GetPendingCredentials();
int title_message_id = 0; int title_message_id = pending_credentials.federation_origin.opaque()
if (!pending_credentials.federation_origin.opaque()) { ? IDS_SAVE_PASSWORD
title_message_id = is_saving_google_account ? IDS_SAVE_ACCOUNT_TO_GOOGLE : IDS_SAVE_ACCOUNT;
: IDS_SAVE_ACCOUNT;
} else {
title_message_id = is_saving_google_account ? IDS_SAVE_PASSWORD_TO_GOOGLE
: IDS_SAVE_PASSWORD;
}
message_->SetTitle(l10n_util::GetStringUTF16(title_message_id)); message_->SetTitle(l10n_util::GetStringUTF16(title_message_id));
base::string16 description = pending_credentials.username_value; const base::string16 masked_password =
description.append(base::ASCIIToUTF16(" ")) base::string16(pending_credentials.password_value.size(), L'•');
.append(pending_credentials.password_value.size(), L'•'); base::string16 description;
if (account_info.has_value()) {
description = l10n_util::GetStringFUTF16(
IDS_SAVE_PASSWORD_SIGNED_IN_MESSAGE_DESCRIPTION,
pending_credentials.username_value, masked_password,
base::UTF8ToUTF16(account_info.value().email));
} else {
description.append(pending_credentials.username_value)
.append(base::ASCIIToUTF16(" "))
.append(masked_password);
}
message_->SetDescription(description); message_->SetDescription(description);
message_->SetPrimaryButtonText( message_->SetPrimaryButtonText(
......
...@@ -7,9 +7,11 @@ ...@@ -7,9 +7,11 @@
#include <memory> #include <memory>
#include "base/optional.h"
#include "components/messages/android/message_wrapper.h" #include "components/messages/android/message_wrapper.h"
#include "components/password_manager/core/browser/password_form_manager_for_ui.h" #include "components/password_manager/core/browser/password_form_manager_for_ui.h"
#include "components/password_manager/core/browser/password_manager_metrics_util.h" #include "components/password_manager/core/browser/password_manager_metrics_util.h"
#include "components/signin/public/identity_manager/account_info.h"
namespace content { namespace content {
class WebContents; class WebContents;
...@@ -38,7 +40,7 @@ class SavePasswordMessageDelegate { ...@@ -38,7 +40,7 @@ class SavePasswordMessageDelegate {
void CreateMessage( void CreateMessage(
content::WebContents* web_contents, content::WebContents* web_contents,
std::unique_ptr<password_manager::PasswordFormManagerForUI> form_to_save, std::unique_ptr<password_manager::PasswordFormManagerForUI> form_to_save,
bool is_saving_google_account); base::Optional<AccountInfo> account_info);
// Called in response to user clicking "Save" and "Never" buttons. // Called in response to user clicking "Save" and "Never" buttons.
void HandleSaveClick(); void HandleSaveClick();
......
...@@ -31,6 +31,7 @@ namespace { ...@@ -31,6 +31,7 @@ namespace {
constexpr char kDefaultUrl[] = "http://example.com"; constexpr char kDefaultUrl[] = "http://example.com";
constexpr char kUsername[] = "username"; constexpr char kUsername[] = "username";
constexpr char kPassword[] = "password"; constexpr char kPassword[] = "password";
constexpr char kAccountEmail[] = "account@example.com";
constexpr char kDismissalReasonHistogramName[] = constexpr char kDismissalReasonHistogramName[] =
"PasswordManager.SaveUIDismissalReason"; "PasswordManager.SaveUIDismissalReason";
} // namespace } // namespace
...@@ -47,7 +48,7 @@ class SavePasswordMessageDelegateTest : public ChromeRenderViewHostTestHarness { ...@@ -47,7 +48,7 @@ class SavePasswordMessageDelegateTest : public ChromeRenderViewHostTestHarness {
void SetUsernameAndPassword(base::string16 username, base::string16 password); void SetUsernameAndPassword(base::string16 username, base::string16 password);
void CreateMessage(std::unique_ptr<PasswordFormManagerForUI> form_to_save, void CreateMessage(std::unique_ptr<PasswordFormManagerForUI> form_to_save,
bool is_saving_google_account); bool user_signed_in);
void TriggerActionClick(); void TriggerActionClick();
void TriggerBlocklistClick(); void TriggerBlocklistClick();
void TriggerMessageDismissedCallback(); void TriggerMessageDismissedCallback();
...@@ -101,9 +102,14 @@ void SavePasswordMessageDelegateTest::SetUsernameAndPassword( ...@@ -101,9 +102,14 @@ void SavePasswordMessageDelegateTest::SetUsernameAndPassword(
void SavePasswordMessageDelegateTest::CreateMessage( void SavePasswordMessageDelegateTest::CreateMessage(
std::unique_ptr<PasswordFormManagerForUI> form_to_save, std::unique_ptr<PasswordFormManagerForUI> form_to_save,
bool is_saving_google_account) { bool user_signed_in) {
base::Optional<AccountInfo> account_info;
if (user_signed_in) {
account_info = AccountInfo();
account_info.value().email = kAccountEmail;
}
delegate_.CreateMessage(web_contents(), std::move(form_to_save), delegate_.CreateMessage(web_contents(), std::move(form_to_save),
is_saving_google_account); account_info);
} }
void SavePasswordMessageDelegateTest::TriggerActionClick() { void SavePasswordMessageDelegateTest::TriggerActionClick() {
...@@ -153,7 +159,7 @@ TEST_F(SavePasswordMessageDelegateTest, MessagePropertyValues) { ...@@ -153,7 +159,7 @@ TEST_F(SavePasswordMessageDelegateTest, MessagePropertyValues) {
SetUsernameAndPassword(base::ASCIIToUTF16(kUsername), SetUsernameAndPassword(base::ASCIIToUTF16(kUsername),
base::ASCIIToUTF16(kPassword)); base::ASCIIToUTF16(kPassword));
auto form_manager = CreateFormManager(GURL(kDefaultUrl)); auto form_manager = CreateFormManager(GURL(kDefaultUrl));
CreateMessage(std::move(form_manager), false /*is_saving_google_account*/); CreateMessage(std::move(form_manager), false /*user_signed_in*/);
EXPECT_EQ(l10n_util::GetStringUTF16(IDS_SAVE_PASSWORD), EXPECT_EQ(l10n_util::GetStringUTF16(IDS_SAVE_PASSWORD),
GetMessageWrapper()->GetTitle()); GetMessageWrapper()->GetTitle());
...@@ -161,6 +167,8 @@ TEST_F(SavePasswordMessageDelegateTest, MessagePropertyValues) { ...@@ -161,6 +167,8 @@ TEST_F(SavePasswordMessageDelegateTest, MessagePropertyValues) {
base::ASCIIToUTF16(kUsername))); base::ASCIIToUTF16(kUsername)));
EXPECT_EQ(base::string16::npos, GetMessageWrapper()->GetDescription().find( EXPECT_EQ(base::string16::npos, GetMessageWrapper()->GetDescription().find(
base::ASCIIToUTF16(kPassword))); base::ASCIIToUTF16(kPassword)));
EXPECT_EQ(base::string16::npos, GetMessageWrapper()->GetDescription().find(
base::ASCIIToUTF16(kAccountEmail)));
EXPECT_EQ(l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_SAVE_BUTTON), EXPECT_EQ(l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_SAVE_BUTTON),
GetMessageWrapper()->GetPrimaryButtonText()); GetMessageWrapper()->GetPrimaryButtonText());
...@@ -175,14 +183,19 @@ TEST_F(SavePasswordMessageDelegateTest, MessagePropertyValues) { ...@@ -175,14 +183,19 @@ TEST_F(SavePasswordMessageDelegateTest, MessagePropertyValues) {
TriggerMessageDismissedCallback(); TriggerMessageDismissedCallback();
} }
// Tests that the title is set correctly when the user is syncing passwords to // Tests that the description is set correctly when the user is signed.
// their Google Account. TEST_F(SavePasswordMessageDelegateTest, SignedInDescription) {
TEST_F(SavePasswordMessageDelegateTest, SaveToGoogleTitle) { SetUsernameAndPassword(base::ASCIIToUTF16(kUsername),
base::ASCIIToUTF16(kPassword));
auto form_manager = CreateFormManager(GURL(kDefaultUrl)); auto form_manager = CreateFormManager(GURL(kDefaultUrl));
CreateMessage(std::move(form_manager), true /*is_saving_google_account*/); CreateMessage(std::move(form_manager), true /*user_signed_in*/);
EXPECT_EQ(l10n_util::GetStringUTF16(IDS_SAVE_PASSWORD_TO_GOOGLE), EXPECT_NE(base::string16::npos, GetMessageWrapper()->GetDescription().find(
GetMessageWrapper()->GetTitle()); base::ASCIIToUTF16(kUsername)));
EXPECT_EQ(base::string16::npos, GetMessageWrapper()->GetDescription().find(
base::ASCIIToUTF16(kPassword)));
EXPECT_NE(base::string16::npos, GetMessageWrapper()->GetDescription().find(
base::ASCIIToUTF16(kAccountEmail)));
TriggerMessageDismissedCallback(); TriggerMessageDismissedCallback();
} }
...@@ -195,7 +208,7 @@ TEST_F(SavePasswordMessageDelegateTest, SaveOnActionClick) { ...@@ -195,7 +208,7 @@ TEST_F(SavePasswordMessageDelegateTest, SaveOnActionClick) {
auto form_manager = CreateFormManager(GURL(kDefaultUrl)); auto form_manager = CreateFormManager(GURL(kDefaultUrl));
EXPECT_CALL(*form_manager, Save()); EXPECT_CALL(*form_manager, Save());
CreateMessage(std::move(form_manager), false /*is_saving_google_account*/); CreateMessage(std::move(form_manager), false /*user_signed_in*/);
EXPECT_NE(nullptr, GetMessageWrapper()); EXPECT_NE(nullptr, GetMessageWrapper());
TriggerActionClick(); TriggerActionClick();
EXPECT_NE(nullptr, GetMessageWrapper()); EXPECT_NE(nullptr, GetMessageWrapper());
...@@ -218,7 +231,7 @@ TEST_F(SavePasswordMessageDelegateTest, DontSaveOnDismiss) { ...@@ -218,7 +231,7 @@ TEST_F(SavePasswordMessageDelegateTest, DontSaveOnDismiss) {
auto form_manager = CreateFormManager(GURL(kDefaultUrl)); auto form_manager = CreateFormManager(GURL(kDefaultUrl));
EXPECT_CALL(*form_manager, Save()).Times(0); EXPECT_CALL(*form_manager, Save()).Times(0);
CreateMessage(std::move(form_manager), false /*is_saving_google_account*/); CreateMessage(std::move(form_manager), false /*user_signed_in*/);
EXPECT_NE(nullptr, GetMessageWrapper()); EXPECT_NE(nullptr, GetMessageWrapper());
TriggerMessageDismissedCallback(); TriggerMessageDismissedCallback();
EXPECT_EQ(nullptr, GetMessageWrapper()); EXPECT_EQ(nullptr, GetMessageWrapper());
......
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