-
Notifications
You must be signed in to change notification settings - Fork 940
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert "[cr134][WIP] Fix TabDragController
tab dragging calculation"
#27909
Conversation
This reverts commit 4d6e57f. Revert reason: This change was added for handling https://chromium-review.googlesource.com/c/chromium/src/+/6160815 but it's reverted by https://chromium-review.googlesource.com/c/chromium/src/+/6234751 fix brave/brave-browser#44362 fix brave/brave-browser#44282
Released in v1.78.11 |
This change has broken |
@cdesouza-chromium was the original change (https://chromium-review.googlesource.com/c/chromium/src/+/6160815) relanded upstream after being reverted in https://chromium-review.googlesource.com/c/chromium/src/+/6234751? If 135 already had a reland (and doesn't have the issue), this PR and SHA can probably be reverted from the cr135 branch |
Verified with
Case 1 - brave/brave-browser#44362
Screen.Recording.2025-03-03.at.4.20.44.PM.movCase 2 - brave/brave-browser#44282 Using 1.76.71 reproduced the mis-positioned mouse cursor: Screen.Recording.2025-03-03.at.4.27.20.PM.movUsing 1.78.11 saw that the mouse cursor is positioned properly (note, in certain cases you will encounter brave/brave-browser#44032): Screen.Recording.2025-03-03.at.4.29.38.PM.mov |
fix brave/brave-browser#44393 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
fix brave/brave-browser#44393 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
fix brave/brave-browser#44393 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
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
This reverts commit 4d6e57f.
Revert reason: This change was added for handling https://chromium-review.googlesource.com/c/chromium/src/+/6160815 but it's reverted by https://chromium-review.googlesource.com/c/chromium/src/+/6234751
By reverting this, we could avoid invalid
non_client_view()
access that causes brave/brave-browser#44362.Also can get proper dragged tab under cursor that fixes brave/brave-browser#44282
fix brave/brave-browser#44362
fix brave/brave-browser#44282
Resolves
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan:
See the linked issue.