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

Issue 2718623004: Added shorcut:"F11" for exit fullscreen context menu. (Closed)

Created:
3 years, 9 months ago by kumar.vikram
Modified:
3 years, 3 months ago
CC:
chromium-reviews, tfarina
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Added shorcut:"F11" for exit fullscreen context menu. We should display F11 shorcut for the exit fullscreen context menu, when the full screen is not site-triggered. BUG=674108 Signed-off-by: kumar.vikram <kumar.vikram@samsung.com>;

Patch Set 1 #

Total comments: 1

Patch Set 2 : added more discription in the commit message #

Patch Set 3 : resolved nit #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -0 lines) Patch
M AUTHORS View 1 chunk +1 line, -0 lines 0 comments Download
M chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc View 1 2 1 chunk +3 lines, -0 lines 1 comment Download

Messages

Total messages: 15 (9 generated)
Srirama
https://codereview.chromium.org/2718623004/diff/1/chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc File chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc (right): https://codereview.chromium.org/2718623004/diff/1/chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc#newcode141 chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc:141: else { nit: move else to previous line. Other ...
3 years, 9 months ago (2017-02-27 06:25:16 UTC) #6
kumar.vikram
On 2017/02/27 06:25:16, Srirama wrote: > https://codereview.chromium.org/2718623004/diff/1/chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc > File > chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc > (right): > > ...
3 years, 9 months ago (2017-02-27 06:53:45 UTC) #7
dsinclair
3 years, 9 months ago (2017-02-27 14:16:42 UTC) #11
Lei Zhang
I'm not 100% sure this is the right place to add the new code. I ...
3 years, 9 months ago (2017-02-27 19:43:35 UTC) #12
Lei Zhang
https://codereview.chromium.org/2718623004/diff/40001/chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc File chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc (right): https://codereview.chromium.org/2718623004/diff/40001/chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc#newcode141 chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc:141: *accel = ui::Accelerator(ui::VKEY_F11, ui::EF_NONE); I don't know what the ...
3 years, 9 months ago (2017-02-28 04:17:35 UTC) #13
Lei Zhang
3 years, 9 months ago (2017-02-28 05:06:26 UTC) #14
On 2017/02/28 04:17:35, Lei Zhang (super slow) wrote:
>
https://codereview.chromium.org/2718623004/diff/40001/chrome/browser/ui/views...
> File
>
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc
> (right):
> 
>
https://codereview.chromium.org/2718623004/diff/40001/chrome/browser/ui/views...
>
chrome/browser/ui/views/renderer_context_menu/render_view_context_menu_views.cc:141:
> *accel = ui::Accelerator(ui::VKEY_F11, ui::EF_NONE);
> I don't know what the right answer is, but this isn't it. See the ChromeOS
code
> right below here. We don't want to show F11 on ChromeOS.

And so I looked and came up with https://codereview.chromium.org/2720963002/

Powered by Google App Engine
This is Rietveld 408576698