New issue
Advanced search Search tips

Issue 851261 link

Starred by 7 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug-Regression

Blocking:
issue 831219



Sign in to add a comment

Toolbar is visible in content fullscreen

Project Member Reported by sdy@chromium.org, Jun 9 2018

Issue description

Chrome Version: 69.0.3453.3
OS: 10.13.5

What steps will reproduce the problem?
(1) Make sure that "Always show toolbar in Full Screen" is checked in the View menu.
(1) Visit a page which can go fullscreen (e.g. a YouTube video).
(2) Use the page controls to make the video fullscreen.

What is the expected result?
The toolbar is not visible.

What happens instead?
The toolbar is visible.
 
Cc: jmukthavaram@chromium.org nyerramilli@chromium.org gov...@chromium.org ligim...@chromium.org
Labels: -ReleaseBlock-Beta -Type-Bug ReleaseBlock-Dev RegressedIn-69 Type-Bug-Regression
Labels: -ReleaseBlock-Dev ReleaseBlock-Beta
Not an RBD at this point.
Friendly ping to get an update on this issue.
Thanks..!

Comment 4 by weili@chromium.org, Jun 13 2018

Status: Started (was: Assigned)

Comment 5 by gov...@chromium.org, Jun 18 2018

M69 branch is coming soon on July 19th, Your bug is marked as ReleaseBlock-Beta for M69. Please try to land the fix ASAP to trunk in order to prevent many merges going after M69 branch. This will also help us to branch M69 from high quality trunk. Thank you.


Project Member

Comment 6 by bugdroid1@chromium.org, Jun 21 2018

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

commit 9bb543155ad47d11896d32cbb71cdce3d5b8d8f1
Author: Wei Li <weili@chromium.org>
Date: Thu Jun 21 16:48:20 2018

[MacViews] Show/hide top UI in fullscreen mode correctly

Unlike Windows and Linux, Mac allows top UI to show or hide in browser
fullscreen mode. Also this option can be toggled at runtime to make the
change. But in tab fullscreen mode, top UI should always hide. These
require propagating fullscreen and user preference changes, recording
the toolbar styles and applying the layout accordingly.

This CL makes sure the top chrome UI status is correct across different
fullscreen modes, and responds to user changes as well. Some tests are
also added.

BUG= 851261 ,831219, 852779 
TEST=Pls check the toolbar/tabstrip show or hide correctly when
entering and exiting various fullscreen modes. Bookmark bar, slide-down
menu and traffic lights will be fixed later.

Change-Id: I78a2b68821d5e633ac5c2823f65390809a57dad5
Reviewed-on: https://chromium-review.googlesource.com/1102026
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569301}
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/chrome/browser/ui/cocoa/browser_window_controller.mm
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/chrome/browser/ui/exclusive_access/fullscreen_controller.cc
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/chrome/browser/ui/views/frame/browser_frame_mac.h
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/chrome/browser/ui/views/frame/browser_frame_mac.mm
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/chrome/browser/ui/views/frame/browser_non_client_frame_view.h
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.mm
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/chrome/browser/ui/views/frame/browser_view.h
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/ui/views/cocoa/bridged_native_widget.mm
[modify] https://crrev.com/9bb543155ad47d11896d32cbb71cdce3d5b8d8f1/ui/views/widget/native_widget_mac.h

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 21 2018

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

commit c83950fc6c2d04db185355e2fff6fb981e886303
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Thu Jun 21 21:34:30 2018

Revert "[MacViews] Show/hide top UI in fullscreen mode correctly"

This reverts commit 9bb543155ad47d11896d32cbb71cdce3d5b8d8f1.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 569301 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzliYjU0MzE1NWFkNDdkMTE4OTZkMzJjYmI3MWNkY2UzZDViOGQ4ZjEM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/Mac%20ASan%2064%20Tests%20%281%29/41389

Sample Failed Step: interactive_ui_tests

Original change's description:
> [MacViews] Show/hide top UI in fullscreen mode correctly
> 
> Unlike Windows and Linux, Mac allows top UI to show or hide in browser
> fullscreen mode. Also this option can be toggled at runtime to make the
> change. But in tab fullscreen mode, top UI should always hide. These
> require propagating fullscreen and user preference changes, recording
> the toolbar styles and applying the layout accordingly.
> 
> This CL makes sure the top chrome UI status is correct across different
> fullscreen modes, and responds to user changes as well. Some tests are
> also added.
> 
> BUG= 851261 ,831219, 852779 
> TEST=Pls check the toolbar/tabstrip show or hide correctly when
> entering and exiting various fullscreen modes. Bookmark bar, slide-down
> menu and traffic lights will be fixed later.
> 
> Change-Id: I78a2b68821d5e633ac5c2823f65390809a57dad5
> Reviewed-on: https://chromium-review.googlesource.com/1102026
> Commit-Queue: Wei Li <weili@chromium.org>
> Reviewed-by: Trent Apted <tapted@chromium.org>
> Reviewed-by: Sarah Chan <spqchan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#569301}

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
BUG= 851261 ,831219, 852779 

Change-Id: I2447ccf0b3a960a4e017dcf957d044bff8ae2433
Reviewed-on: https://chromium-review.googlesource.com/1110898
Cr-Commit-Position: refs/heads/master@{#569379}
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/chrome/browser/ui/cocoa/browser_window_controller.mm
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/chrome/browser/ui/exclusive_access/fullscreen_controller.cc
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/chrome/browser/ui/views/frame/browser_frame_mac.h
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/chrome/browser/ui/views/frame/browser_frame_mac.mm
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/chrome/browser/ui/views/frame/browser_non_client_frame_view.h
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.mm
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/chrome/browser/ui/views/frame/browser_view.h
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/ui/views/cocoa/bridged_native_widget.mm
[modify] https://crrev.com/c83950fc6c2d04db185355e2fff6fb981e886303/ui/views/widget/native_widget_mac.h

Labels: MacViews-Release
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 22 2018

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

commit 0eadb78c0a33c57f92d339be5139c0c0d7bc53ab
Author: Wei Li <weili@chromium.org>
Date: Fri Jun 22 23:27:07 2018

Reland "[MacViews] Show/hide top UI in fullscreen mode correctly"

The original CL https://crrev.com/c/1102026 got reverted due to failed
test BrowserViewTest.BrowserFullscreenShowTopView on
"Mac ASan 64 Tests (1)" bot. The failure is most likely due to
that the system preference for "always show toolbar in full screen"
may be different on different bots. This CL explicitly set up the
preference at the beginning of the test.

Original change's description:
> [MacViews] Show/hide top UI in fullscreen mode correctly
>
> Unlike Windows and Linux, Mac allows top UI to show or hide in browser
> fullscreen mode. Also this option can be toggled at runtime to make the
> change. But in tab fullscreen mode, top UI should always hide. These
> require propagating fullscreen and user preference changes, recording
> the toolbar styles and applying the layout accordingly.
>
> This CL makes sure the top chrome UI status is correct across different
> fullscreen modes, and responds to user changes as well. Some tests are
> also added.
>
> BUG= 851261 ,831219, 852779 
> TEST=Pls check the toolbar/tabstrip show or hide correctly when
> entering and exiting various fullscreen modes. Bookmark bar, slide-down
> menu and traffic lights will be fixed later.
>
> Change-Id: I78a2b68821d5e633ac5c2823f65390809a57dad5
> Reviewed-on: https://chromium-review.googlesource.com/1102026
> Commit-Queue: Wei Li <weili@chromium.org>
> Reviewed-by: Trent Apted <tapted@chromium.org>
> Reviewed-by: Sarah Chan <spqchan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#569301}

Change-Id: I37a2641b5a578dabcf922792158c0f13b2e197e8
Reviewed-on: https://chromium-review.googlesource.com/1111015
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Sarah Chan <spqchan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569836}
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/chrome/browser/ui/cocoa/browser_window_controller.mm
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/chrome/browser/ui/exclusive_access/fullscreen_controller.cc
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/chrome/browser/ui/views/frame/browser_frame_mac.h
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/chrome/browser/ui/views/frame/browser_frame_mac.mm
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/chrome/browser/ui/views/frame/browser_non_client_frame_view.cc
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/chrome/browser/ui/views/frame/browser_non_client_frame_view.h
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.h
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/chrome/browser/ui/views/frame/browser_non_client_frame_view_mac.mm
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/chrome/browser/ui/views/frame/browser_view.cc
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/chrome/browser/ui/views/frame/browser_view.h
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/chrome/browser/ui/views/frame/browser_view_interactive_uitest.cc
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/ui/views/cocoa/bridged_native_widget.mm
[modify] https://crrev.com/0eadb78c0a33c57f92d339be5139c0c0d7bc53ab/ui/views/widget/native_widget_mac.h

Issue 855819 has been merged into this issue.

Comment 11 by weili@chromium.org, Jun 27 2018

Status: Fixed (was: Started)
Labels: TE-Verified-69.0.3493.0 TE-Verified-M69
Able to reproduce the issue on Mac 10.13.3 using chrome reported version #69.0.3453.3

Verified the fix on Mac 10.13.3 using latest chrome version #69.0.3493.0 as per the merged  issue #852779 .
Attaching screen cast for reference.
Observed that Fullscreen keyboard shortcut (Ctrl+Cmd+f) worked as expected and toolbar/tabstrip showed and hide correctly when entering and exiting various fullscreen modes.
Hence, the fix is working as expected. 
Adding the verified labels.

Thanks...!!
851261.mp4
4.7 MB View Download
MacOS 10.14 Beta
Chrome Version 69.0.3493.0
All flags of Views enabled
Always Show Toolbar in Fullscreen is checked
Browser is Fullscreen

Verified: Going full screen on a video hides the toolbar as expected until exiting the video fullscreen mode.


Not sure if related but when 'Always Show Toolbar in Fullscreen' is unchecked, the toolbar still fails to appear when moving the cursor to the top of the screen. Command+L does not spawn the toolbar either. This remains unsolved.

Sign in to add a comment