Issue metadata
Sign in to add a comment
|
Exiting AppKit fullscreen using the main menu does not update JS layout.
Reported by
khushal....@etouch.net,
Jun 14 2018
|
||||||||||||||||||||||
Issue descriptionChrome Version : 69.0.3457.2 (Official Build) Revision 9c525f32d5d7caf55004d2318ac334580d63a579-refs/branch-heads/3457@{#5} (64 bit) OS: Mac (10.12.6, 10.13.1, 10.13.5, 10.13.6) Pre-condition: Enable "Use Views browser windows instead of Cocoa." flag in chrome://flags Steps to reproduce: 1. Launch chrome, navigate to https://www.youtube.com/ and play any video. 2. Click on "Full screen" from video on Youtube. 3. Now use keyboard shortcut "Ctrl+Cmd+f" and Observe. Actual Result: Fullscreen keyboard shortcut (Ctrl+Cmd+f) doesn't work. Expected Result: Fullscreen keyboard shortcut (Ctrl+Cmd+f) should work. This is Regression issue broken in 'M-69' and providing the bisect info below: Good Build: 69.0.3452.0 (Revision: 565144) Bad Build: 69.0.3453.0 (Revision: 565531) You are probably looking for a change made after 565481 (known good), but no later than 565490 (first known bad). CHANGELOG URL: The script might not always return single CL as suspect as some perf builds might get missing due to failure. https://chromium.googlesource.com/chromium/src/+log/172be4b9edff0ef700e1b57d11452f331d9ba0bf..8f2f9568808ee117698e0a9008a0b52902d2363e Suspect: https://chromium.googlesource.com/chromium/src/+/8f2f9568808ee117698e0a9008a0b52902d2363e @erikchen: Could you please check whether this is caused with respect to your change, if not please help us in assigning it to the right owner. Note: 1. "Exit full screen" button from Youtube video player also doesn't respond after using "Ctrl+Cmd+f". 2. Issue is not seen on Windows & Linux OS. Kindly refer attached screen-cast. Thank You..!!
,
Jun 15 2018
,
Jun 18 2018
With respect to comment #1, @erikchen: Retested the issue on Mac OS using latest canary build #69.0.3463.0, and the issue is still reproducible. Please refer the attached Screencast.
,
Jun 18 2018
Reassigning back to erikchen@
,
Jun 18 2018
Thanks, I can repro now. Not sure what changed.
,
Jun 19 2018
,
Jun 19 2018
The problem is that when we exit appkit fullscreen, the web contents isn't getting the message. Another way to repro the problem without using hotkeys: 1) fullscreen a youtube video by clicking the web element 2) exit appkit fullscreen by click main menu > view > exit fullscreen. This CL (https://chromium-review.googlesource.com/c/chromium/src/+/1105067) changes the hotkey behavior in views to invoke (2), so fixing (2) will also fix the hotkey case. Over to weili@, as fixing (2) should be a part of https://bugs.chromium.org/p/chromium/issues/detail?id=851261. Note to self: In Cocoa, we use -[NSWindow willEnterFullscreen:] and -[NSWindow didEnterFullscreen:] to send signals, e.g. [BrowserWindowController windowWillExitFullScreen:] calls browser_->WindowFullscreenStateWillChange(); and [BrowserWindowController windowDidExitFullScreen:] calls browser_->WindowFullscreenStateChanged(); I'm guessing that this isn't correctly hooked up for views.
,
Jun 19 2018
,
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
,
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
,
Jun 22 2018
Update : Rechecked the above issue on Mac (10.12.6, 10.13.1, 10.13.5, 10.13.6) OS with latest Canary Chrome version #69.0.3469.2 and the issue is still reproducible. Kindly refer the attached screen cast. Thank You..!!
,
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
,
Jun 27 2018
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by erikc...@chromium.org
, Jun 14 2018