Issue 3124032: Add Full screen item to video context menu

Issue 3124032: Add Full screen item to video context menu (Closed)

Created:
10 years, 4 months ago by boliu

Modified:
8 years, 8 months ago

Reviewers:
scherkus (not reviewing)

CC:
chromium-reviews, ben+cc_chromium.org, brettw-cc_chromium.org

Visibility:
Public.

Description

Add Full screen item to video context menu Corresponding WebKit patch: https://bugs.webkit.org/show_bug.cgi?id=44205 Should be committed after the WebKit patch All code behind a command line switch BUG=16735 TEST=None

Patch Set 1 #

Total comments: 1

Patch Set 2 : Add user metric. #

Total comments: 1

Patch Set 3 : Change user metric name. #

Created: 10 years, 4 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+19 lines, -0 lines)			Patch
	M	chrome/app/chrome_dll_resource.h	View		1 chunk	+1 line, -0 lines	0 comments	Download
	M	chrome/browser/tab_contents/render_view_context_menu.cc	View	1 2	3 chunks	+18 lines, -0 lines	0 comments	Download

Messages

Total messages: 5 (0 generated)

Expand Messages | Collapse Messages

scherkus (not reviewing)

don't we also need a .grd change to provide the context menu string? http://codereview.chromium.org/3124032/diff/1/3 File ...

10 years, 4 months ago (2010-08-19 17:47:09 UTC) #2

boliu

No need for grd change because the full screen items are already there. (At least ...

10 years, 4 months ago (2010-08-19 18:31:56 UTC) #3

scherkus (not reviewing)

LGTM w/ nits http://codereview.chromium.org/3124032/diff/5001/6002 File chrome/browser/tab_contents/render_view_context_menu.cc (right): http://codereview.chromium.org/3124032/diff/5001/6002#newcode1103 chrome/browser/tab_contents/render_view_context_menu.cc:1103: UserMetricsAction("MediaContextMenu_Full_Screen"), nit: Fullscreen to match constants ...

10 years, 4 months ago (2010-08-20 18:44:23 UTC) #4

The WebKit patch needs to land before this one.

Expand Messages | Collapse Messages