New issue
Advanced search Search tips

Issue 823770 link

Starred by 7 users

Issue metadata

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



Sign in to add a comment

MacViews: Traffic light buttons and tab dragging are broken

Project Member Reported by lgrey@chromium.org, Mar 20 2018

Issue description

What steps will reproduce the problem?
(1) Build ToT Chromium with mac_views_browser = true
(2) Launch Chromium with --enable-features=ViewsBrowserWindows
(3) Try to close/minimize/full-screen windows or drag tabs

What does work:
Switching tabs
Interacting with the profile button
Closing tabs via X
 
Status: Assigned (was: Untriaged)
Labels: -Pri-2 Target-67 Pri-1
MacViews triage: this is important; let's fix it for M-67 before we start canarying this.
Labels: MacViews-Browser
FYI: The fix I put up for  issue 775984  seems to also fix this.

Comment 5 by gov...@chromium.org, Mar 26 2018

Labels: M-67
Thank you ellyjones@.

Pls see https://bugs.chromium.org/p/chromium/issues/detail?id=775984#c7.
Status: Fixed (was: Assigned)
I confirmed that <https://chromium-review.googlesource.com/980701> also fixes this issue.

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

Status: Assigned (was: Fixed)
It looks like this may not be quite the right fix — with this change, it's possible to drag the window from controls like the window buttons, *and* the control gets activated when you let go after a drag. Reopening.

Comment 8 by gov...@chromium.org, Mar 29 2018

** Bulk Edit **

There are only two M67 dev releases left on 04/03 & 04/10 before M67 branch on 04/12. Please try to land the fix ASAP to trunk so we can move forward with 50%-50% experiment on M67 Canary/Dev (if possible at all). Thank you.
** Bulk Edit **

There is only one dev release left 04/10 before M67 branch on 04/12. Please try to land the fix ASAP to trunk so we can move forward with 50%-50% experiment on M67 Canary/Dev (if possible at all). Thank you.

FYI: Change/Fix has to be landed in trunk latest by 4:00 PM PT, Friday (04/06) so we can pick it up for next week dev release. 
Cc: sdy@chromium.org
 Issue 828059  has been merged into this issue.
sdy@, any progress here? Thank you.

Comment 12 by sdy@chromium.org, Apr 10 2018

Status: Started (was: Assigned)
Hi govind@, yes, a CL is in review:

https://chromium-review.googlesource.com/c/chromium/src/+/1000212
Project Member

Comment 13 by bugdroid1@chromium.org, Apr 11 2018

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

commit 91ee9c23182a6f91b45da5540ace7e80462045c0
Author: Sidney San Martín <sdy@chromium.org>
Date: Wed Apr 11 06:54:49 2018

[MacViews] Fix browser windows dragging unexpectedly, especially when trying to drag tabs.

There are two paths which lead to dragging a Mac app's window around:

- The slow path is initiated by the app: some hit testing happens on mouse
  down, and something (most often the frame view) calls `[self.window
  performWindowDragWithEvent:theEvent]` to start server-side dragging.

- The fast path involves the window submitting a map of draggable regions to
  the window server. A `mouseDown:` in those regions starts a drag without
  waiting for the app.

AppKit uses both: it traverses the view hierarchy and reads the
`mouseDownCanMoveWindow` property of each view (which defaults to NO if the
view `isOpaque` and YES otherwise), and submits a map based on the views'
frames. However, a view may still override `-hitTest:` to punt some events
(say, if it's not rectangular, like a Chrome tab). If the title bar gets the
event, it starts a drag on the slow path. This means that, if an app is hung,
dragging still works, you just can't start drags from the transparent parts of
non-rectangular views.

One more complication: views that underlap the title bar in a window with a
full size content view, even if the title bar is hidden, only cut a hole in the
drag map if they override `-mouseDown:` *and* return YES for
`acceptsFirstResponder`. That was an issue because `BridgedContentView` returns
YES only if it's already first responder, so dragging was unpredictable.

This change:

- …makes BridgedContentView return nil when `-hitTest:` is passed a spot that
  should be draggable. The event ends up at the frame view, which could result
  in a drag or in a double-click miniaturizing or zooming the window.

- …deals with the "one more complication" by overriding `-_copyDragRegion` in
  BrowserWindowFrame to return nil, which effectively opts the window out of
  the fast path.

- …adds a custom frame view class for frameless app windows which uses
  `-[NSWindow performWindowDragWithEvent:]` (and an old undocumented version on
  10.10) to start a drag on `mouseDown:`, because the
  `movableByWindowBackground` property on windows seems to only affect "fast
  path" dragging.

Bug:  823770 ,  826477 
Change-Id: I9b7d727776d45853d1160b4cec53baf678e70d93
Reviewed-on: https://chromium-review.googlesource.com/1000212
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549802}
[modify] https://crrev.com/91ee9c23182a6f91b45da5540ace7e80462045c0/chrome/browser/ui/views/apps/app_window_native_widget_mac.mm
[modify] https://crrev.com/91ee9c23182a6f91b45da5540ace7e80462045c0/chrome/browser/ui/views/apps/chrome_native_app_window_views_mac.h
[modify] https://crrev.com/91ee9c23182a6f91b45da5540ace7e80462045c0/chrome/browser/ui/views/apps/chrome_native_app_window_views_mac.mm
[modify] https://crrev.com/91ee9c23182a6f91b45da5540ace7e80462045c0/chrome/browser/ui/views/frame/browser_native_widget_window_mac.mm
[modify] https://crrev.com/91ee9c23182a6f91b45da5540ace7e80462045c0/chrome/browser/ui/views/frame/native_widget_mac_frameless_nswindow.mm
[modify] https://crrev.com/91ee9c23182a6f91b45da5540ace7e80462045c0/ui/views/cocoa/bridged_content_view.h
[modify] https://crrev.com/91ee9c23182a6f91b45da5540ace7e80462045c0/ui/views/cocoa/bridged_content_view.mm
[modify] https://crrev.com/91ee9c23182a6f91b45da5540ace7e80462045c0/ui/views/cocoa/bridged_native_widget.h
[modify] https://crrev.com/91ee9c23182a6f91b45da5540ace7e80462045c0/ui/views/cocoa/bridged_native_widget.mm
[modify] https://crrev.com/91ee9c23182a6f91b45da5540ace7e80462045c0/ui/views/cocoa/bridged_native_widget_interactive_uitest.mm
[modify] https://crrev.com/91ee9c23182a6f91b45da5540ace7e80462045c0/ui/views/cocoa/native_widget_mac_nswindow.mm

Can this be marked as fixed if nothing else is pending?

Comment 15 by sdy@chromium.org, Apr 11 2018

Status: Fixed (was: Started)
Labels: TE-Verified-M67 TE-Verified-67.0.3396.0
Able to reproduce this issue on Mac OS 10.12.6 on the build without fix 67.0.3381.0 and the issue is fixed on the latest Canary 67.0.3396.0 as per  issue 826477 .

By enabling #views-browser-windows flag, able to click on the maximize/minimize/close buttons and on clicking on the tabstripe, able to ex[and/shrink the window.
Attached is the screen cast for reference.

Hence adding TE verified labels as the fix is working as intended.

Thanks..
823770.mp4
2.6 MB View Download

Comment 17 by sdy@chromium.org, Apr 16 2018

Status: Verified (was: Fixed)
Thanks for verifying!

Sign in to add a comment