Skip to content

Commit 99b5abf

Browse files
simonhongcdesouza-chromium
authored andcommitted
Fixed wrong cursor position when dragging window by tab detach
fix brave/brave-browser#44393 This is f/u fix as #27909 is reverted in cr135. We adjusted window offset while dragging in vertical tab mode because TabDragController refers widget from TabDraggingContext but that widget is vertical tab widget not browser window widget. However, upstream refactored TabDragController so we need to adjust based on latest code again. Upstream changes - https://chromium-review.googlesource.com/c/chromium/src/+/6160815 Our previous offset adjust - #27909 Enabled tab detach browser test again(VerticalTabStripDragAndDropBrowserTest.DragTabToDetach) to check mouse cursor is located over the dragged tab properly during the dragging. But not sure it works well on CI.
1 parent 5a2073a commit 99b5abf

File tree

5 files changed

+41
-1
lines changed

5 files changed

+41
-1
lines changed

browser/ui/views/tabs/dragging/tab_drag_controller.cc

+17
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,23 @@ gfx::Point TabDragController::GetAttachedDragPoint(
119119
return {x, y};
120120
}
121121

122+
gfx::Vector2d TabDragController::CalculateWindowDragOffset() {
123+
gfx::Vector2d offset = TabDragControllerChromium::CalculateWindowDragOffset();
124+
if (!is_showing_vertical_tabs_) {
125+
return offset;
126+
}
127+
128+
// Re-calculate offset as above result is based on vertical tab widget.
129+
// Convert it based on browser window widget(top level widget).
130+
gfx::Point new_offset(offset.x(), offset.y());
131+
views::View::ConvertPointFromWidget(attached_context_, &new_offset);
132+
views::View::ConvertPointToScreen(attached_context_, &new_offset);
133+
views::View::ConvertPointFromScreen(
134+
attached_context_->GetWidget()->GetTopLevelWidget()->GetRootView(),
135+
&new_offset);
136+
return new_offset.OffsetFromOrigin();
137+
}
138+
122139
void TabDragController::MoveAttached(const gfx::Point& point_in_screen,
123140
bool just_attached) {
124141
TabDragControllerChromium::MoveAttached(point_in_screen, just_attached);

browser/ui/views/tabs/dragging/tab_drag_controller.h

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ class TabDragController : public TabDragControllerChromium {
3434
void MoveAttached(const gfx::Point& point_in_screen,
3535
bool just_attached) override;
3636
views::Widget* GetAttachedBrowserWidget() override;
37+
gfx::Vector2d CalculateWindowDragOffset() override;
3738

3839
Liveness GetLocalProcessWindow(const gfx::Point& screen_point,
3940
bool exclude_dragged_view,

browser/ui/views/tabs/vertical_tab_strip_browsertest.cc

+13-1
Original file line numberDiff line numberDiff line change
@@ -956,7 +956,11 @@ IN_PROC_BROWSER_TEST_F(VerticalTabStripDragAndDropBrowserTest,
956956
// TODO(sko) On Linux test environment, the test doesn't work well
957957
// TODO(sko) On Windows CI, SendMouse() doesn't work.
958958
// TODO(sko) As of Dec, 2023 this test is flaky on Mac CI.
959+
#if BUILDFLAG(IS_WIN)
960+
#define MAYBE_DragTabToDetach DragTabToDetach
961+
#else
959962
#define MAYBE_DragTabToDetach DISABLED_DragTabToDetach
963+
#endif
960964

961965
IN_PROC_BROWSER_TEST_F(VerticalTabStripDragAndDropBrowserTest,
962966
MAYBE_DragTabToDetach) {
@@ -979,8 +983,16 @@ IN_PROC_BROWSER_TEST_F(VerticalTabStripDragAndDropBrowserTest,
979983
std::ranges::count_if(*browser_list, [&](Browser* b) {
980984
return b->profile() == browser()->profile();
981985
}));
982-
ReleaseMouse();
983986
auto* new_browser = browser_list->GetLastActive();
987+
auto* browser_view =
988+
BrowserView::GetBrowserViewForBrowser(new_browser);
989+
auto* tab = browser_view->tabstrip()->tab_at(0);
990+
ASSERT_TRUE(tab);
991+
// During the tab detaching, mouse should be over the dragged
992+
// tab.
993+
EXPECT_TRUE(tab->IsMouseHovered());
994+
EXPECT_TRUE(tab->dragging());
995+
ReleaseMouse();
984996
new_browser->window()->Close();
985997
}));
986998
}

chromium_src/chrome/browser/ui/views/tabs/dragging/tab_drag_controller.cc

+2
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@
3737
// StackAtTop() should be called vertical tab widget's top level widget.
3838
// This also works in horizontal tab mode because it's already top level window.
3939
#define StackAtTop GetTopLevelWidget()->StackAtTop
40+
#define GetWindowBoundsInScreen GetTopLevelWidget()->GetWindowBoundsInScreen
4041

4142
#include "src/chrome/browser/ui/views/tabs/dragging/tab_drag_controller.cc"
4243

44+
#undef GetWindowBoundsInScreen
4345
#undef StackAtTop
4446
#undef GetHorizontalDragThreshold
4547
#undef GetBrowserViewForNativeWindow

chromium_src/chrome/browser/ui/views/tabs/dragging/tab_drag_controller.h

+8
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,13 @@ using TabDragControllerBrave = TabDragController;
2929
return {}; \
3030
} \
3131
virtual views::Widget* GetAttachedBrowserWidget
32+
33+
#define CalculateWindowDragOffset \
34+
CalculateWindowDragOffset_Unused() { \
35+
return {}; \
36+
} \
37+
virtual gfx::Vector2d CalculateWindowDragOffset
38+
3239
#define GetLocalProcessWindow virtual GetLocalProcessWindow
3340
#define DetachAndAttachToNewContext virtual DetachAndAttachToNewContext
3441
#define ContinueDragging virtual ContinueDragging
@@ -38,6 +45,7 @@ using TabDragControllerBrave = TabDragController;
3845
#undef ContinueDragging
3946
#undef DetachAndAttachToNewContext
4047
#undef GetLocalProcessWindow
48+
#undef CalculateWindowDragOffset
4149
#undef GetAttachedBrowserWidget
4250
#undef TabDragController
4351
#undef InitDragData

0 commit comments

Comments
 (0)