New issue
Advanced search Search tips

Issue 839530 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

[Fullscreen Control] Integrate FullscreenControlHost with Mac

Project Member Reported by yuweih@chromium.org, May 3 2018

Issue description

We don't need the fullscreen exit UI on Mac for mouse and touch scenarios, but we do need it for system keyboard lock. This bug tracks works to get FullscreenControlHost integrated with Mac.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 7 2018

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

commit d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6
Author: Yuwei Huang <yuweih@chromium.org>
Date: Mon May 07 20:46:11 2018

[Fullscreen Control] Integrate FullscreenControlHost with Mac Cocoa toolkit

The FullscreenControlHost is an "X" button dropped down from the top of the
screen for exiting browser fullscreen (shortn/_U0aj1caGIS).

This CL integrates the FullscreenControlHost with the Cocoa toolkit. On Mac
it is not used for the mouse or touch triggered scenario that
FullscreenControlHost::IsExitUiNeeded() indicates, instead it is only used
as a visual feedback for press-and-hold ESC required by keyboard lock.

Bug:  839530 
Change-Id: Ib6dd7e8fdea009351065ccb675c343db1b04335e
Reviewed-on: https://chromium-review.googlesource.com/1043352
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556562}
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/browser_command_controller_unittest.cc
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/cocoa/browser/exclusive_access_controller_views.h
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/cocoa/browser/exclusive_access_controller_views.mm
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/exclusive_access/exclusive_access_context.h
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/exclusive_access/fullscreen_controller_state_unittest.cc
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.h
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/views/frame/browser_view.h
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/views/fullscreen_control/fullscreen_control_host.cc
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/views/fullscreen_control/fullscreen_control_host.h
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/views/media_router/presentation_receiver_window_view.cc
[modify] https://crrev.com/d0f610cfbd1a1d4257ac4464654ebfeedf7e59a6/chrome/browser/ui/views/media_router/presentation_receiver_window_view.h

Project Member

Comment 2 by bugdroid1@chromium.org, May 9 2018

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

commit 9d0fab63c914bdb0536ef0b7b4d7b16e9ecb2c63
Author: Yuwei Huang <yuweih@chromium.org>
Date: Wed May 09 23:23:14 2018

[Fullscreen Control] Integrate with MacViews

This CL integrates the FullscreenControlHost with the MacViews toolkit
so that keyboard lock can use it as the visual feedback for
press-and-hold ESC to exit fullscreen.

Bug:  839530 
Change-Id: I0d8792114840b039ce72ec4a755bdf1fc887d723
Reviewed-on: https://chromium-review.googlesource.com/1050930
Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557373}
[modify] https://crrev.com/9d0fab63c914bdb0536ef0b7b4d7b16e9ecb2c63/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/9d0fab63c914bdb0536ef0b7b4d7b16e9ecb2c63/chrome/browser/ui/views/frame/browser_view.h
[modify] https://crrev.com/9d0fab63c914bdb0536ef0b7b4d7b16e9ecb2c63/chrome/browser/ui/views/fullscreen_control/fullscreen_control_host.cc

Comment 3 by yuweih@chromium.org, May 25 2018

Status: Fixed (was: Assigned)
Labels: Needs-Feedback
@Yuwei Huang: As we are not very clear about the expected behaviour after the fix and the earlier behaviour, It would be highly helpful if elaborated on it. Any other inputs from your end may be very helpful in verifying the fix.

Thanks! 
Basically before the integration is done, the drop down "X" animation will not show up on Mac when:

1. The web page requests keyboard lock on the ESC key
2. The user enters fullscreen
3. The user presses and holds the ESC key

This is working now after the fix.

Sign in to add a comment