Commit 5020f8a4 authored by Elly Fong-Jones's avatar Elly Fong-Jones Committed by Commit Bot

cbuiv desktopmedia: disallow selecting sources by double-click

See the bug for details about why.

Bug: 1102153
Change-Id: I71423c13405cfd57a1eee024205c48145af18009
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2313156
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791283}
parent 592b68f4
...@@ -79,10 +79,6 @@ void DesktopMediaListView::OnSelectionChanged() { ...@@ -79,10 +79,6 @@ void DesktopMediaListView::OnSelectionChanged() {
controller_->OnSourceSelectionChanged(); controller_->OnSourceSelectionChanged();
} }
void DesktopMediaListView::OnDoubleClick() {
controller_->AcceptSource();
}
gfx::Size DesktopMediaListView::CalculatePreferredSize() const { gfx::Size DesktopMediaListView::CalculatePreferredSize() const {
int total_rows = (int{children().size()} + active_style_->columns - 1) / int total_rows = (int{children().size()} + active_style_->columns - 1) /
active_style_->columns; active_style_->columns;
......
...@@ -31,9 +31,6 @@ class DesktopMediaListView ...@@ -31,9 +31,6 @@ class DesktopMediaListView
// Called by DesktopMediaSourceView when selection has changed. // Called by DesktopMediaSourceView when selection has changed.
void OnSelectionChanged(); void OnSelectionChanged();
// Called by DesktopMediaSourceView when a source has been double-clicked.
void OnDoubleClick();
// views::View: // views::View:
gfx::Size CalculatePreferredSize() const override; gfx::Size CalculatePreferredSize() const override;
void Layout() override; void Layout() override;
......
...@@ -57,8 +57,6 @@ void DesktopMediaPickerViewsTestApi::PressMouseOnSourceAtIndex( ...@@ -57,8 +57,6 @@ void DesktopMediaPickerViewsTestApi::PressMouseOnSourceAtIndex(
// within a larger view would be breakage-prone; just ask the TableView to // within a larger view would be breakage-prone; just ask the TableView to
// to select. // to select.
GetTableView()->Select(index); GetTableView()->Select(index);
if (double_click)
picker_->dialog_->GetSelectedController()->AcceptSource();
} }
} }
......
...@@ -164,9 +164,10 @@ TEST_F(DesktopMediaPickerViewsTest, SelectMediaSourceViewOnSingleClick) { ...@@ -164,9 +164,10 @@ TEST_F(DesktopMediaPickerViewsTest, SelectMediaSourceViewOnSingleClick) {
} }
} }
TEST_F(DesktopMediaPickerViewsTest, DoneCallbackCalledOnDoubleClick) { // Regression test for https://crbug.com/1102153
TEST_F(DesktopMediaPickerViewsTest, DoneCallbackNotCalledOnDoubleClick) {
const DesktopMediaID kFakeId(DesktopMediaID::TYPE_WEB_CONTENTS, 222); const DesktopMediaID kFakeId(DesktopMediaID::TYPE_WEB_CONTENTS, 222);
EXPECT_CALL(*this, OnPickerDone(kFakeId)); EXPECT_CALL(*this, OnPickerDone(kFakeId)).Times(0);
media_lists_[DesktopMediaID::TYPE_WEB_CONTENTS]->AddSourceByFullMediaID( media_lists_[DesktopMediaID::TYPE_WEB_CONTENTS]->AddSourceByFullMediaID(
kFakeId); kFakeId);
...@@ -177,9 +178,10 @@ TEST_F(DesktopMediaPickerViewsTest, DoneCallbackCalledOnDoubleClick) { ...@@ -177,9 +178,10 @@ TEST_F(DesktopMediaPickerViewsTest, DoneCallbackCalledOnDoubleClick) {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
TEST_F(DesktopMediaPickerViewsTest, DoneCallbackCalledOnDoubleTap) { // Regression test for https://crbug.com/1102153
TEST_F(DesktopMediaPickerViewsTest, DoneCallbackNotCalledOnDoubleTap) {
const DesktopMediaID kFakeId(DesktopMediaID::TYPE_SCREEN, 222); const DesktopMediaID kFakeId(DesktopMediaID::TYPE_SCREEN, 222);
EXPECT_CALL(*this, OnPickerDone(kFakeId)); EXPECT_CALL(*this, OnPickerDone(kFakeId)).Times(0);
test_api_.SelectTabForSourceType(DesktopMediaID::TYPE_SCREEN); test_api_.SelectTabForSourceType(DesktopMediaID::TYPE_SCREEN);
test_api_.GetAudioShareCheckbox()->SetChecked(false); test_api_.GetAudioShareCheckbox()->SetChecked(false);
...@@ -244,6 +246,8 @@ TEST_F(DesktopMediaPickerViewsTest, OkButtonDisabledWhenNoSelection) { ...@@ -244,6 +246,8 @@ TEST_F(DesktopMediaPickerViewsTest, OkButtonDisabledWhenNoSelection) {
test_api_.SelectTabForSourceType(source_type); test_api_.SelectTabForSourceType(source_type);
media_lists_[source_type]->AddSourceByFullMediaID( media_lists_[source_type]->AddSourceByFullMediaID(
DesktopMediaID(source_type, 111)); DesktopMediaID(source_type, 111));
EXPECT_FALSE(
GetPickerDialogView()->IsDialogButtonEnabled(ui::DIALOG_BUTTON_OK));
test_api_.FocusSourceAtIndex(0); test_api_.FocusSourceAtIndex(0);
EXPECT_TRUE( EXPECT_TRUE(
......
...@@ -147,24 +147,11 @@ void DesktopMediaSourceView::OnFocus() { ...@@ -147,24 +147,11 @@ void DesktopMediaSourceView::OnFocus() {
} }
bool DesktopMediaSourceView::OnMousePressed(const ui::MouseEvent& event) { bool DesktopMediaSourceView::OnMousePressed(const ui::MouseEvent& event) {
if (event.GetClickCount() == 1) { RequestFocus();
RequestFocus();
} else if (event.GetClickCount() == 2) {
RequestFocus();
parent_->OnDoubleClick();
}
return true; return true;
} }
void DesktopMediaSourceView::OnGestureEvent(ui::GestureEvent* event) { void DesktopMediaSourceView::OnGestureEvent(ui::GestureEvent* event) {
if (event->type() == ui::ET_GESTURE_TAP &&
event->details().tap_count() == 2) {
RequestFocus();
parent_->OnDoubleClick();
event->SetHandled();
return;
}
// Detect tap gesture using ET_GESTURE_TAP_DOWN so the view also gets focused // Detect tap gesture using ET_GESTURE_TAP_DOWN so the view also gets focused
// on the long tap (when the tap gesture starts). // on the long tap (when the tap gesture starts).
if (event->type() == ui::ET_GESTURE_TAP_DOWN) { if (event->type() == ui::ET_GESTURE_TAP_DOWN) {
......
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