Add a section for the fullscreen JS API in the content settings page.

Add a section for the fullscreen JS API in the content settings page.


BUG=100292
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110654

[Evan Stade, 2011-11-17 23:07:37]

I don't see why you permit Ask, shouldn't it be allow or nothing?

http://codereview.chromium.org/8585047/diff/1/chrome/browser/content_settings...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8585047/diff/1/chrome/browser/content_settings...
chrome/browser/content_settings/host_content_settings_map.cc:329: return
(setting == CONTENT_SETTING_ASK &&
as long as you are removing random parens, can you get rid of these too?

http://codereview.chromium.org/8585047/diff/1/chrome/browser/content_settings...
chrome/browser/content_settings/host_content_settings_map.cc:336: case
CONTENT_SETTINGS_TYPE_FULLSCREEN:
why is this separate from the above cases?

[koz (OOO until 15th September), 2011-11-17 23:37:53]

Yep, good point. I've removed the ask option.

http://codereview.chromium.org/8585047/diff/1/chrome/browser/content_settings...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8585047/diff/1/chrome/browser/content_settings...
chrome/browser/content_settings/host_content_settings_map.cc:329: return
(setting == CONTENT_SETTING_ASK &&
On 2011/11/17 23:07:37, Evan Stade wrote:
> as long as you are removing random parens, can you get rid of these too?

Done.

http://codereview.chromium.org/8585047/diff/1/chrome/browser/content_settings...
chrome/browser/content_settings/host_content_settings_map.cc:336: case
CONTENT_SETTINGS_TYPE_FULLSCREEN:
On 2011/11/17 23:07:37, Evan Stade wrote:
> why is this separate from the above cases?

Done.

[Evan Stade, 2011-11-17 23:41:11]

http://codereview.chromium.org/8585047/diff/3001/chrome/browser/resources/opt...
File chrome/browser/resources/options/content_settings_exceptions_area.js
(right):

http://codereview.chromium.org/8585047/diff/3001/chrome/browser/resources/opt...
chrome/browser/resources/options/content_settings_exceptions_area.js:114: //
Editing notifications and geolocation is disabled for now.
also disable editing imo

[koz (OOO until 15th September), 2011-11-17 23:53:42]

http://codereview.chromium.org/8585047/diff/3001/chrome/browser/resources/opt...
File chrome/browser/resources/options/content_settings_exceptions_area.js
(right):

http://codereview.chromium.org/8585047/diff/3001/chrome/browser/resources/opt...
chrome/browser/resources/options/content_settings_exceptions_area.js:114: //
Editing notifications and geolocation is disabled for now.
On 2011/11/17 23:41:12, Evan Stade wrote:
> also disable editing imo

Cool, done. I didn't know about that.

[Evan Stade, 2011-11-17 23:56:52]

lgtm

[jochen (gone - plz use gerrit), 2011-11-18 00:00:53]

http://codereview.chromium.org/8585047/diff/7001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8585047/diff/7001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:336: return setting
== CONTENT_SETTING_ASK;
hum, didn't you say you've removed ask?

[koz (OOO until 15th September), 2011-11-18 00:07:59]

http://codereview.chromium.org/8585047/diff/7001/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8585047/diff/7001/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:336: return setting
== CONTENT_SETTING_ASK;
On 2011/11/18 00:00:53, jochen wrote:
> hum, didn't you say you've removed ask?

Ah, only from the UI. Done.

[Evan Stade, 2011-11-18 00:13:58]

http://codereview.chromium.org/8585047/diff/5002/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/content_settings_handler.cc (right):

http://codereview.chromium.org/8585047/diff/5002/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/content_settings_handler.cc:592: // Fullscreen
doesn't have enable / disable functionality.
what is enable disable functionality?

[jochen (gone - plz use gerrit), 2011-11-18 00:14:28]

LGTM with nit

http://codereview.chromium.org/8585047/diff/5002/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8585047/diff/5002/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:313: // BLOCK
semantics are not implemented for fullscreen.
can you file a bug and add a TODO entry with a reference to that bug here?

[koz (OOO until 15th September), 2011-11-18 00:24:50]

Thanks for the quick reviews, guys.

http://codereview.chromium.org/8585047/diff/5002/chrome/browser/content_setti...
File chrome/browser/content_settings/host_content_settings_map.cc (right):

http://codereview.chromium.org/8585047/diff/5002/chrome/browser/content_setti...
chrome/browser/content_settings/host_content_settings_map.cc:313: // BLOCK
semantics are not implemented for fullscreen.
On 2011/11/18 00:14:28, jochen wrote:
> can you file a bug and add a TODO entry with a reference to that bug here?

Done.

[commit-bot: I haz the power, 2011-11-18 03:11:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/koz@chromium.org/8585047/11003

[commit-bot: I haz the power, 2011-11-18 04:13:58]

Change committed as 110654