Skip to content

GitLab

T
tangled
Commit f6fa5f43 authored by Alex Moshchuk's avatar Alex Moshchuk Committed by Commit Bot
Browse files
Options

Revert "[omnibox] Add tests for dedicated row and pedals a11y" 

This reverts commit 4dfbdf6a.

Reason for revert: The EmitAccessibilityEventsOnButtonFocusHint test that's modified here is failing on Mac10.11:
https://ci.chromium.org/p/chromium/builders/ci/Mac10.11%20Tests/53696

Failure output:
[ RUN      ] All/OmniboxPopupContentsViewTest.EmitAccessibilityEventsOnButtonFocusHint/1
[26756:3843:0819/131719.560091:WARNING:notification_platform_bridge_mac.mm(572)] AlertNotificationService: XPC connection invalidated.
../../chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc:163: Failure
Value of: contains(omnibox_value(), expected_value)
  Actual: false
Expected: true
Google Test trace:
../../chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc:162: Omnibox value (foo/, 2 of 3) is expected to contain "The Foo Of All Bars"
Stack trace:
0   browser_tests                       0x00000001072bee49 (anonymous namespace)::TestAXEventObserver::CheckOmniboxValue(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >, std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >) + 825
1   browser_tests                       0x00000001072bda50 OmniboxPopupContentsViewTest_EmitAccessibilityEventsOnButtonFocusHint_Test::RunTestOnMainThread() + 640
2   browser_tests                       0x000000010c21394d content::BrowserTestBase::ProxyRunTestOnMainThreadLoop() + 717

Original change's description:
> [omnibox] Add tests for dedicated row and pedals a11y
> 
> Added tests for keyword button, pedal button, remove suggestion button,
> multiple buttons for one suggestion (tab switch and keyword button).
> Cleaned up tests.
> 
> Fixed a11y for remove suggestion button and buttons in dedicated row
> (set role to be list box option and set correct accessible name).
> Fixed selection event not being triggered for dedicated row buttons
> (GetSecondaryButton wasn't returning buttons from dedicated row).
> Fixed value changed event triggering extra time when switching selection
> from button to button.
> 
> Bug: 1104264
> Change-Id: I0c7a7a9cee0622cce2a24f6ea58c73a56ac996bc
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352826
> Commit-Queue: Olesia Marukhno <olesiamarukhno@google.com>
> Reviewed-by: Bret Sepulveda <bsep@chromium.org>
> Reviewed-by: Tommy Li <tommycli@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#799723}

TBR=tommycli@chromium.org,bsep@chromium.org,olesiamarukhno@google.com

Change-Id: I97466a40db517ef10dc138ebbea1a0d8569e0a3e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1104264
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2364961Reviewed-by: default avatarAlex Moshchuk <alexmos@chromium.org>
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799799}
parent 66870c32
Hide whitespace changes
Inline Side-by-side
Showing with 85 additions and 359 deletions
chrome/browser/ui/views/omnibox/omnibox_popup_contents_view_browsertest.cc
View file @ f6fa5f43
......@@ -23,10 +23,7 @@
#include "chrome/test/base/ui_test_utils.h"
#include "components/omnibox/browser/omnibox_edit_model.h"
#include "components/omnibox/browser/omnibox_field_trial.h"
#include "components/omnibox/browser/omnibox_pedal.h"
#include "components/omnibox/browser/omnibox_popup_model.h"
#include "components/omnibox/common/omnibox_features.h"
#include "components/strings/grit/components_strings.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/test_utils.h"
#include "ui/accessibility/ax_enums.mojom.h"
......@@ -144,50 +141,6 @@ class TestAXEventObserver : public views::AXEventObserver {
std::string omnibox_value() { return omnibox_value_; }
std::string selected_option_name() { return selected_option_name_; }
void CheckAccessibilityCounters(int expected_count) {
// TODO(olesiamarukhno): Add check for text_changed_on_listboxoption_count.
// We need to check all thses counters to make sure all screen readers would
// work (different screen readers use different events).
EXPECT_EQ(selected_children_changed_count(), expected_count);
EXPECT_EQ(selection_changed_count(), expected_count);
EXPECT_EQ(active_descendant_changed_count(), expected_count);
EXPECT_EQ(value_changed_count(), expected_count);
}
void CheckOmniboxValue(std::vector<std::string> expected_values,
std::vector<std::string> forbidden_values) {
for (auto expected_value : expected_values) {
SCOPED_TRACE(
base::StringPrintf("Omnibox value (%s) is expected to contain \"%s\"",
omnibox_value_.c_str(), expected_value.c_str()));
EXPECT_TRUE(contains(omnibox_value(), expected_value));
}
for (auto forbidden_value : forbidden_values) {
SCOPED_TRACE(
base::StringPrintf("Omnibox value (%s) should not contain \"%s\"",
omnibox_value_.c_str(), forbidden_value.c_str()));
EXPECT_FALSE(contains(omnibox_value(), forbidden_value));
}
}
void CheckSelectedOptionName(std::vector<std::string> expected_values,
std::vector<std::string> forbidden_values) {
for (auto expected_value : expected_values) {
SCOPED_TRACE(base::StringPrintf(
"Selected option name (%s) is expected to contain \"%s\"",
selected_option_name_.c_str(), expected_value.c_str()));
EXPECT_TRUE(contains(selected_option_name_, expected_value));
}
for (auto forbidden_value : forbidden_values) {
SCOPED_TRACE(base::StringPrintf(
"Selected option name (%s) should not contain \"%s\"",
selected_option_name_.c_str(), forbidden_value.c_str()));
EXPECT_FALSE(contains(selected_option_name_, forbidden_value));
}
}
private:
int text_changed_on_listboxoption_count_ = 0;
int selected_children_changed_count_ = 0;
......@@ -202,17 +155,9 @@ class TestAXEventObserver : public views::AXEventObserver {
} // namespace
class OmniboxPopupContentsViewTest
: public InProcessBrowserTest,
public ::testing::WithParamInterface<bool> {
class OmniboxPopupContentsViewTest : public InProcessBrowserTest {
public:
OmniboxPopupContentsViewTest() {
if (GetParam()) {
feature_list_.InitWithFeatures({omnibox::kOmniboxSuggestionButtonRow,
omnibox::kOmniboxKeywordSearchButton},
{});
}
}
OmniboxPopupContentsViewTest() {}
views::Widget* CreatePopupForTestQuery();
views::Widget* GetPopupWidget() { return popup_view()->GetWidget(); }
......@@ -231,28 +176,6 @@ class OmniboxPopupContentsViewTest
return static_cast<OmniboxPopupContentsView*>(popup_model()->view());
}
AutocompleteMatch CreateAutocompleteMatch() {
AutocompleteMatch match(nullptr, 500, false,
AutocompleteMatchType::HISTORY_TITLE);
match.contents = base::ASCIIToUTF16("https://foobar.com");
match.description = base::ASCIIToUTF16("The Foo Of All Bars");
return match;
}
void InitPopupWithMatch(AutocompleteMatch match) {
ACMatches matches;
matches.push_back(match);
InitPopupWithMatches(matches);
}
void InitPopupWithMatches(const ACMatches& matches) {
CreatePopupForTestQuery();
AutocompleteController* controller =
popup_model()->autocomplete_controller();
controller->result_.AppendMatches(controller->input_, matches);
popup_view()->UpdatePopupAppearance();
}
private:
base::test::ScopedFeatureList feature_list_;
......@@ -279,7 +202,7 @@ views::Widget* OmniboxPopupContentsViewTest::CreatePopupForTestQuery() {
}
// Tests widget alignment of the different popup types.
IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, PopupAlignment) {
IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest, PopupAlignment) {
views::Widget* popup = CreatePopupForTestQuery();
#if defined(USE_AURA)
......@@ -298,7 +221,7 @@ IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, PopupAlignment) {
}
// Integration test for omnibox popup theming.
IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, ThemeIntegration) {
IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest, ThemeIntegration) {
// This test relies on the light/dark variants of the result background to be
// different. But when using the GTK theme on Linux, these colors will be the
// same. Ensure we're not using the system (GTK) theme, which may be
......@@ -381,7 +304,7 @@ IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, ThemeIntegration) {
#define MAYBE_ClickOmnibox ClickOmnibox
#endif
// Test that clicks over the omnibox do not hit the popup.
IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, MAYBE_ClickOmnibox) {
IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest, MAYBE_ClickOmnibox) {
CreatePopupForTestQuery();
gfx::NativeWindow event_window = browser()->window()->GetNativeWindow();
......@@ -435,7 +358,7 @@ IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, MAYBE_ClickOmnibox) {
// Check that the location bar background (and the background of the textfield
// it contains) changes when it receives focus, and matches the popup background
// color.
IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest,
IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest,
PopupMatchesLocationBarBackground) {
// In dark mode the omnibox focused and unfocused colors are the same, which
// makes this test fail; see comments below.
......@@ -470,22 +393,23 @@ IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest,
EXPECT_EQ(color_before_focus, omnibox_view()->GetBackgroundColor());
}
IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, EmitAccessibilityEvents) {
IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest, EmitAccessibilityEvents) {
// Creation and population of the popup should not result in a text/name
// change accessibility event.
TestAXEventObserver observer;
CreatePopupForTestQuery();
ACMatches matches;
AutocompleteMatch match(nullptr, 500, false,
AutocompleteMatchType::HISTORY_TITLE);
AutocompleteController* controller = popup_model()->autocomplete_controller();
match.contents = base::ASCIIToUTF16("https://foobar.com");
match.description = base::ASCIIToUTF16("FooBarCom");
matches.push_back(match);
match.contents = base::ASCIIToUTF16("https://foobarbaz.com");
match.description = base::ASCIIToUTF16("FooBarBazCom");
matches.push_back(match);
InitPopupWithMatches(matches);
controller->result_.AppendMatches(controller->input_, matches);
popup_view()->UpdatePopupAppearance();
EXPECT_EQ(observer.text_changed_on_listboxoption_count(), 0);
// Changing the user text while in the input rather than the list should not
......@@ -538,186 +462,63 @@ IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest, EmitAccessibilityEvents) {
selected_result_view->GetViewAccessibility().GetUniqueId().Get());
}
IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest,
IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest,
EmitAccessibilityEventsOnButtonFocusHint) {
TestAXEventObserver observer;
AutocompleteMatch match = CreateAutocompleteMatch();
match.has_tab_match = true;
InitPopupWithMatch(match);
popup_view()->model()->SetSelection(OmniboxPopupModel::Selection(1));
observer.CheckAccessibilityCounters(2);
observer.CheckOmniboxValue(
{"The Foo Of All Bars", "press Tab then Enter", "2 of 2"}, {});
observer.CheckSelectedOptionName({"foobar.com", "press Tab then Enter"},
{"2 of 2"});
popup_view()->model()->SetSelection(OmniboxPopupModel::Selection(
1, OmniboxPopupModel::FOCUSED_BUTTON_TAB_SWITCH));
observer.CheckAccessibilityCounters(3);
observer.CheckOmniboxValue({"The Foo Of All Bars", "press Enter to switch"},
{"2 of 2"});
observer.CheckSelectedOptionName({"press Enter to switch"}, {"2 of 2"});
popup_view()->model()->SetSelection(
OmniboxPopupModel::Selection(1, OmniboxPopupModel::NORMAL));
observer.CheckAccessibilityCounters(4);
observer.CheckOmniboxValue(
{"The Foo Of All Bars", "press Tab then Enter", "2 of 2"}, {});
observer.CheckSelectedOptionName({"foobar.com", "press Tab then Enter"},
{"2 of 2"});
}
IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest,
EmitAccessibilityEventsOnKeywordButtonFocusHint) {
if (!GetParam())
return;
TestAXEventObserver observer;
AutocompleteMatch match = CreateAutocompleteMatch();
match.keyword = base::ASCIIToUTF16("match");
match.associated_keyword = std::make_unique<AutocompleteMatch>();
InitPopupWithMatch(match);
popup_view()->model()->SetSelection(OmniboxPopupModel::Selection(1));
observer.CheckAccessibilityCounters(2);
observer.CheckOmniboxValue(
{"The Foo Of All Bars", "press Tab then Enter to search", "2 of 2"}, {});
observer.CheckSelectedOptionName(
{"foobar.com", "press Tab then Enter to search"}, {"2 of 2"});
popup_view()->model()->SetSelection(OmniboxPopupModel::Selection(
1, OmniboxPopupModel::FOCUSED_BUTTON_KEYWORD));
observer.CheckAccessibilityCounters(3);
observer.CheckOmniboxValue({"The Foo Of All Bars", "press Enter to search"},
{"2 of 2"});
observer.CheckSelectedOptionName({"press Enter to search"}, {"2 of 2"});
popup_view()->model()->SetSelection(
OmniboxPopupModel::Selection(1, OmniboxPopupModel::NORMAL));
observer.CheckAccessibilityCounters(4);
observer.CheckOmniboxValue(
{"The Foo Of All Bars", "press Tab then Enter to search", "2 of 2"}, {});
observer.CheckSelectedOptionName(
{"foobar.com", "press Tab then Enter to search"}, {"2 of 2"});
}
IN_PROC_BROWSER_TEST_P(
OmniboxPopupContentsViewTest,
EmitAccessibilityEventsOnRemoveSuggestionButtonFocusHint) {
TestAXEventObserver observer;
AutocompleteMatch match = CreateAutocompleteMatch();
match.deletable = true;
InitPopupWithMatch(match);
popup_view()->model()->SetSelection(OmniboxPopupModel::Selection(1));
observer.CheckAccessibilityCounters(2);
observer.CheckOmniboxValue({"The Foo Of All Bars", "2 of 2"},
{"press Enter to remove"});
observer.CheckSelectedOptionName({"foobar.com"},
{"press Enter to remove", "2 of 2"});
popup_view()->model()->SetSelection(OmniboxPopupModel::Selection(
1, OmniboxPopupModel::FOCUSED_BUTTON_REMOVE_SUGGESTION));
observer.CheckAccessibilityCounters(3);
observer.CheckOmniboxValue({"The Foo Of All Bars", "press Enter to remove"},
{"2 of 2"});
observer.CheckSelectedOptionName({"press Enter to remove"}, {"2 of 2"});
popup_view()->model()->SetSelection(
OmniboxPopupModel::Selection(1, OmniboxPopupModel::NORMAL));
observer.CheckAccessibilityCounters(4);
observer.CheckOmniboxValue({"The Foo Of All Bars", "2 of 2"},
{"press Enter to remove"});
observer.CheckSelectedOptionName({"foobar.com"},
{"press Enter to remove", "2 of 2"});
}
IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest,
EmitAccessibilityEventsOnPedalButtonFocusHint) {
if (!GetParam())
return;
TestAXEventObserver observer;
AutocompleteMatch match = CreateAutocompleteMatch();
std::unique_ptr<OmniboxPedal> pedal = std::make_unique<OmniboxPedal>(
OmniboxPedal::LabelStrings(
IDS_OMNIBOX_PEDAL_CLEAR_BROWSING_DATA_HINT,
IDS_OMNIBOX_PEDAL_CLEAR_BROWSING_DATA_HINT_SHORT,
IDS_OMNIBOX_PEDAL_CLEAR_BROWSING_DATA_SUGGESTION_CONTENTS,
IDS_ACC_OMNIBOX_PEDAL_CLEAR_BROWSING_DATA_SUFFIX,
IDS_ACC_OMNIBOX_PEDAL_CLEAR_BROWSING_DATA),
GURL());
match.pedal = pedal.get();
InitPopupWithMatch(match);
popup_view()->model()->SetSelection(OmniboxPopupModel::Selection(1));
observer.CheckAccessibilityCounters(2);
observer.CheckOmniboxValue(
{"The Foo Of All Bars", "press Tab then Enter", "2 of 2"}, {});
observer.CheckSelectedOptionName({"foobar.com", "press Tab then Enter"},
{"2 of 2"});
popup_view()->model()->SetSelection(
OmniboxPopupModel::Selection(1, OmniboxPopupModel::FOCUSED_BUTTON_PEDAL));
observer.CheckAccessibilityCounters(3);
observer.CheckOmniboxValue({"press Enter to clear"},
{"The Foo Of All Bars", "2 of 2"});
observer.CheckSelectedOptionName({"press Enter to clear"},
{"foobar.com", "2 of 2"});
popup_view()->model()->SetSelection(
OmniboxPopupModel::Selection(1, OmniboxPopupModel::NORMAL));
observer.CheckAccessibilityCounters(4);
observer.CheckOmniboxValue(
{"The Foo Of All Bars", "press Tab then Enter", "2 of 2"}, {});
observer.CheckSelectedOptionName({"foobar.com", "press Tab then Enter"},
{"2 of 2"});
}
IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest,
EmitAccessibilityEventsOnMultipleButtonsFocusHint) {
if (!GetParam())
return;
TestAXEventObserver observer;
AutocompleteMatch match = CreateAutocompleteMatch();
match.keyword = base::ASCIIToUTF16("match");
match.associated_keyword = std::make_unique<AutocompleteMatch>();
CreatePopupForTestQuery();
ACMatches matches;
AutocompleteMatch match(nullptr, 500, false,
AutocompleteMatchType::HISTORY_TITLE);
AutocompleteController* controller = popup_model()->autocomplete_controller();
match.contents = base::ASCIIToUTF16("https://foobar.com");
match.description = base::ASCIIToUTF16("The Foo Of All Bars");
match.has_tab_match = true;
InitPopupWithMatch(match);
matches.push_back(match);
controller->result_.AppendMatches(controller->input_, matches);
popup_view()->UpdatePopupAppearance();
popup_view()->model()->SetSelection(OmniboxPopupModel::Selection(1));
observer.CheckAccessibilityCounters(2);
observer.CheckOmniboxValue(
{"The Foo Of All Bars", "multiple actions are available", "2 of 2"}, {});
observer.CheckSelectedOptionName(
{"foobar.com", "multiple actions are available"}, {"2 of 2"});
EXPECT_EQ(observer.selected_children_changed_count(), 2);
EXPECT_EQ(observer.selection_changed_count(), 2);
EXPECT_EQ(observer.active_descendant_changed_count(), 2);
EXPECT_EQ(observer.value_changed_count(), 2);
EXPECT_TRUE(contains(observer.omnibox_value(), "The Foo Of All Bars"));
EXPECT_TRUE(contains(observer.selected_option_name(), "foobar.com"));
EXPECT_TRUE(contains(observer.omnibox_value(), "press Tab then Enter"));
EXPECT_TRUE(contains(observer.omnibox_value(), "2 of 2"));
EXPECT_TRUE(
contains(observer.selected_option_name(), "press Tab then Enter"));
EXPECT_FALSE(contains(observer.selected_option_name(), "2 of 2"));
popup_view()->model()->SetSelection(OmniboxPopupModel::Selection(
1, OmniboxPopupModel::FOCUSED_BUTTON_TAB_SWITCH));
observer.CheckAccessibilityCounters(3);
observer.CheckOmniboxValue({"The Foo Of All Bars", "press Enter to switch"},
{"2 of 2"});
observer.CheckSelectedOptionName({"press Enter to switch"}, {"2 of 2"});
popup_view()->model()->SetSelection(OmniboxPopupModel::Selection(
1, OmniboxPopupModel::FOCUSED_BUTTON_KEYWORD));
observer.CheckAccessibilityCounters(4);
observer.CheckOmniboxValue({"The Foo Of All Bars", "press Enter to search"},
{"2 of 2"});
observer.CheckSelectedOptionName({"press Enter to search"}, {"2 of 2"});
EXPECT_TRUE(contains(observer.omnibox_value(), "The Foo Of All Bars"));
EXPECT_EQ(observer.selected_children_changed_count(), 3);
EXPECT_EQ(observer.selection_changed_count(), 3);
EXPECT_EQ(observer.active_descendant_changed_count(), 3);
EXPECT_EQ(observer.value_changed_count(), 3);
EXPECT_TRUE(contains(observer.omnibox_value(), "press Enter to switch"));
EXPECT_FALSE(contains(observer.omnibox_value(), "2 of 2"));
EXPECT_TRUE(
contains(observer.selected_option_name(), "press Enter to switch"));
EXPECT_FALSE(contains(observer.selected_option_name(), "2 of 2"));
popup_view()->model()->SetSelection(
OmniboxPopupModel::Selection(1, OmniboxPopupModel::NORMAL));
observer.CheckAccessibilityCounters(5);
observer.CheckOmniboxValue(
{"The Foo Of All Bars", "multiple actions are available", "2 of 2"}, {});
observer.CheckSelectedOptionName(
{"foobar.com", "multiple actions are available"}, {"2 of 2"});
EXPECT_TRUE(contains(observer.omnibox_value(), "The Foo Of All Bars"));
EXPECT_TRUE(contains(observer.selected_option_name(), "foobar.com"));
EXPECT_EQ(observer.selected_children_changed_count(), 4);
EXPECT_EQ(observer.selection_changed_count(), 4);
EXPECT_EQ(observer.active_descendant_changed_count(), 4);
EXPECT_EQ(observer.value_changed_count(), 4);
EXPECT_TRUE(contains(observer.omnibox_value(), "press Tab then Enter"));
EXPECT_TRUE(contains(observer.omnibox_value(), "2 of 2"));
EXPECT_TRUE(
contains(observer.selected_option_name(), "press Tab then Enter"));
EXPECT_FALSE(contains(observer.selected_option_name(), "2 of 2"));
}
IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest,
IN_PROC_BROWSER_TEST_F(OmniboxPopupContentsViewTest,
EmitSelectedChildrenChangedAccessibilityEvent) {
// Create a popup for the matches.
GetPopupWidget();
......@@ -784,7 +585,3 @@ IN_PROC_BROWSER_TEST_P(OmniboxPopupContentsViewTest,
EXPECT_TRUE(popup_node_data_while_open.HasIntAttribute(
ax::mojom::IntAttribute::kPopupForId));
}
INSTANTIATE_TEST_SUITE_P(All,
OmniboxPopupContentsViewTest,
::testing::Values(false, true));
chrome/browser/ui/views/omnibox/omnibox_result_view.cc
View file @ f6fa5f43
......@@ -45,26 +45,6 @@
#include "base/win/atl.h"
#endif
namespace {
class OmniboxRemoveSuggestionButton : public views::ImageButton {
public:
explicit OmniboxRemoveSuggestionButton(views::ButtonListener* listener)
: ImageButton(listener) {
views::ConfigureVectorImageButton(this);
}
void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
node_data->SetName(
l10n_util::GetStringUTF16(IDS_ACC_REMOVE_SUGGESTION_BUTTON));
// Although this appears visually as a button, expose as a list box option
// so that it matches the other options within its list box container.
node_data->role = ax::mojom::Role::kListBoxOption;
}
};
} // namespace
////////////////////////////////////////////////////////////////////////////////
// OmniboxResultView, public:
......@@ -92,7 +72,7 @@ OmniboxResultView::OmniboxResultView(
// TODO(tommycli): Make sure we announce the Shift+Delete capability in the
// accessibility node data for removable suggestions.
remove_suggestion_button_ =
AddChildView(std::make_unique<OmniboxRemoveSuggestionButton>(this));
AddChildView(views::CreateVectorImageButton(this));
views::InstallCircleHighlightPathGenerator(remove_suggestion_button_);
remove_suggestion_button_->SetTooltipText(
l10n_util::GetStringUTF16(IDS_OMNIBOX_REMOVE_SUGGESTION));
......@@ -293,10 +273,6 @@ views::Button* OmniboxResultView::GetSecondaryButton() {
if (remove_suggestion_button_->GetVisible())
return remove_suggestion_button_;
if (OmniboxFieldTrial::IsSuggestionButtonRowEnabled()) {
return button_row_->GetActiveButton();
}
return nullptr;
}
......
chrome/browser/ui/views/omnibox/omnibox_suggestion_button_row_view.cc
View file @ f6fa5f43
......@@ -17,11 +17,9 @@
#include "components/strings/grit/components_strings.h"
#include "components/vector_icons/vector_icons.h"
#include "third_party/metrics_proto/omnibox_event.pb.h"
#include "ui/accessibility/ax_node_data.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/gfx/color_utils.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/views/accessibility/view_accessibility.h"
#include "ui/views/animation/ink_drop_highlight.h"
#include "ui/views/background.h"
#include "ui/views/controls/button/label_button_border.h"
......@@ -35,12 +33,8 @@ class OmniboxSuggestionRowButton : public views::MdTextButton {
OmniboxSuggestionRowButton(views::ButtonListener* listener,
const base::string16& text,
const gfx::VectorIcon& icon,
OmniboxPopupContentsView* popup_contents_view,
OmniboxPopupModel::Selection selection)
: MdTextButton(listener, text, CONTEXT_OMNIBOX_PRIMARY),
icon_(icon),
popup_contents_view_(popup_contents_view),
selection_(selection) {
const views::FocusRing::ViewPredicate& predicate)
: MdTextButton(listener, text, CONTEXT_OMNIBOX_PRIMARY), icon_(icon) {
views::InstallPillHighlightPathGenerator(this);
SetImageLabelSpacing(ChromeLayoutProvider::Get()->GetDistanceMetric(
DISTANCE_RELATED_LABEL_HORIZONTAL_LIST));
......@@ -50,10 +44,7 @@ class OmniboxSuggestionRowButton : public views::MdTextButton {
GetLayoutConstant(LOCATION_BAR_ICON_SIZE));
set_ink_drop_highlight_opacity(CalculateInkDropHighlightOpacity());
focus_ring()->SetHasFocusPredicate([=](View* view) {
return view->GetVisible() &&
popup_contents_view_->model()->selection() == selection_;
});
focus_ring()->SetHasFocusPredicate(predicate);
}
OmniboxSuggestionRowButton(const OmniboxSuggestionRowButton&) = delete;
......@@ -68,8 +59,6 @@ class OmniboxSuggestionRowButton : public views::MdTextButton {
void OnStyleRefresh() { focus_ring()->SchedulePaint(); }
OmniboxPopupModel::Selection selection() { return selection_; }
std::unique_ptr<views::InkDropHighlight> CreateInkDropHighlight()
const override {
// MdTextButton uses custom colors when creating ink drop highlight.
......@@ -88,17 +77,8 @@ class OmniboxSuggestionRowButton : public views::MdTextButton {
icon_, GetLayoutConstant(LOCATION_BAR_ICON_SIZE), color));
}
void GetAccessibleNodeData(ui::AXNodeData* node_data) override {
node_data->SetName(GetAccessibleName());
// Although this appears visually as a button, expose as a list box option
// so that it matches the other options within its list box container.
node_data->role = ax::mojom::Role::kListBoxOption;
}
private:
const gfx::VectorIcon& icon_;
OmniboxPopupContentsView* popup_contents_view_;
OmniboxPopupModel::Selection selection_;
float CalculateInkDropHighlightOpacity() {
// Ink drop highlight opacity is result of mixing a layer with hovered
......@@ -129,27 +109,30 @@ OmniboxSuggestionButtonRowView::OmniboxSuggestionButtonRowView(
gfx::Insets(0, ChromeLayoutProvider::Get()->GetDistanceMetric(
views::DISTANCE_RELATED_BUTTON_HORIZONTAL)));
const auto make_predicate = [=](auto state) {
return [=](View* view) {
return view->GetVisible() &&
model()->selection() ==
OmniboxPopupModel::Selection(model_index_, state);
};
};
// For all of these buttons, the visibility set from UpdateFromModel().
// The Keyword and Pedal buttons also get their text from there, since the
// text depends on the actual match. That shouldn't produce a flicker, because
// it's called directly from OmniboxResultView::SetMatch(). If this flickers,
// then so does everything else in the result view.
keyword_button_ = AddChildView(std::make_unique<OmniboxSuggestionRowButton>(
this, base::string16(), vector_icons::kSearchIcon, popup_contents_view_,
OmniboxPopupModel::Selection(model_index_,
OmniboxPopupModel::FOCUSED_BUTTON_KEYWORD)));
this, base::string16(), vector_icons::kSearchIcon,
make_predicate(OmniboxPopupModel::FOCUSED_BUTTON_KEYWORD)));
tab_switch_button_ =
AddChildView(std::make_unique<OmniboxSuggestionRowButton>(
this, l10n_util::GetStringUTF16(IDS_OMNIBOX_TAB_SUGGEST_HINT),
omnibox::kSwitchIcon, popup_contents_view_,
OmniboxPopupModel::Selection(
model_index_, OmniboxPopupModel::FOCUSED_BUTTON_TAB_SWITCH)));
tab_switch_button_->SetAccessibleName(
l10n_util::GetStringUTF16(IDS_ACC_TAB_SWITCH_BUTTON));
omnibox::kSwitchIcon,
make_predicate(OmniboxPopupModel::FOCUSED_BUTTON_TAB_SWITCH)));
pedal_button_ = AddChildView(std::make_unique<OmniboxSuggestionRowButton>(
this, base::string16(), omnibox::kProductIcon, popup_contents_view_,
OmniboxPopupModel::Selection(model_index_,
OmniboxPopupModel::FOCUSED_BUTTON_PEDAL)));
this, base::string16(), omnibox::kProductIcon,
make_predicate(OmniboxPopupModel::FOCUSED_BUTTON_PEDAL)));
}
OmniboxSuggestionButtonRowView::~OmniboxSuggestionButtonRowView() = default;
......@@ -167,17 +150,14 @@ void OmniboxSuggestionButtonRowView::UpdateFromModel() {
const auto names = SelectedKeywordView::GetKeywordLabelNames(
keyword, edit_model->client()->GetTemplateURLService());
keyword_button_->SetText(names.full_name);
keyword_button_->SetAccessibleName(
l10n_util::GetStringFUTF16(IDS_ACC_KEYWORD_BUTTON, names.short_name));
}
SetPillButtonVisibility(pedal_button_,
OmniboxPopupModel::FOCUSED_BUTTON_PEDAL);
if (pedal_button_->GetVisible()) {
const auto pedal_strings = match().pedal->GetLabelStrings();
pedal_button_->SetText(pedal_strings.hint);
pedal_button_->SetTooltipText(pedal_strings.suggestion_contents);
pedal_button_->SetAccessibleName(pedal_strings.accessibility_hint);
pedal_button_->SetText(match().pedal->GetLabelStrings().hint);
pedal_button_->SetTooltipText(
match().pedal->GetLabelStrings().suggestion_contents);
}
SetPillButtonVisibility(tab_switch_button_,
......@@ -230,28 +210,6 @@ void OmniboxSuggestionButtonRowView::ButtonPressed(views::Button* button,
}
}
views::Button* OmniboxSuggestionButtonRowView::GetActiveButton() const {
std::vector<OmniboxSuggestionRowButton*> visible_buttons;
if (keyword_button_->GetVisible())
visible_buttons.push_back(keyword_button_);
if (tab_switch_button_->GetVisible())
visible_buttons.push_back(tab_switch_button_);
if (pedal_button_->GetVisible())
visible_buttons.push_back(pedal_button_);
if (visible_buttons.empty())
return nullptr;
// Find first visible button that matches model selection.
auto selected_button =
std::find_if(visible_buttons.begin(), visible_buttons.end(),
[=](OmniboxSuggestionRowButton* button) {
return model()->selection() == button->selection();
});
return selected_button == visible_buttons.end() ? visible_buttons.front()
: *selected_button;
}
const OmniboxPopupModel* OmniboxSuggestionButtonRowView::model() const {
return popup_contents_view_->model();
}
......
chrome/browser/ui/views/omnibox/omnibox_suggestion_button_row_view.h
View file @ f6fa5f43
......@@ -32,8 +32,6 @@ class OmniboxSuggestionButtonRowView : public views::View,
// views::ButtonListener:
void ButtonPressed(views::Button* sender, const ui::Event& event) override;
views::Button* GetActiveButton() const;
private:
// Get the popup model from the view.
const OmniboxPopupModel* model() const;
......
components/omnibox/browser/autocomplete_controller.h
View file @ f6fa5f43
......@@ -177,6 +177,10 @@ class AutocompleteController : public AutocompleteProviderListener,
FRIEND_TEST_ALL_PREFIXES(AutocompleteProviderTest,
RedundantKeywordsIgnoredInResult);
FRIEND_TEST_ALL_PREFIXES(AutocompleteProviderTest, UpdateAssistedQueryStats);
FRIEND_TEST_ALL_PREFIXES(OmniboxPopupContentsViewTest,
EmitAccessibilityEvents);
FRIEND_TEST_ALL_PREFIXES(OmniboxPopupContentsViewTest,
EmitAccessibilityEventsOnButtonFocusHint);
FRIEND_TEST_ALL_PREFIXES(OmniboxViewTest, DoesNotUpdateAutocompleteOnBlur);
FRIEND_TEST_ALL_PREFIXES(OmniboxViewViewsTest, CloseOmniboxPopupOnTextDrag);
FRIEND_TEST_ALL_PREFIXES(OmniboxViewViewsTest, FriendlyAccessibleLabel);
......@@ -201,7 +205,6 @@ class AutocompleteController : public AutocompleteProviderListener,
PopupStepSelectionWithButtonRowAndKeywordButton);
FRIEND_TEST_ALL_PREFIXES(OmniboxPopupContentsViewTest,
EmitSelectedChildrenChangedAccessibilityEvent);
friend class OmniboxPopupContentsViewTest;
// Updates |result_| to reflect the current provider state and fires
// notifications. If |regenerate_result| then we clear the result
......
components/omnibox/browser/omnibox_popup_model.cc
View file @ f6fa5f43
......@@ -181,7 +181,6 @@ void OmniboxPopupModel::SetSelection(Selection new_selection,
base::string16());
} else if (old_selection.line != selection_.line ||
(old_selection.IsButtonFocused() &&
!new_selection.IsButtonFocused() &&
new_selection.state != KEYWORD_MODE)) {
// Otherwise, only update the edit model for line number changes, or
// when the old selection was a button and we're not entering keyword mode.
......
testing/buildbot/filters/lacros.browser_tests.filter
View file @ f6fa5f43
......@@ -121,10 +121,8 @@
-NtpExtensionBubbleViewBrowserTest.InvokeUi_ntp_override
-NtpExtensionBubbleViewBrowserTest.TestControlledNewTabPageMessageBubble
-NtpExtensionBubbleViewBrowserTest.TestControlledNewTabPageMessageBubbleLearnMore
-All/OmniboxPopupContentsViewTest.ClickOmnibox/0
-All/OmniboxPopupContentsViewTest.ClickOmnibox/1
-All/OmniboxPopupContentsViewTest.PopupMatchesLocationBarBackground/0
-All/OmniboxPopupContentsViewTest.PopupMatchesLocationBarBackground/1
-OmniboxPopupContentsViewTest.ClickOmnibox
-OmniboxPopupContentsViewTest.PopupMatchesLocationBarBackground
-OutOfProcessPPAPITest.FlashClipboard
-OutOfProcessPPAPITest.NetAddress
-OutOfProcessPPAPITest.Printing
......
testing/buildbot/filters/ozone-linux.wayland_browser_tests.filter
View file @ f6fa5f43
......@@ -13,10 +13,8 @@
-ExtensionDialogTest.TextInputViaKeyEvent
-ExternalProtocolDialogBrowserTest.TestFocus
-FolderUploadConfirmationViewTest.InitiallyFocusesCancel
-All/OmniboxPopupContentsViewTest.ClickOmnibox/0
-All/OmniboxPopupContentsViewTest.ClickOmnibox/1
-All/OmniboxPopupContentsViewTest.PopupMatchesLocationBarBackground/0
-All/OmniboxPopupContentsViewTest.PopupMatchesLocationBarBackground/1
-OmniboxPopupContentsViewTest.ClickOmnibox
-OmniboxPopupContentsViewTest.PopupMatchesLocationBarBackground
-OutOfProcessPPAPITest.FlashClipboard
-PPAPINaClNewlibTest.TrueTypeFont
-PPAPINaClPNaClNonSfiTest.TrueTypeFont
......
testing/buildbot/filters/ozone-linux.x11_browser_tests.filter
View file @ f6fa5f43
......@@ -15,8 +15,7 @@
-AutofillProviderBrowserTestWithSkipFlagOn.LabelTagChangeImpactFormComparing
-ExecuteScriptApiTest/DestructiveScriptTest.SynchronousRemoval/0
-ExtensionInstallDialogViewTest.InstallButtonDelay
-All/OmniboxPopupContentsViewTest.ClickOmnibox/0
-All/OmniboxPopupContentsViewTest.ClickOmnibox/1
-OmniboxPopupContentsViewTest.ClickOmnibox
-PageInfoBubbleViewBrowserTest.FocusDoesNotReturnToContentsOnReloadPrompt
-PaymentRequestCreditCardEditorTest.EditingExpiredCard
-PaymentRequestShippingAddressEditorTest.FocusFirstField_Name
......
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