macos mojave: drag tab horizontally will move entire browser window
Reported by
x0s...@gmail.com,
Jun 24 2018
|
|||||||||||||||
Issue descriptionUserAgent: 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
,
Jun 24 2018
,
Jun 24 2018
,
Jun 25 2018
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...!
,
Jun 26 2018
This behavior does not happen in Chrome 68.0.3440.33 on Mojave beta 2 (18A314h).
,
Jun 26 2018
It's reproducible when using MacViews.
,
Jul 11
I can repro this on 69.0.3488.0 with MacViews, with and without MD refresh.
,
Jul 13
mac triage: to sdy@
,
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
,
Jul 27
This should be fixed in tomorrow's canary.
,
Jul 30
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!
,
Aug 3
,
Sep 4
Issue 880580 has been merged into this issue.
,
Sep 4
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.
,
Sep 4
Filed issue 880622 re. blockerbot not adding Merge-TBD.
,
Sep 4
Blockerbot only adds TBD label if "Release Block" label is applied to the bug along with milestone labels, Few example - https://bugs.chromium.org/p/chromium/issues/list?can=1&q=OS%3DWindows%2CMac%2CLinux%2CAll+Merge%3DTBD+M%3D70&sort=pri&colspec=ID+Pri+M+Stars+ReleaseBlock+Component+Status+Owner+Summary+OS+Modified&x=m&y=releaseblock&cells=ids
,
Sep 4
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.
,
Sep 4
How safe is the cl listed at #9 to merge to M69 this late in release cycle?
,
Sep 4
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
,
Sep 4
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.
,
Sep 5
Issue 880834 has been merged into this issue.
,
Sep 6
Approving merge to M69 branch 3497 based on comment #17, #20 and per offline chat with sdy@.
,
Sep 6
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
,
Sep 6
,
Sep 7
Issue 881799 has been merged into this issue.
,
Sep 7
This looks good to me on 69.0.3497.87.
,
Sep 10
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.
,
Sep 10
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.
,
Sep 11
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. |
|||||||||||||||
►
Sign in to add a comment |
|||||||||||||||
Comment 1 by meh...@chromium.org
, Jun 24 2018Labels: Proj-MacMojave