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

Unified Diff: chrome/browser/ui/cocoa/website_settings_bubble_controller.mm

Issue 11571010: fixed the DCHECK and also corrected the website setting UI for media (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: removed the DCHECK part code, and added allow support to media for https Created 8 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/cocoa/website_settings_bubble_controller.mm
diff --git a/chrome/browser/ui/cocoa/website_settings_bubble_controller.mm b/chrome/browser/ui/cocoa/website_settings_bubble_controller.mm
index ee9d575a15fca276cbf899a9d43178925d621977..663c2790307c4fa9f8820779571e02880836072d 100644
--- a/chrome/browser/ui/cocoa/website_settings_bubble_controller.mm
+++ b/chrome/browser/ui/cocoa/website_settings_bubble_controller.mm
@@ -872,9 +872,10 @@ NSColor* IdentityVerifiedTextColor() {
// Create the popup menu.
// TODO(dubroy): Refactor this code to use PermissionMenuModel.
- // Media stream permission does not support "Always allow".
- // TODO(xians): Media supports "allow" for https but not for http.
- if (permissionInfo.type != CONTENT_SETTINGS_TYPE_MEDIASTREAM) {
+ bool secure_scheme = webContents_->GetURL().SchemeIsSecure();
markusheintz_ 2012/12/14 15:46:43 nit: I think we could inline this.
no longer working on chromium 2012/12/14 15:56:19 this secure_scheme is used in two places, and it i
markusheintz_ 2012/12/14 16:12:09 If we keep using it in two places then ignore this
+ // Media stream permission only support "Always allow" for https.
+ if (permissionInfo.type != CONTENT_SETTINGS_TYPE_MEDIASTREAM ||
+ secure_scheme) {
[button addItemWithTitle:
l10n_util::GetNSString(IDS_WEBSITE_SETTINGS_MENU_ITEM_ALLOW)];
[[button lastItem] setTag:CONTENT_SETTING_ALLOW];
@@ -893,7 +894,15 @@ NSColor* IdentityVerifiedTextColor() {
permissionInfo.default_setting))];
[[button lastItem] setTag:CONTENT_SETTING_DEFAULT];
- [button selectItemWithTag:permissionInfo.setting];
+ if (permissionInfo.type == CONTENT_SETTINGS_TYPE_MEDIASTREAM &&
markusheintz_ 2012/12/14 15:46:43 Can this condition ever be true while the conditi
no longer working on chromium 2012/12/14 15:56:19 line 877 decides if it will add a "allow" button t
markusheintz_ 2012/12/14 16:12:09 But the permission should never be ALLOW for an ht
+ !secure_scheme &&
+ permissionInfo.setting == CONTENT_SETTING_ALLOW) {
+ // Select the default setting as the current setting since MEDIASTREAM
+ // does not support allow for http.
+ [button selectItemWithTag:CONTENT_SETTING_DEFAULT];
+ } else {
+ [button selectItemWithTag:permissionInfo.setting];
+ }
// Set the button title.
scoped_nsobject<NSMenuItem> titleItem([[NSMenuItem alloc] init]);

Powered by Google App Engine
This is Rietveld 408576698