Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(202)

Issue 576393002: Mac: Fix accidental changes to fullscreen logic from refactor. (reland) (Closed)

Created:
6 years, 3 months ago by erikchen
Modified:
6 years, 3 months ago
Reviewers:
Robert Sesek
CC:
chromium-reviews
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Mac: Fix accidental changes to fullscreen logic from refactor. (reland) This CL was reverted due to test failures. Under inspection, the test proved to already be flaky. The test has been disabled. See http://crbug.com/415422 for more details. --------------Original CL Description----------------- The menu item "Enter Presentation Mode" has different effects in 10.6 and 10.7+. In 10.6, the menu item invokes Immersive Fullscreen. In 10.7+, the menu item invokes AppKit Fullscreen. Similar logic applies to fullscreen invoked by an extension. I accidentally changed this logic during the fullscreen refactor by removing the 10.6 vs 10.7+ conditional. This CL adds back the conditional. I also updated the code to explicitly indicate the mechanism that is invoking fullscreen to prevent similar future mistakes. Original CL: https://codereview.chromium.org/575653003/ BUG=NONE TBR=rsesek@chromium.org Committed: https://crrev.com/dd63d2496e2c6de6ee1ee2f451ddd815d1db5982 Cr-Commit-Position: refs/heads/master@{#295598}

Patch Set 1 : Copy of patch set 2 from https://codereview.chromium.org/575653003/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+68 lines, -15 lines) Patch
M chrome/browser/ui/cocoa/browser_window_cocoa.mm View 2 chunks +15 lines, -3 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_controller.h View 2 chunks +33 lines, -5 lines 0 comments Download
M chrome/browser/ui/cocoa/browser_window_controller.mm View 2 chunks +20 lines, -7 lines 0 comments Download

Messages

Total messages: 6 (2 generated)
erikchen
Relanding this CL after it got reverted.
6 years, 3 months ago (2014-09-18 21:58:58 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/576393002/1
6 years, 3 months ago (2014-09-18 22:00:17 UTC) #4
commit-bot: I haz the power
Committed patchset #1 (id:1) as dc096b209d8cdd761886ab5f3c4af3550be7f0b2
6 years, 3 months ago (2014-09-18 23:30:23 UTC) #5
commit-bot: I haz the power
6 years, 3 months ago (2014-09-18 23:31:12 UTC) #6
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/dd63d2496e2c6de6ee1ee2f451ddd815d1db5982
Cr-Commit-Position: refs/heads/master@{#295598}

Powered by Google App Engine
This is Rietveld 408576698