Commit 99a7932c authored by Richard Knoll's avatar Richard Knoll Committed by Commit Bot

Merge origin into help text of Click to Call dialogs.

If we show help text in Click to Call dialogs, we can just embed the
origin into the help text itself instead of showing it as a separate
line. We keep showing it as a standalone label in the footnote if there
is no help text.

Before: https://imgur.com/PSIfHmz
After: https://imgur.com/UXeUbmp

Bug: 1019242
Change-Id: Id59ff2e53d2f3a5e7ab9da9a1ed876fb5d45641d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1887617
Commit-Queue: Richard Knoll <knollr@chromium.org>
Reviewed-by: default avatarHimanshu Jaju <himanshujaju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711361}
parent 1c324d34
...@@ -187,6 +187,8 @@ SharingDialogData ClickToCallUiController::CreateDialogData( ...@@ -187,6 +187,8 @@ SharingDialogData ClickToCallUiController::CreateDialogData(
data.help_text_id = data.help_text_id =
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_HELP_TEXT_NO_DEVICES; IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_HELP_TEXT_NO_DEVICES;
data.help_text_origin_id =
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_HELP_TEXT_NO_DEVICES_ORIGIN;
data.help_link_text_id = data.help_link_text_id =
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_TROUBLESHOOT_LINK; IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_TROUBLESHOOT_LINK;
data.origin_text_id = data.origin_text_id =
......
...@@ -39,6 +39,7 @@ struct SharingDialogData { ...@@ -39,6 +39,7 @@ struct SharingDialogData {
base::string16 title; base::string16 title;
base::string16 error_text; base::string16 error_text;
int help_text_id = 0; int help_text_id = 0;
int help_text_origin_id = 0;
int help_link_text_id = 0; int help_link_text_id = 0;
const gfx::VectorIcon* header_image_light = nullptr; const gfx::VectorIcon* header_image_light = nullptr;
const gfx::VectorIcon* header_image_dark = nullptr; const gfx::VectorIcon* header_image_dark = nullptr;
......
...@@ -80,15 +80,44 @@ base::string16 GetLastUpdatedTimeInDays(base::Time last_updated_timestamp) { ...@@ -80,15 +80,44 @@ base::string16 GetLastUpdatedTimeInDays(base::Time last_updated_timestamp) {
time_in_days); time_in_days);
} }
bool ShouldShowOrigin(const SharingDialogData& data,
content::WebContents* web_contents) {
return data.initiating_origin &&
!data.initiating_origin->IsSameOriginWith(
web_contents->GetMainFrame()->GetLastCommittedOrigin());
}
base::string16 PrepareHelpTextWithoutOrigin(const SharingDialogData& data,
const base::string16& link,
size_t* link_offset) {
DCHECK_NE(0, data.help_text_id);
return l10n_util::GetStringFUTF16(data.help_text_id, link, link_offset);
}
base::string16 PrepareHelpTextWithOrigin(const SharingDialogData& data,
const base::string16& link,
size_t* link_offset) {
DCHECK_NE(0, data.help_text_origin_id);
base::string16 origin = url_formatter::FormatOriginForSecurityDisplay(
*data.initiating_origin,
url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS);
std::vector<size_t> offsets;
base::string16 text = l10n_util::GetStringFUTF16(data.help_text_origin_id,
{origin, link}, &offsets);
*link_offset = offsets[1];
return text;
}
std::unique_ptr<views::StyledLabel> CreateHelpText( std::unique_ptr<views::StyledLabel> CreateHelpText(
const SharingDialogData& data, const SharingDialogData& data,
views::StyledLabelListener* listener) { views::StyledLabelListener* listener,
DCHECK_NE(0, data.help_text_id); bool show_origin) {
DCHECK_NE(0, data.help_link_text_id); DCHECK_NE(0, data.help_link_text_id);
const base::string16 link = l10n_util::GetStringUTF16(data.help_link_text_id); const base::string16 link = l10n_util::GetStringUTF16(data.help_link_text_id);
size_t offset; size_t offset;
const base::string16 text = const base::string16 text =
l10n_util::GetStringFUTF16(data.help_text_id, link, &offset); show_origin ? PrepareHelpTextWithOrigin(data, link, &offset)
: PrepareHelpTextWithoutOrigin(data, link, &offset);
auto label = std::make_unique<views::StyledLabel>(text, listener); auto label = std::make_unique<views::StyledLabel>(text, listener);
views::StyledLabel::RangeStyleInfo link_style = views::StyledLabel::RangeStyleInfo link_style =
views::StyledLabel::RangeStyleInfo::CreateForLink(); views::StyledLabel::RangeStyleInfo::CreateForLink();
...@@ -146,26 +175,16 @@ int SharingDialogView::GetDialogButtons() const { ...@@ -146,26 +175,16 @@ int SharingDialogView::GetDialogButtons() const {
} }
std::unique_ptr<views::View> SharingDialogView::CreateFootnoteView() { std::unique_ptr<views::View> SharingDialogView::CreateFootnoteView() {
constexpr int kLabelSpacing = 8; bool show_origin = ShouldShowOrigin(data_, web_contents());
switch (GetDialogType()) {
bool show_help_text = case SharingDialogType::kDialogWithoutDevicesWithApp:
GetDialogType() == SharingDialogType::kDialogWithoutDevicesWithApp; return CreateHelpText(data_, this, show_origin);
bool show_origin = case SharingDialogType::kDialogWithDevicesMaybeApps:
data_.initiating_origin && return show_origin ? CreateOriginView(data_) : nullptr;
!data_.initiating_origin->IsSameOriginWith( case SharingDialogType::kErrorDialog:
web_contents()->GetMainFrame()->GetLastCommittedOrigin()); case SharingDialogType::kEducationalDialog:
if (!show_help_text && !show_origin) return nullptr;
return nullptr; }
auto footnote_view = std::make_unique<views::View>();
footnote_view->SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kVertical, gfx::Insets(), kLabelSpacing));
if (show_help_text)
footnote_view->AddChildView(CreateHelpText(data_, this));
if (show_origin)
footnote_view->AddChildView(CreateOriginView(data_));
return footnote_view;
} }
void SharingDialogView::StyledLabelLinkClicked(views::StyledLabel* label, void SharingDialogView::StyledLabelLinkClicked(views::StyledLabel* label,
...@@ -315,7 +334,8 @@ void SharingDialogView::InitListView() { ...@@ -315,7 +334,8 @@ void SharingDialogView::InitListView() {
} }
void SharingDialogView::InitEmptyView() { void SharingDialogView::InitEmptyView() {
AddChildView(CreateHelpText(data_, this)); bool show_origin = ShouldShowOrigin(data_, web_contents());
AddChildView(CreateHelpText(data_, this, show_origin));
} }
void SharingDialogView::InitErrorView() { void SharingDialogView::InitErrorView() {
......
...@@ -15,13 +15,16 @@ ...@@ -15,13 +15,16 @@
#include "chrome/browser/ui/views/hover_button.h" #include "chrome/browser/ui/views/hover_button.h"
#include "chrome/test/base/browser_with_test_window_test.h" #include "chrome/test/base/browser_with_test_window_test.h"
#include "components/sync_device_info/device_info.h" #include "components/sync_device_info/device_info.h"
#include "components/url_formatter/elide_url.h"
#include "components/vector_icons/vector_icons.h" #include "components/vector_icons/vector_icons.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/events/event_utils.h" #include "ui/events/event_utils.h"
#include "ui/strings/grit/ui_strings.h" #include "ui/strings/grit/ui_strings.h"
#include "ui/views/bubble/bubble_dialog_delegate_view.h" #include "ui/views/bubble/bubble_dialog_delegate_view.h"
#include "ui/views/bubble/bubble_frame_view.h" #include "ui/views/bubble/bubble_frame_view.h"
#include "ui/views/controls/styled_label.h"
#include "url/gurl.h" #include "url/gurl.h"
#include "url/origin.h" #include "url/origin.h"
...@@ -115,6 +118,8 @@ class SharingDialogViewTest : public BrowserWithTestWindowTest { ...@@ -115,6 +118,8 @@ class SharingDialogViewTest : public BrowserWithTestWindowTest {
data.help_text_id = data.help_text_id =
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_HELP_TEXT_NO_DEVICES; IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_HELP_TEXT_NO_DEVICES;
data.help_text_origin_id =
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_HELP_TEXT_NO_DEVICES_ORIGIN;
data.help_link_text_id = data.help_link_text_id =
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_TROUBLESHOOT_LINK; IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_TROUBLESHOOT_LINK;
data.origin_text_id = data.origin_text_id =
...@@ -235,3 +240,42 @@ TEST_F(SharingDialogViewTest, OriginView) { ...@@ -235,3 +240,42 @@ TEST_F(SharingDialogViewTest, OriginView) {
// match the main frame origin. // match the main frame origin.
EXPECT_EQ(nullptr, dialog->CreateFootnoteView()); EXPECT_EQ(nullptr, dialog->CreateFootnoteView());
} }
TEST_F(SharingDialogViewTest, HelpTextContent) {
url::Origin current_origin = url::Origin::Create(GURL("https://google.com"));
url::Origin other_origin = url::Origin::Create(GURL("https://example.com"));
base::string16 link_text = l10n_util::GetStringUTF16(
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_TROUBLESHOOT_LINK);
base::string16 origin_text = url_formatter::FormatOriginForSecurityDisplay(
other_origin, url_formatter::SchemeDisplay::OMIT_HTTP_AND_HTTPS);
base::string16 expected_default = l10n_util::GetStringFUTF16(
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_HELP_TEXT_NO_DEVICES, link_text);
base::string16 expected_origin = l10n_util::GetStringFUTF16(
IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_HELP_TEXT_NO_DEVICES_ORIGIN,
origin_text, link_text);
// Expect default help text if no initiating origin is set.
auto dialog_data = CreateDialogData(/*devices=*/0, /*apps=*/1);
auto dialog = CreateDialogView(std::move(dialog_data));
auto footnote_view = dialog->CreateFootnoteView();
EXPECT_EQ(expected_default,
static_cast<views::StyledLabel*>(footnote_view.get())->GetText());
// Still expect the default help text if the initiating origin matches the
// main frame origin.
dialog_data = CreateDialogData(/*devices=*/0, /*apps=*/1);
dialog_data.initiating_origin = current_origin;
dialog = CreateDialogView(std::move(dialog_data));
footnote_view = dialog->CreateFootnoteView();
EXPECT_EQ(expected_default,
static_cast<views::StyledLabel*>(footnote_view.get())->GetText());
// Expect the origin to be included in the help text if it does not match the
// main frame origin.
dialog_data = CreateDialogData(/*devices=*/0, /*apps=*/1);
dialog_data.initiating_origin = other_origin;
dialog = CreateDialogView(std::move(dialog_data));
footnote_view = dialog->CreateFootnoteView();
EXPECT_EQ(expected_origin,
static_cast<views::StyledLabel*>(footnote_view.get())->GetText());
}
...@@ -968,6 +968,9 @@ need to be translated for each locale.--> ...@@ -968,6 +968,9 @@ need to be translated for each locale.-->
<message name="IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_HELP_TEXT_NO_DEVICES" desc="The label to be shown as a help text of the dialog when user click on a phone number, if there are no phones to choose from."> <message name="IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_HELP_TEXT_NO_DEVICES" desc="The label to be shown as a help text of the dialog when user click on a phone number, if there are no phones to choose from.">
To send a number from here to your Android phone, <ph name="TROUBLESHOOT_LINK">$1<ex>turn on sync</ex></ph> for both devices in settings. To send a number from here to your Android phone, <ph name="TROUBLESHOOT_LINK">$1<ex>turn on sync</ex></ph> for both devices in settings.
</message> </message>
<message name="IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_HELP_TEXT_NO_DEVICES_ORIGIN" desc="The label to be shown as a help text of the dialog when user click on a phone number, if there are no phones to choose from.">
To send a number from <ph name="ORIGIN">$1<ex>www.google.com</ex></ph> to your Android phone, <ph name="TROUBLESHOOT_LINK">$2<ex>turn on sync</ex></ph> for both devices in settings.
</message>
<message name="IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_TROUBLESHOOT_LINK" desc="The label to be shown on the dialog when user click on a phone number for the link to a help page to turn on sync."> <message name="IDS_BROWSER_SHARING_CLICK_TO_CALL_DIALOG_TROUBLESHOOT_LINK" desc="The label to be shown on the dialog when user click on a phone number for the link to a help page to turn on sync.">
turn on sync turn on sync
</message> </message>
......
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