New issue
Advanced search Search tips

Issue 651287 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 3
Type: Feature



Sign in to add a comment

Remove workaround code which prevents incorrect highlighting of window buttons

Project Member Reported by eugene...@chromium.org, Sep 29 2016

Issue description

There is a MacOS bug, where window buttons highlighting tracking area is not updated after buttons are moved. This bug was created to track that workaround, which should be removed once rdar://28535344 is fixed.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 9 2017

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

commit 1f71c6ec3d33ac53f8e2474fbfd9e6396f731008
Author: Sidney San Martín <sdy@chromium.org>
Date: Wed Aug 09 14:40:17 2017

Use a custom frame view class for browser windows.

This replaces swizzling and repositioning of views in the window frame (e.g.
moving the traffic light buttons to be centered in the tab strip). It's meant
to fail gracefully:

- The entry point is a single undocumented method (+[NSWindow
  frameViewClassForStyleMask:])
- On 10.10+, the overridden undocumented methods are all getters for simple
  values (BOOL, CGFloat, and NSPoint).
- The new code doesn't call any undocumented methods (other than super).

Window frame customization is difficult to test in an automated way, but a
manual test looks roughly like this (I skip some tests on some OSs, e.g. I
didn't put Accessibility Inspector on each test system):

  - Launch Chrome.
  - Verify that everything looks generally right.
  - Verify that the window buttons are in the same place as shipping Chrome.
  - If NSVisualEffectView is available, verify that things look right when I
    move a window around behind the browser window.
  - Verify that the window buttons respond on hover.
  - Verify that the window buttons can be clicked.
  - Close the window, open a new one, repeat the above checks.
  - Resize the window, repeat the above checks.
  - Bring the window into and out of fullscreen a couple of times and verify
    that the animations look the same as the current ones.
  - Repeat the above with "Always Show Toolbar In Full Screen" turned on/off.
  - Open an Incognito window.
  - Verify that the inactive window buttons are visible.
  - Verify that the inactive window buttons respond on hover.
  - Verify that the inactive window buttons can be clicked, but don't bring the
    window to the front on mouse down.
  - Repeat all of the above checks for the incognito window.
  - On 10.9, pay special attention to the positions of the profile picker and
    fullscreen button.
  - Navigate here: data:text/html,<script>window.onclick=()=>window.open('',
    '', 'width: 100; height: 100');</script>
  - Click the page to open a popup window. Repeat all of the above checks.
  - Turn on VoiceOver. Verify that you can navigate to and activate the window
    buttons. Turn off VoiceOver.
  - Launch Accessibility Inspector. Click the crosshair button in the toolbar
    and verify that you can use the mouse to target the window buttons.
  - On newer OSs, launch Chrome in RTL: -NSForceRightToLeftWritingDirection YES
    -AppleTextDirection YES --force-ui-direction=rtl --enable-features=MacRTL
  - On 10.12+, verify that the window buttons are on the right side of the
    window.
  - Repeat for each of macOS 10.{9..13}.


Bug:  605219 ,  651287 
Change-Id: Iee9b3b03ad677255b5df50c26742551347d7d4d8
Reviewed-on: https://chromium-review.googlesource.com/562603
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Leonard Grey <lgrey@chromium.org>
Reviewed-by: Jayson Adams <shrike@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Sidney San Martín <sdy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492972}
[modify] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/browser/ui/cocoa/browser_window_controller_private.mm
[modify] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/browser/ui/cocoa/browser_window_controller_unittest.mm
[modify] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/browser/ui/cocoa/browser_window_fullscreen_transition.mm
[modify] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/browser/ui/cocoa/chrome_browser_window.h
[delete] https://crrev.com/d5ea6ad9b7b5af521c634c6461227c619b6728a1/chrome/browser/ui/cocoa/custom_frame_view.h
[delete] https://crrev.com/d5ea6ad9b7b5af521c634c6461227c619b6728a1/chrome/browser/ui/cocoa/custom_frame_view.mm
[delete] https://crrev.com/d5ea6ad9b7b5af521c634c6461227c619b6728a1/chrome/browser/ui/cocoa/custom_frame_view_unittest.mm
[modify] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/browser/ui/cocoa/framed_browser_window.h
[modify] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/browser/ui/cocoa/framed_browser_window.mm
[modify] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/browser/ui/cocoa/framed_browser_window_unittest.mm
[delete] https://crrev.com/d5ea6ad9b7b5af521c634c6461227c619b6728a1/chrome/browser/ui/cocoa/full_size_content_window.h
[delete] https://crrev.com/d5ea6ad9b7b5af521c634c6461227c619b6728a1/chrome/browser/ui/cocoa/full_size_content_window.mm
[add] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/browser/ui/cocoa/tabbed_browser_window.h
[add] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/browser/ui/cocoa/tabbed_browser_window.mm
[add] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/browser/ui/cocoa/tabbed_browser_window_unittest.mm
[modify] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/browser/ui/cocoa/tabs/tab_window_controller.mm
[modify] https://crrev.com/1f71c6ec3d33ac53f8e2474fbfd9e6396f731008/chrome/test/BUILD.gn

Comment 2 by sdy@chromium.org, Aug 10 2017

Owner: sdy@chromium.org
Status: Fixed (was: ExternalDependency)

Sign in to add a comment