Commit aab20273 authored by Connie Wan's avatar Connie Wan Committed by Commit Bot

Disallow dragging pinned tabs into groups

Bug: 1043290
Change-Id: I74c497c43f6a34cb2b7c570aeaed42573ada35ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2016402
Commit-Queue: Connie Wan <connily@chromium.org>
Reviewed-by: default avatarCharlene Yan <cyan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735538}
parent 71ec2719
......@@ -2128,8 +2128,20 @@ void TabDragController::UpdateGroupForDraggedTabs() {
const ui::ListSelectionModel::SelectedIndices& selected =
attached_model->selection_model().selected_indices();
// Pinned tabs cannot be grouped, so we only change the group membership of
// unpinned tabs.
std::vector<int> selected_unpinned;
for (const int& selected_index : selected) {
if (!attached_model->IsTabPinned(selected_index))
selected_unpinned.push_back(selected_index);
}
if (selected_unpinned.empty())
return;
const base::Optional<tab_groups::TabGroupId> updated_group =
GetTabGroupForTargetIndex(selected);
GetTabGroupForTargetIndex(selected_unpinned);
// Removing a tab from a group could change the index of the selected tabs.
// Store this to move the tab back to the proper position.
......@@ -2138,19 +2150,18 @@ void TabDragController::UpdateGroupForDraggedTabs() {
// All selected tabs should all be in the same group unless it is the initial
// move.
if (initial_move_) {
attached_model->RemoveFromGroup(selected);
attached_model->RemoveFromGroup(selected_unpinned);
attached_model->MoveSelectedTabsTo(to_index);
}
if (updated_group == attached_model->GetTabGroupForTab(selected[0])) {
if (updated_group == attached_model->GetTabGroupForTab(selected_unpinned[0]))
return;
}
if (updated_group.has_value()) {
attached_model->MoveTabsAndSetGroup(selected, selected[0],
attached_model->MoveTabsAndSetGroup(selected_unpinned, selected_unpinned[0],
updated_group.value());
} else {
attached_model->RemoveFromGroup(selected);
attached_model->RemoveFromGroup(selected_unpinned);
attached_model->MoveSelectedTabsTo(to_index);
}
}
......
......@@ -1243,6 +1243,35 @@ IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTestWithTabGroupsEnabled,
testing::ElementsAre(0, 1));
}
// Creates a browser with four tabs. The first tab is pinned, and the last
// three belong in the same Tab Group. Dragging the pinned tab to the middle
// of the group will not result in the pinned tab getting grouped or moving
// into the group.
IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTestWithTabGroupsEnabled,
DragPinnedTabDoesNotGroup) {
TabStrip* tab_strip = GetTabStripForBrowser(browser());
TabStripModel* model = browser()->tab_strip_model();
AddTabsAndResetBrowser(browser(), 3);
model->SetTabPinned(0, true);
tab_groups::TabGroupId group = model->AddToNewGroup({1, 2, 3});
StopAnimating(tab_strip);
ASSERT_EQ(4, model->count());
ASSERT_EQ(3u, model->group_model()->GetTabGroup(group)->ListTabs().size());
ASSERT_TRUE(PressInput(GetCenterInScreenCoordinates(tab_strip->tab_at(0))));
ASSERT_TRUE(DragInputTo(GetCenterInScreenCoordinates(tab_strip->tab_at(2))));
ASSERT_TRUE(ReleaseInput());
StopAnimating(tab_strip);
// The pinned tab should not have moved or joined the group.
EXPECT_EQ("0 1 2 3", IDString(model));
EXPECT_THAT(model->group_model()->GetTabGroup(group)->ListTabs(),
testing::ElementsAre(1, 2, 3));
EXPECT_EQ(base::nullopt, model->GetTabGroupForTab(0));
}
// Drags a tab within the window (without dragging the whole window) then
// pressing a key ends the drag.
IN_PROC_BROWSER_TEST_P(DetachToBrowserTabDragControllerTest,
......
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