From 3638d12af0c91a11510e487809f9b1b95df270a7 Mon Sep 17 00:00:00 2001 From: Keren Zhu Date: Wed, 27 Apr 2022 18:54:58 +0000 Subject: [PATCH] [Payments] Fix invisible overflowed legal message in Migration dialog The BoxLayout that contains the legal messages improperly uses the horizontal orientation (the default orientation), but the parent does not have enough width. As a result, long legal messages are clipped out. This CL fixes it by using vertical orientation. Also added a pixel test. Screenshot: https://screenshot.googleplex.com/7VPPY8pHrNRNYYB (cherry picked from commit f4df9598f7a20fe3bab24ee9bec963ba6c24ab85) Bug: 1306662 Change-Id: I560b867699cf2f009c1273653d067f83ccd1a2c3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3600416 Auto-Submit: Keren Zhu Reviewed-by: Evan Stade Commit-Queue: Evan Stade Cr-Original-Commit-Position: refs/heads/main@{#995340} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3606198 Reviewed-by: Keren Zhu Commit-Queue: Siyu An Bot-Commit: Rubber Stamper Reviewed-by: Mohamed Amir Yosef Cr-Commit-Position: refs/branch-heads/5005@{#204} Cr-Branched-From: 5b4d9450fee01f821b6400e947b3839727643a71-refs/heads/main@{#992738} --- .../local_card_migration_browsertest.cc | 34 +++++++++++++++++++ .../autofill/payments/payments_view_util.cc | 6 +--- .../filters/pixel_browser_tests.filter | 1 + 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/chrome/browser/ui/views/autofill/payments/local_card_migration_browsertest.cc b/chrome/browser/ui/views/autofill/payments/local_card_migration_browsertest.cc index 0f1a85e5f2ddae..6a089b312ca3c6 100644 --- a/chrome/browser/ui/views/autofill/payments/local_card_migration_browsertest.cc +++ b/chrome/browser/ui/views/autofill/payments/local_card_migration_browsertest.cc @@ -32,6 +32,7 @@ #include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/location_bar/location_bar.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/browser/ui/test/test_browser_dialog.h" #include "chrome/browser/ui/views/autofill/payments/dialog_view_ids.h" #include "chrome/browser/ui/views/autofill/payments/local_card_migration_bubble_views.h" #include "chrome/browser/ui/views/autofill/payments/local_card_migration_dialog_view.h" @@ -114,6 +115,12 @@ constexpr char kResponseGetUploadDetailsSuccess[] = "link: " "{0}.\",\"template_parameter\":[{\"display_text\":\"Link\",\"url\":\"https:" "//www.example.com/\"}]}]},\"context_token\":\"dummy_context_token\"}"; +constexpr char kResponseGetUploadDetailsSuccessLong[] = + "{\"legal_message\":{\"line\":[{\"template\":\"Long message 1 long message " + "2 long message 3 long message 4 long message 5 long message 6 long " + "message 7 long message 8 long message 9 long message 10 long message 11 " + "long message 12 long message 13 long message " + "14\"}]},\"context_token\":\"dummy_context_token\"}"; constexpr char kResponseGetUploadDetailsFailure[] = "{\"error\":{\"code\":\"FAILED_PRECONDITION\",\"user_error_message\":\"An " "unexpected error has occurred. Please try again later.\"}}"; @@ -543,6 +550,29 @@ class LocalCardMigrationBrowserTestForStatusChip base::test::ScopedFeatureList feature_list_; }; +class LocalCardMigrationBrowserUiTest + : public SupportsTestDialog { + public: + LocalCardMigrationBrowserUiTest(const LocalCardMigrationBrowserUiTest&) = + delete; + LocalCardMigrationBrowserUiTest& operator=( + const LocalCardMigrationBrowserUiTest&) = delete; + + protected: + LocalCardMigrationBrowserUiTest() = default; + ~LocalCardMigrationBrowserUiTest() override = default; + + // SupportsTestDialog: + void ShowUi(const std::string& name) override { + test_url_loader_factory()->AddResponse( + kURLGetUploadDetailsRequest, kResponseGetUploadDetailsSuccessLong); + SaveLocalCard(kFirstCardNumber); + SaveLocalCard(kSecondCardNumber); + UseCardAndWaitForMigrationOffer(kFirstCardNumber); + ClickOnOkButton(GetLocalCardMigrationOfferBubbleViews()); + } +}; + // Ensures that migration is not offered when user saves a new card. IN_PROC_BROWSER_TEST_F(LocalCardMigrationBrowserTest, UsingNewCardDoesNotShowIntermediateMigrationOffer) { @@ -1230,6 +1260,10 @@ IN_PROC_BROWSER_TEST_F(LocalCardMigrationBrowserTest, CardIdentifierString) { first_card.NicknameAndLastFourDigitsForTesting()); } +IN_PROC_BROWSER_TEST_F(LocalCardMigrationBrowserUiTest, InvokeUi_default) { + ShowAndVerifyUi(); +} + // TODO(crbug.com/897998): // - Add more tests for feedback dialog. diff --git a/chrome/browser/ui/views/autofill/payments/payments_view_util.cc b/chrome/browser/ui/views/autofill/payments/payments_view_util.cc index dc59739cecff7a..a6d20cfa52f2ec 100644 --- a/chrome/browser/ui/views/autofill/payments/payments_view_util.cc +++ b/chrome/browser/ui/views/autofill/payments/payments_view_util.cc @@ -166,12 +166,8 @@ LegalMessageView::LegalMessageView(const LegalMessageLines& legal_message_lines, SetBetweenChildSpacing(ChromeLayoutProvider::Get()->GetDistanceMetric( DISTANCE_RELATED_CONTROL_VERTICAL_SMALL)); for (const LegalMessageLine& line : legal_message_lines) { - // To ensure spacing between child paragraphs, all legal message lines are - // enclosed in their own view. - auto* line_view_ptr = - AddChildView(std::make_unique()); views::StyledLabel* label = - line_view_ptr->AddChildView(std::make_unique()); + AddChildView(std::make_unique()); label->SetText(line.text()); label->SetTextContext(views::style::CONTEXT_DIALOG_BODY_TEXT); label->SetDefaultTextStyle(views::style::STYLE_SECONDARY); diff --git a/testing/buildbot/filters/pixel_browser_tests.filter b/testing/buildbot/filters/pixel_browser_tests.filter index bcb89d18b9313c..bc3b0d9634825b 100644 --- a/testing/buildbot/filters/pixel_browser_tests.filter +++ b/testing/buildbot/filters/pixel_browser_tests.filter @@ -24,6 +24,7 @@ HatsBubbleTest.* HungRendererDialogViewBrowserTest.* ImportLockDialogViewBrowserTest.* InlineLoginHelperBrowserTest.InvokeUi_* +LocalCardMigrationBrowserUiTest.* NewTabPageTest.* OneTimePermissionPromptBubbleViewBrowserTest.* OutdatedUpgradeBubbleTest.*