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

Issue 2824803004: Implement BrowserWindowCocoa::IsToolbarShowing (Closed)

Created:
3 years, 8 months ago by Hzj_jie
Modified:
3 years, 8 months ago
CC:
chromium-reviews, mac-reviews_chromium.org, msw
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Implement BrowserWindowCocoa::IsToolbarShowing According to the discussion in http://crbug.com/702251, we would prefer to only disable browser keyboard shortcuts once the toolbar is hidden in fullscreen mode on Mac. So a new function IsToolbarShowing() is added to BrowserWindow interface. This change implements this function in BrowserWindowCocoa by retrieving information from BrowserWindowController. R=shrike@chromium.org CC=msw@chromium.org BUG=680809 Review-Url: https://codereview.chromium.org/2824803004 Cr-Commit-Position: refs/heads/master@{#467141} Committed: https://chromium.googlesource.com/chromium/src/+/901f389a22a01c1c2bbee6b665685a8db123e70e

Patch Set 1 #

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

Messages

Total messages: 28 (18 generated)
Hzj_jie
3 years, 8 months ago (2017-04-21 22:32:55 UTC) #13
Hzj_jie
On 2017/04/21 22:32:55, Hzj_jie wrote: Jayson, do you happen to know who could review this ...
3 years, 8 months ago (2017-04-25 01:33:57 UTC) #14
shrike
On 2017/04/25 01:33:57, Hzj_jie wrote: > On 2017/04/21 22:32:55, Hzj_jie wrote: > > Jayson, do ...
3 years, 8 months ago (2017-04-25 18:23:52 UTC) #15
Hzj_jie
Add Sarah.
3 years, 8 months ago (2017-04-25 19:22:57 UTC) #17
spqchan
lgtm
3 years, 8 months ago (2017-04-25 19:36:54 UTC) #18
Hzj_jie
I may still need an LGTM from OWNERS of cocoa.
3 years, 8 months ago (2017-04-25 21:30:10 UTC) #20
Robert Sesek
LGTM, but please fix capitalization in the CL description (isToolbarShowing).
3 years, 8 months ago (2017-04-25 21:31:46 UTC) #21
Hzj_jie
On 2017/04/25 21:31:46, Robert Sesek wrote: > LGTM, but please fix capitalization in the CL ...
3 years, 8 months ago (2017-04-25 22:11:15 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2824803004/40001
3 years, 8 months ago (2017-04-25 22:13:04 UTC) #25
commit-bot: I haz the power
3 years, 8 months ago (2017-04-25 22:34:18 UTC) #28
Message was sent while issue was closed.
Committed patchset #1 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/901f389a22a01c1c2bbee6b66568...

Powered by Google App Engine
This is Rietveld 408576698