[HBD] Update Old Options Strings for HBD

chrome/browser/ui/webui/options/content_settings_handler.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/webui/options/content_settings_handler.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <utility>
@@ skipping 21 matching lines @@
 #include "chrome/browser/permissions/chooser_context_base.h"
 #include "chrome/browser/permissions/permission_uma_util.h"
 #include "chrome/browser/permissions/permission_util.h"
 #include "chrome/browser/plugins/plugins_field_trial.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/exclusive_access/exclusive_access_manager.h"
 #include "chrome/browser/ui/webui/site_settings_helper.h"
 #include "chrome/browser/usb/usb_chooser_context.h"
 #include "chrome/browser/usb/usb_chooser_context_factory.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/features.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
 #include "chrome/grit/locale_settings.h"
 #include "components/content_settings/core/browser/content_settings_details.h"
 #include "components/content_settings/core/browser/content_settings_utils.h"
 #include "components/content_settings/core/browser/host_content_settings_map.h"
@@ skipping 313 matching lines @@
 }
 
 void ContentSettingsHandler::GetLocalizedValues(
     base::DictionaryValue* localized_strings) {
   DCHECK(localized_strings);
 
   static OptionsStringResource resources[] = {
     {"allowException", IDS_EXCEPTIONS_ALLOW_BUTTON},
     {"blockException", IDS_EXCEPTIONS_BLOCK_BUTTON},
     {"sessionException", IDS_EXCEPTIONS_SESSION_ONLY_BUTTON},
-    {"detectException", IDS_EXCEPTIONS_DETECT_IMPORTANT_CONTENT_BUTTON},
     {"askException", IDS_EXCEPTIONS_ASK_BUTTON},
     {"otrExceptionsExplanation", IDS_EXCEPTIONS_OTR_LABEL},
     {"addNewExceptionInstructions", IDS_EXCEPTIONS_ADD_NEW_INSTRUCTIONS},
     {"manageExceptions", IDS_EXCEPTIONS_MANAGE},
     {"manageHandlers", IDS_HANDLERS_MANAGE},
     {"exceptionPatternHeader", IDS_EXCEPTIONS_PATTERN_HEADER},
     {"exceptionBehaviorHeader", IDS_EXCEPTIONS_ACTION_HEADER},
     {"exceptionUsbDeviceHeader", IDS_EXCEPTIONS_USB_DEVICE_HEADER},
     {"exceptionZoomHeader", IDS_EXCEPTIONS_ZOOM_HEADER},
     {"embeddedOnHost", IDS_EXCEPTIONS_GEOLOCATION_EMBEDDED_ON_HOST},
@@ skipping 19 matching lines @@
     // JavaScript filter.
     {"javascriptTabLabel", IDS_JAVASCRIPT_TAB_LABEL},
     {"javascriptHeader", IDS_JAVASCRIPT_HEADER},
     {"javascriptAllow", IDS_JS_ALLOW_RADIO},
     {"javascriptBlock", IDS_JS_DONOTALLOW_RADIO},
     // Plugins filter.
     {"pluginsTabLabel", IDS_PLUGIN_TAB_LABEL},
     {"pluginsHeader", IDS_PLUGIN_HEADER},
     {"pluginsAllow", IDS_PLUGIN_ALLOW_RADIO},
     {"pluginsBlock", IDS_PLUGIN_BLOCK_RADIO},
-    {"pluginsDetectImportantContent", IDS_PLUGIN_DETECT_RECOMMENDED_RADIO},
     {"manageIndividualPlugins", IDS_PLUGIN_MANAGE_INDIVIDUAL},
     // Pop-ups filter.
     {"popupsTabLabel", IDS_POPUP_TAB_LABEL},
     {"popupsHeader", IDS_POPUP_HEADER},
     {"popupsAllow", IDS_POPUP_ALLOW_RADIO},
     {"popupsBlock", IDS_POPUP_BLOCK_RADIO},
     // Location filter.
     {"locationTabLabel", IDS_GEOLOCATION_TAB_LABEL},
     {"locationHeader", IDS_GEOLOCATION_HEADER},
     {"locationAllow", IDS_GEOLOCATION_ALLOW_RADIO},
@@ skipping 77 matching lines @@
     {"zoomLevelsManage", IDS_ZOOMLEVELS_MANAGE_BUTTON},
     // Keygen filter.
     {"keygenTabLabel", IDS_KEYGEN_TAB_LABEL},
     {"keygenHeader", IDS_KEYGEN_HEADER},
     {"keygenAllow", IDS_KEYGEN_ALLOW_RADIO},
     {"keygenBlock", IDS_KEYGEN_DONOTALLOW_RADIO},
   };
 
   RegisterStrings(localized_strings, resources, arraysize(resources));
 
+  // TODO(tommycli): When the HTML5 By Default feature flag is on, we want to
+  // display strings that begin with "Ask...", even though the setting remains
+  // DETECT. Once this feature is finalized, then we migrate the setting to ASK.

[Dan Beam, 2016/09/01 06:15:15]

wat

[tommycli, 2016/09/01 17:59:33]

Acknowledged.

+  if (base::FeatureList::IsEnabled(features::kPreferHtmlOverPlugins)) {
+    static OptionsStringResource flash_strings[] = {
+        {"pluginsDetectImportantContent", IDS_PLUGIN_ASK_RECOMMENDED_RADIO},
+        {"detectException", IDS_EXCEPTIONS_ASK_BUTTON},
+    };
+    RegisterStrings(localized_strings, flash_strings, arraysize(flash_strings));
+  } else {
+    static OptionsStringResource flash_strings[] = {
+        {"pluginsDetectImportantContent", IDS_PLUGIN_DETECT_RECOMMENDED_RADIO},
+        {"detectException", IDS_EXCEPTIONS_DETECT_IMPORTANT_CONTENT_BUTTON},
+    };
+    RegisterStrings(localized_strings, flash_strings, arraysize(flash_strings));
+  }

[Dan Beam, 2016/09/01 06:15:15]

please tell me there's any other way to do this... that's less cray

[tommycli, 2016/09/01 17:59:33]

This is unfortunate. The story is:

ASK was originally Left Click to Play. Since we weren't ready to kill Left C2P
yet, and because DETECT is weaker than ASK, we created a new DETECT setting.
Legacy ASK users are mapped to BLOCK.

Now HBD is semantically closest to ASK, but in terms of security danger, it's
closest to DETECT, since we're exempting sites with any Site Engagement
currently.

Long story short: We aren't ready to start migrating peoples' content settings
yet, especially while it's a feature flag. Once HBD ships and it's done, we will
clean up the content settings.

[tommycli, 2016/09/01 18:04:25]

I could just also change IDS_PLUGIN_ASK_RECOMMEND_RADIO to
IDS_PLUGIN_HTML_BY_DEFAULT_RECOMMENDED_RADIO, if that would prevent confusion.

I just called it ASK because the string starts with "Ask first..." and
eventually we want to migrate the DETECT setting to ASK. (After the SEI
threshold is high. See schedule here:
https://docs.google.com/document/d/1iHhg7wxYTgBHsTzcfaZy36UJ33JmjRLro8TZR6lb0...)

[raymes, 2016/09/14 01:48:05]

I agree it's confusing but I can't really think of a better way. The only thing
I can think of that may help would be to have some functions in a util class
which abstract the indirection we're adding everywhere, e.g.:

GetPluginsDetectImportantContentString
GetDetectExceptionString

This wouldn't help in all the cases though, e.g. with some of the cases
https://codereview.chromium.org/2291323003. So I don't feel strongly about it. 

I don't know this code so well, so if you want an extra pair of eyes you may
want to add bauerb. 

[tommycli, 2016/09/14 20:55:16]

Okay sounds good, I'll add bauerb. I don't want to add any more abstractions
unless it's going to be used in multiple places. I think this is the best
(temporary) solution.

+
   PrefService* prefs = Profile::FromWebUI(web_ui())->GetPrefs();
   const base::Value* default_pref = prefs->GetDefaultPrefValue(
       content_settings::WebsiteSettingsRegistry::GetInstance()
           ->Get(CONTENT_SETTINGS_TYPE_PLUGINS)
           ->default_value_pref_name());
 
   int default_value = CONTENT_SETTING_DEFAULT;
   bool success = default_pref->GetAsInteger(&default_value);
   DCHECK(success);
   DCHECK_NE(CONTENT_SETTING_DEFAULT, default_value);
@@ skipping 981 matching lines @@
 
   // Exceptions apply only when the feature is enabled.
   PrefService* prefs = user_prefs::UserPrefs::Get(GetBrowserContext(web_ui()));
   bool enable_exceptions = prefs->GetBoolean(prefs::kEnableDRM);
   web_ui()->CallJavascriptFunctionUnsafe(
       "ContentSettings.enableProtectedContentExceptions",
       base::FundamentalValue(enable_exceptions));
 }
 
 }  // namespace options