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

Issue 9187063: [Mac] Fix Cmd-W closing whole window under fullscreen. (Closed)

Created:
8 years, 11 months ago by Alexei Svitkine (slow)
Modified:
8 years, 11 months ago
Reviewers:
Robert Sesek
CC:
chromium-reviews
Visibility:
Public.

Description

Fix Cmd-W closing whole window under fullscreen. This got broken by http://crrev.com/116144, which made Cmd-W (close tab) route to -performClose:. However, FullscreenWindow had an override for -performClose:, resulting in the logic to route Cmd-W to -commandDispatch: not taking place for fullscreen windows. Also fixes -[BWC validateUserInterfaceItem:] to correctly disable IDC_CLOSE_TAB menu items, which was also broken in the same revision. BUG=109793 TEST=1. Open a fullscreen window with multiple tabs. Hit Cmd-W. A single tab should close. 2. Open a window that doesn't have tabs - e.g. Window -> Task Manager. File -> Close Tab should be disabled. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=117681

Patch Set 1 #

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -5 lines) Patch
M chrome/browser/ui/cocoa/browser_window_controller.mm View 1 2 chunks +4 lines, -3 lines 0 comments Download
M chrome/browser/ui/cocoa/fullscreen_window.mm View 2 chunks +10 lines, -2 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
Alexei Svitkine (slow)
8 years, 11 months ago (2012-01-12 21:16:00 UTC) #1
Alexei Svitkine (slow)
ping
8 years, 11 months ago (2012-01-13 18:16:29 UTC) #2
Robert Sesek
lgtm. Sorry for the delay.
8 years, 11 months ago (2012-01-13 18:46:38 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/asvitkine@chromium.org/9187063/5001
8 years, 11 months ago (2012-01-13 18:47:18 UTC) #4
commit-bot: I haz the power
8 years, 11 months ago (2012-01-13 20:30:49 UTC) #5
Change committed as 117681

Powered by Google App Engine
This is Rietveld 408576698