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

Issue 747001: Mac: fix fullscreen floating bar in popup windows. (Closed)

Created:
10 years, 9 months ago by viettrungluu
Modified:
9 years, 6 months ago
CC:
chromium-reviews_googlegroups.com, John Grabowski, pam+watch_chromium.org, ben+cc_chromium.org
Visibility:
Public.

Description

Mac: fix fullscreen floating bar in popup windows. BUG=37699 TEST=Make sure the following types of windows look correct in fullscreen mode (in particular, the toolbar/menu should slide down properly and look nice): (1) normal windows (with tab strip), (2) popup windows with location bar, (3) "app" windows such as the dev tools window. Also make sure these things show correctly with themes and the bookmark bar enabled. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=41073

Patch Set 1 #

Total comments: 4

Patch Set 2 : changes per review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -40 lines) Patch
M chrome/browser/cocoa/browser_frame_view.mm View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/cocoa/browser_window_controller.h View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/cocoa/browser_window_controller.mm View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/cocoa/browser_window_controller_private.mm View 1 7 chunks +34 lines, -28 lines 0 comments Download
M chrome/browser/cocoa/fullscreen_controller.h View 1 chunk +5 lines, -4 lines 0 comments Download
M chrome/browser/cocoa/fullscreen_controller.mm View 2 chunks +5 lines, -4 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
viettrungluu
10 years, 9 months ago (2010-03-09 20:04:35 UTC) #1
rohitrao (ping after 24h)
LGTM, with a couple nits. http://codereview.chromium.org/747001/diff/1/5 File chrome/browser/cocoa/browser_window_controller_private.mm (right): http://codereview.chromium.org/747001/diff/1/5#newcode216 chrome/browser/cocoa/browser_window_controller_private.mm:216: totalHeight += NSHeight([[self tabStripView] ...
10 years, 9 months ago (2010-03-09 20:22:47 UTC) #2
viettrungluu
10 years, 9 months ago (2010-03-09 21:04:53 UTC) #3
Thanks.

http://codereview.chromium.org/747001/diff/1/5
File chrome/browser/cocoa/browser_window_controller_private.mm (right):

http://codereview.chromium.org/747001/diff/1/5#newcode216
chrome/browser/cocoa/browser_window_controller_private.mm:216: totalHeight += 
NSHeight([[self tabStripView] frame]);
On 2010/03/09 20:22:47, rohitrao wrote:
> Extra space?

Fixed. (It's my job to complain about such trivialities!)

http://codereview.chromium.org/747001/diff/1/5#newcode350
chrome/browser/cocoa/browser_window_controller_private.mm:350: // But we want
the logic to work as usual.
On 2010/03/09 20:22:47, rohitrao wrote:
> What kind of windows don't have overlays?  Why do we still need to call
> overlayFrameChanged for these?  Expand the comment a bit, please.

Done.

Powered by Google App Engine
This is Rietveld 408576698