New issue
Advanced search Search tips

Issue 819339 link

Starred by 5 users

Issue metadata

Status: Verified
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

No longer possible to drag multiple tabs at once

Project Member Reported by amineer@chromium.org, Mar 6 2018

Issue description

Chrome Version: 66.0.3355.0
OS: OSX 10.13.3

What steps will reproduce the problem?
(1) Select multiple tabs via shift or command click
(2) Drag tabs to open in new window

What is the expected result?
All selected tabs are dragged into a new window

What happens instead?
Only the tab that is dragged is opened in a new window
 
Labels: -Type-Bug -Pri-3 Needs-Bisect Pri-1 Type-Bug-Regression
Works fine in Chrome Stable 65. --> +NeedsBisect

Comment 2 by a...@chromium.org, Mar 7 2018

Labels: -Needs-Bisect
Owner: sdy@chromium.org
Status: Assigned (was: Untriaged)
Bisect:

You are probably looking for a change made after 533363 (known good), but no later than 533376 (first known bad).
CHANGELOG URL: https://chromium.googlesource.com/chromium/src/+log/06f36141f82b1ff89597f8bce32e81d166f14505..977167ae0a734221c35f723ae545ac539b8b7731

That's a wide band, but behavior suggests https://crrev.com/c/895925 which changed mouse event stuff in the tab strip and is r533373. Local revert confirms.
I think step 1 from comment 0 should mean:

Select multiple tabs via shift or ***cmd*** click
Correct, I don't always Mac properly.
Description: Show this description

Comment 6 by sdy@chromium.org, Mar 7 2018

Eep, okay. I'm reverting that change right now. The workaround is to hold shift/control while you drag.

The behavior I *want* is for mouse down to switch tabs and mouse up to clear the selection. The model (ListSelectionModel) resets the selection implicitly when you change the active item, so this would need some bigger changes to get right.
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 7 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a58fea8dff2faf5a465fea718a31ebbd58f499ab

commit a58fea8dff2faf5a465fea718a31ebbd58f499ab
Author: Sidney San Martín <sdy@chromium.org>
Date: Wed Mar 07 21:49:39 2018

Revert "Switch tabs on mouse down instead of mouse up."

This reverts commit a47974deb41061714d3390b7373c174b750edd11.

Reason for revert:  https://crbug.com/819339 

This breaks the flow of:

1. Click a tab.
2. Shift-click to select a range of tabs.
3. Release shift and drag the tabs out into a new window.

…because the ListSelectionModel which backs the tab strip clears the selection when the active item is changed. It's going to take some work to make it behave the right way.

Original change's description:
> Switch tabs on mouse down instead of mouse up.
>
> This is consistent with other Mac apps and with Views, and makes the tab
> strip feel more responsive.
>
> Bug:  799230 
> Change-Id: I45a375f5892c10e5886e18f27ba049eb6215a17f
> Reviewed-on: https://chromium-review.googlesource.com/895925
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Commit-Queue: Sidney San Martín <sdy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#533373}

TBR=ellyjones@chromium.org,sdy@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  819339 ,  799230 
Change-Id: I9921e0d41669d6783ff0f8f3f0d4b0746eb72b68
Reviewed-on: https://chromium-review.googlesource.com/953304
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541597}
[modify] https://crrev.com/a58fea8dff2faf5a465fea718a31ebbd58f499ab/chrome/browser/ui/cocoa/tabs/tab_strip_drag_controller.mm

Comment 8 by sdy@chromium.org, Mar 7 2018

Labels: Merge-Request-66
Status: Fixed (was: Assigned)
Labels: TE-Verified-M67 TE-Verified-67.0.3365.0
Tested the issue using #67.0.3365.0 on Mac 10.13.3 as per the steps mentioned in comment #0. Observed the all selected tabs are dragged into a new window.

Please find the screencast. Hence adding Verified labels.

Thanks!!
819339.mp4
4.3 MB View Download
Project Member

Comment 10 by sheriffbot@chromium.org, Mar 8 2018

Labels: -Merge-Request-66 Merge-Review-66 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-66 Merge-Approved-66
Approving merge to M66 branch 3359 based on comment #9. Please merge ASAP so we can pick it up for M66 Dev/Beta release. Thank you.
Project Member

Comment 12 by bugdroid1@chromium.org, Mar 9 2018

Labels: -merge-approved-66 merge-merged-3359
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b7bed18c8c076a6548fc1502567e7b8857be1a1

commit 8b7bed18c8c076a6548fc1502567e7b8857be1a1
Author: Sidney San Martín <sdy@chromium.org>
Date: Fri Mar 09 18:55:47 2018

Revert "Switch tabs on mouse down instead of mouse up."

This reverts commit a47974deb41061714d3390b7373c174b750edd11.

Reason for revert:  https://crbug.com/819339 

This breaks the flow of:

1. Click a tab.
2. Shift-click to select a range of tabs.
3. Release shift and drag the tabs out into a new window.

…because the ListSelectionModel which backs the tab strip clears the selection when the active item is changed. It's going to take some work to make it behave the right way.

Original change's description:
> Switch tabs on mouse down instead of mouse up.
>
> This is consistent with other Mac apps and with Views, and makes the tab
> strip feel more responsive.
>
> Bug:  799230 
> Change-Id: I45a375f5892c10e5886e18f27ba049eb6215a17f
> Reviewed-on: https://chromium-review.googlesource.com/895925
> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
> Commit-Queue: Sidney San Martín <sdy@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#533373}

TBR=ellyjones@chromium.org,sdy@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  819339 ,  799230 
Change-Id: I9921e0d41669d6783ff0f8f3f0d4b0746eb72b68
Reviewed-on: https://chromium-review.googlesource.com/953304
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#541597}(cherry picked from commit a58fea8dff2faf5a465fea718a31ebbd58f499ab)
Reviewed-on: https://chromium-review.googlesource.com/957323
Cr-Commit-Position: refs/branch-heads/3359@{#134}
Cr-Branched-From: 66afc5e5d10127546cc4b98b9117aff588b5e66b-refs/heads/master@{#540276}
[modify] https://crrev.com/8b7bed18c8c076a6548fc1502567e7b8857be1a1/chrome/browser/ui/cocoa/tabs/tab_strip_drag_controller.mm

Comment 13 by sdy@chromium.org, Mar 9 2018

Status: Verified (was: Fixed)
Thank you!
Labels: TE-Verified-M66 TE-Verified-66.0.3359.26
Able to reproduce the issue on mac 10.13.3 using chrome reported version #66.0.3355.0.
Verified the fix as per comment #0 on Mac 10.13.3 using chrome version #66.0.3359.26.
Attaching screen cast for reference.
Observed that all selected tabs are dragged into a new window after releasing "command" button as expected.
Hence, the fix is working as expected. 
Adding the verified labels.

Thanks...!!
819339@M66.mp4
1.9 MB View Download

Sign in to add a comment