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

Issue 11308085: Enable fullscreen for apps windows (Closed)

Created:
8 years, 1 month ago by sail
Modified:
8 years, 1 month ago
CC:
chromium-reviews
Visibility:
Public.

Description

Enable fullscreen for apps windows This CL allows apps windows to enter fullscreen on all platforms. This fixes a regression introduced by r167006. BUG=161246 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=168694

Patch Set 1 #

Total comments: 2

Patch Set 2 : address comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -4 lines) Patch
M chrome/browser/ui/browser.h View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/browser_command_controller.cc View 1 1 chunk +5 lines, -4 lines 0 comments Download
M chrome/browser/ui/browser_command_controller_unittest.cc View 1 2 chunks +26 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
sail
sky/rsesek please take a look. Also, do you think this is correct? Is there a ...
8 years, 1 month ago (2012-11-17 19:12:59 UTC) #1
sky
Not sure about we we limit fullscreen. Maybe Yuzhu or Glen could tell you.
8 years, 1 month ago (2012-11-19 16:22:01 UTC) #2
yzshen1
I don't think I know enough about app windows to comment on this issue. Adding ...
8 years, 1 month ago (2012-11-19 17:44:01 UTC) #3
scheib
I'm unaware of why there would be a platform difference here. I also wonder what ...
8 years, 1 month ago (2012-11-19 18:05:48 UTC) #4
sail
https://codereview.chromium.org/11308085/diff/1/chrome/browser/ui/browser_command_controller.cc File chrome/browser/ui/browser_command_controller.cc (right): https://codereview.chromium.org/11308085/diff/1/chrome/browser/ui/browser_command_controller.cc#newcode1112 chrome/browser/ui/browser_command_controller.cc:1112: #else On 2012/11/19 18:05:49, scheib wrote: > Document why ...
8 years, 1 month ago (2012-11-19 18:07:30 UTC) #5
scheib
Before this change, is there a platform difference? Aren't you adding the difference via this ...
8 years, 1 month ago (2012-11-19 18:12:18 UTC) #6
sail
On 2012/11/19 18:12:18, scheib wrote: > Before this change, is there a platform difference? Aren't ...
8 years, 1 month ago (2012-11-19 18:18:58 UTC) #7
scheib
As discussed directly: looks like the fullscreen limitation should only apply to app-panels. Just apps ...
8 years, 1 month ago (2012-11-19 21:27:59 UTC) #8
scheib
On 2012/11/19 21:27:59, scheib wrote: > As discussed directly: looks like the fullscreen limitation should ...
8 years, 1 month ago (2012-11-19 21:28:30 UTC) #9
sail
Updated, please take another look.
8 years, 1 month ago (2012-11-19 22:58:38 UTC) #10
scheib
lgtm
8 years, 1 month ago (2012-11-19 23:02:14 UTC) #11
sky
LGTM
8 years, 1 month ago (2012-11-19 23:11:43 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sail@chromium.org/11308085/10002
8 years, 1 month ago (2012-11-19 23:18:23 UTC) #13
commit-bot: I haz the power
Retried try job too often for step(s) interactive_ui_tests
8 years, 1 month ago (2012-11-20 00:20:16 UTC) #14
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/sail@chromium.org/11308085/10002
8 years, 1 month ago (2012-11-20 01:15:56 UTC) #15
commit-bot: I haz the power
8 years, 1 month ago (2012-11-20 01:56:57 UTC) #16
Change committed as 168694

Powered by Google App Engine
This is Rietveld 408576698