New issue
Advanced search Search tips

Issue 855901 link

Starred by 13 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

macos mojave: drag tab horizontally will move entire browser window

Reported by x0s...@gmail.com, Jun 24 2018

Issue description

UserAgent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/69.0.3469.2 Safari/537.36

Steps to reproduce the problem:
1. open some tabs
2. drag tab horizontally

What is the expected behavior?

What went wrong?
entire browser window will be moved

Did this work before? N/A 

Chrome version: 69.0.3469.2  Channel: canary
OS Version: OS X 10.14.0
Flash Version: 

OS version: macOS 10.14 beta 2(18A314h)
Video: https://www.youtube.com/watch?v=O2c3PZw-wAU
 
chrome_version.txt
2.2 KB View Download

Comment 1 by meh...@chromium.org, Jun 24 2018

Components: -UI
Labels: Proj-MacMojave

Comment 2 by meh...@chromium.org, Jun 24 2018

Components: UI>Browser>TabStrip
Labels: Needs-Triage-M69
Cc: phanindra.mandapaka@chromium.org
Labels: Triaged-ET TE-Hardware-Dependency
Thanks for filling the issue...

It seems that the issue is specific to mac mojave. As we don't have that particular OS-mac to test the issue from our end. Hence, adding label TE-Hardware-Dependency.

Thanks...!

Comment 5 by ja...@imaj.es, Jun 26 2018

This behavior does not happen in Chrome 68.0.3440.33 on Mojave beta 2 (18A314h).

Comment 6 by x0s...@gmail.com, Jun 26 2018

It's reproducible when using MacViews.
Status: Untriaged (was: Unconfirmed)
I can repro this on 69.0.3488.0 with MacViews, with and without MD refresh.
Labels: -Pri-2 M-69 Target-69 Pri-1
Owner: sdy@chromium.org
Status: Assigned (was: Untriaged)
mac triage: to sdy@
Project Member

Comment 9 by bugdroid1@chromium.org, Jul 27

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

commit 312cc70dbd68686332c4a489bafba694181e0339
Author: Sidney San Martín <sdy@chromium.org>
Date: Fri Jul 27 14:59:20 2018

Fix windows sometimes being draggable from the tab strip on 10.14.

AppKit is eager to allow server-side dragging in the title bar area
(meaning marking the region as draggable at the window server level),
but Views currently decides on a click-by-click basis (see |-hitTest:|).
One AppKit requirement is that, for a view to block title bar dragging,
it needs to return YES from |-acceptsFirstResponder:|, even if it
doesn't actually accept first responder later. BridgedContentView
returned different values at different times, which made dragging
inconsistent. This was sidestepped by overriding |-[NSWindow
_copyDragRegion]|, but that method no longer exists on 10.14.

This change makes |-acceptsFirstResponder| always return YES and removes
the hack. A similar hack is left over to support 10.10. If this change
turns out to not be viable, an override like this should work instead:

    @implementation BrowserNativeWidgetWindow
    // …
    - (NSRect)_opaqueRectForWindowMoveWhenInTitlebar {
      return NSZeroRect;
    }
    //…
    @end

Bug:  855901 
Change-Id: I4ef93f1bee1b2cc7b39fab0eec7fe82a526b61e4
Reviewed-on: https://chromium-review.googlesource.com/1152076
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578642}
[modify] https://crrev.com/312cc70dbd68686332c4a489bafba694181e0339/chrome/browser/ui/views/frame/browser_native_widget_window_mac.mm
[modify] https://crrev.com/312cc70dbd68686332c4a489bafba694181e0339/ui/views/cocoa/bridged_content_view.mm

Status: Fixed (was: Assigned)
This should be fixed in tomorrow's canary.
Cc: viswa.karala@chromium.org
Labels: Needs-Feedback
x0ssrk@gmail.com: As per comment# 9 & 10, this issue seems to be fixed on latest chrome canary, could you please try to check this issue on latest chrome canary and let us know the if the issue got fixed there. You can download latest chrome canary from URL: https://www.chromium.org/getting-involved/dev-channel.

Thanks!
Cc: sdy@chromium.org
 Issue 868429  has been merged into this issue.
Cc: ellyjo...@chromium.org a...@chromium.org pbomm...@chromium.org
 Issue 880580  has been merged into this issue.
Prudhvi: Since people are using 69 on Mojave, does it at all make sense to merge this for the next respin? It's fine in M70. I'm fine either way.
Filed issue 880622 re. blockerbot not adding Merge-TBD.
Labels: Merge-Request-69
After a discussion with govind@:

This probably should have been merged to M69 at the time it was landed. It isn't critical enough to block M69 rollout, since Mojave is still in beta, but should be merged in case Mojave is released before M70. Hence, requesting a merge to be picked up in a future respin.
How safe is the cl listed at #9 to merge to M69 this late in release cycle?
Project Member

Comment 19 by sheriffbot@chromium.org, Sep 4

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: Request affecting a post-stable build
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Because it touches the path by which all of Chrome's UI gets keyboard input, I considered it risky when I landed it. But, the fact that we haven't seen had any reports of issues makes me feel fairly confident about it now.

I think the risk is low enough to merge.
 Issue 880834  has been merged into this issue.
Labels: -Merge-Review-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comment #17, #20 and per offline chat with sdy@.
Project Member

Comment 23 by bugdroid1@chromium.org, Sep 6

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8d3f27fbbbaa20f142e32b96d2936782e168e3aa

commit 8d3f27fbbbaa20f142e32b96d2936782e168e3aa
Author: Sidney San Martín <sdy@chromium.org>
Date: Thu Sep 06 01:23:16 2018

Fix windows sometimes being draggable from the tab strip on 10.14.

AppKit is eager to allow server-side dragging in the title bar area
(meaning marking the region as draggable at the window server level),
but Views currently decides on a click-by-click basis (see |-hitTest:|).
One AppKit requirement is that, for a view to block title bar dragging,
it needs to return YES from |-acceptsFirstResponder:|, even if it
doesn't actually accept first responder later. BridgedContentView
returned different values at different times, which made dragging
inconsistent. This was sidestepped by overriding |-[NSWindow
_copyDragRegion]|, but that method no longer exists on 10.14.

This change makes |-acceptsFirstResponder| always return YES and removes
the hack. A similar hack is left over to support 10.10. If this change
turns out to not be viable, an override like this should work instead:

    @implementation BrowserNativeWidgetWindow
    // …
    - (NSRect)_opaqueRectForWindowMoveWhenInTitlebar {
      return NSZeroRect;
    }
    //…
    @end

Bug:  855901 
Change-Id: I4ef93f1bee1b2cc7b39fab0eec7fe82a526b61e4
Reviewed-on: https://chromium-review.googlesource.com/1152076
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#578642}(cherry picked from commit 312cc70dbd68686332c4a489bafba694181e0339)
Reviewed-on: https://chromium-review.googlesource.com/1208423
Reviewed-by: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#891}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/8d3f27fbbbaa20f142e32b96d2936782e168e3aa/chrome/browser/ui/views/frame/browser_native_widget_window_mac.mm
[modify] https://crrev.com/8d3f27fbbbaa20f142e32b96d2936782e168e3aa/ui/views/cocoa/bridged_content_view.mm

Cc: robliao@chromium.org
 Issue 881117  has been merged into this issue.
 Issue 881799  has been merged into this issue.
This looks good to me on 69.0.3497.87.
Labels: TE-Verified-M69 TE-Verified-69.0.3497.87
Update:
Rechecked the above issue on MacOS Mojave (10.14) machine using M-69 build #69.0.3497.87 and issue is fixed. Hence adding TE-Verified labels.

please refer below attached screen-cast for reference.

Thank You.
Fixed_behavior.mov
5.6 MB View Download
Update:
Re-tested this issue on MacOS Mojave (10.14) machine using latest M-69 build #69.0.3497.91 and issue is fixed.

Thank You.
Labels: TE-Verified-69.0.3497.92
Update:
Rechecked the above issue on MacOS Mojave (10.14) machine using M-69 Stable build #69.0.3497.92 and issue is fixed. Hence adding TE-Verified labels.

please refer below attached screen-cast for reference.

Thank You.
69.0.3497.92 behavior.mov
7.2 MB View Download

Sign in to add a comment