Remove the managed banner from the Chrome settings UI

Remove the managed banner from the Chrome settings UI

This banner has been obsoleted by the controlled setting indicators.

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

[bartfab (slow), 2012-10-25 09:10:43]

Hi everyone, I would appreciate your review as I try to finish this feature for
M24.

James, this CL makes changes to the settings UI.
Nikita, this CL makes some changes to the Chrome OS specific settings UI.
Joao, this CL removes all tests pertaining to the managed banner.
Matt, I would like to keep the uber_shared.css you added to an example extension
in sync with the main one (only comment changes here).

[Nikita (slow), 2012-10-25 09:28:54]

lgtm

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/resourc...
File chrome/browser/resources/options/chromeos/internet_detail.html (right):

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/resourc...
chrome/browser/resources/options/chromeos/internet_detail.html:522: <span
id="banner-text" class="page-banner-text"></span>
Warning text has to stay?

[bartfab (slow), 2012-10-25 09:31:44]

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/resourc...
File chrome/browser/resources/options/chromeos/internet_detail.html (right):

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/resourc...
chrome/browser/resources/options/chromeos/internet_detail.html:522: <span
id="banner-text" class="page-banner-text"></span>
Yes, this is a custom banner used only here that is staying for now. I mentioned
this in the bug: The proxy dialog is due for an overhaul soon anyway and the
banner will be cleaned up as part of that.

The banner-icon div was actually not being used by anyone for anything, so I
removed it as general clean-up.

[Joao da Silva, 2012-10-25 11:07:52]

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/proxy_cros_settings_parser.cc (right):

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/proxy_cros_settings_parser.cc:327: case
ProxyPrefs::CONFIG_POLICY:
The indentation of the case: labels should be +2 from the switch statement

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/policy/...
File chrome/browser/policy/policy_prefs_browsertest.cc (right):

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/policy/...
chrome/browser/policy/policy_prefs_browsertest.cc:394: TEST(PolicyPrefsTest,
AllPoliciesHaveATestCase) {
TEST() expand the arguments into a class name, and this one could clash with
class PolicyPrefsTest above. Please rename one of them.

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/resourc...
File chrome/browser/resources/options/options_page.js (left):

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/resourc...
chrome/browser/resources/options/options_page.js:756: var text =
bannerDiv.querySelector('#managed-prefs-text');
An element with id 'managed-prefs-text' exists in
chrome/browser/resources/extensions/extensions.html. Can this code really be
removed?

That element seems related to managed mode, maybe Bernhard knows more about
this.

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/test/data/polic...
File chrome/test/data/policy/policy_test_cases.json (left):

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/test/data/polic...
chrome/test/data/policy/policy_test_cases.json:323: "note": "TODO(joaodasilva):
Should be chrome://settings-frame/passwords. http://crbug.com/97749"
Please close this bug after landing this CL

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/test/data/polic...
chrome/test/data/policy/policy_test_cases.json:392: "settings_pages": []
Please leave a TODO here. This policy works, but needs to be set before the
browser is launched. We can refactor PolicyPrefsTest later to do this for some
of these tests.

[falken, 2012-10-25 13:01:55]

chrome/common/extensions/docs/examples/api/fontSettings/ lgtm

[bartfab (slow), 2012-10-25 13:13:12]

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/proxy_cros_settings_parser.cc (right):

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/proxy_cros_settings_parser.cc:327: case
ProxyPrefs::CONFIG_POLICY:
On 2012/10/25 11:07:52, Joao da Silva wrote:
> The indentation of the case: labels should be +2 from the switch statement

Done.

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/policy/...
File chrome/browser/policy/policy_prefs_browsertest.cc (right):

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/policy/...
chrome/browser/policy/policy_prefs_browsertest.cc:394: TEST(PolicyPrefsTest,
AllPoliciesHaveATestCase) {
On 2012/10/25 11:07:52, Joao da Silva wrote:
> TEST() expand the arguments into a class name, and this one could clash with
> class PolicyPrefsTest above. Please rename one of them.

Done.

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/resourc...
File chrome/browser/resources/options/options_page.js (left):

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/browser/resourc...
chrome/browser/resources/options/options_page.js:756: var text =
bannerDiv.querySelector('#managed-prefs-text');
The managed mode banner actually has a completely independent implementation and
only shared CSS with the options banners. I left the CSS intact, so that banner
is still working (I verified it just in case).

The managed mode banner did have some stale ids/classes that nothing in CSS, JS
or C++ ever referenced. I removed those now.

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/test/data/polic...
File chrome/test/data/policy/policy_test_cases.json (left):

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/test/data/polic...
chrome/test/data/policy/policy_test_cases.json:323: "note": "TODO(joaodasilva):
Should be chrome://settings-frame/passwords. http://crbug.com/97749"
Yep, that's the plan.

https://chromiumcodereview.appspot.com/11272026/diff/1/chrome/test/data/polic...
chrome/test/data/policy/policy_test_cases.json:392: "settings_pages": []
On 2012/10/25 11:07:52, Joao da Silva wrote:
> Please leave a TODO here. This policy works, but needs to be set before the
> browser is launched. We can refactor PolicyPrefsTest later to do this for some
> of these tests.

Done.

[Joao da Silva, 2012-10-25 13:22:52]

lgtm

[bartfab (slow), 2012-10-25 14:10:13]

Pam, I removed the ids that were not being used anyway from two div elements in
the managed mode banner. You added the banner, so it would be great if you could
review that bit.

[Pam (message me for reviews), 2012-10-25 14:19:26]

https://chromiumcodereview.appspot.com/11272026/diff/8001/chrome/browser/reso...
File chrome/browser/resources/extensions/extensions.html (right):

https://chromiumcodereview.appspot.com/11272026/diff/8001/chrome/browser/reso...
chrome/browser/resources/extensions/extensions.html:45: <div class="page-banner
managed-mode-banner">
extensions.css uses #managed-mode-banner

It should be OK to change it to .managed-mode-banner instead.

[bartfab (slow), 2012-10-25 14:26:28]

https://chromiumcodereview.appspot.com/11272026/diff/8001/chrome/browser/reso...
File chrome/browser/resources/extensions/extensions.html (right):

https://chromiumcodereview.appspot.com/11272026/diff/8001/chrome/browser/reso...
chrome/browser/resources/extensions/extensions.html:45: <div class="page-banner
managed-mode-banner">
Oh, I just knew I would confuse id and class :(. Fixed.

[Pam (message me for reviews), 2012-10-25 14:49:43]

LGTM, assuming you've done the quick manual test again.

- Pam

[James Hawkins, 2012-10-25 15:25:24]

lgtm

[commit-bot: I haz the power, 2012-10-25 19:55:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bartfab@chromium.org/11272026/13002

[commit-bot: I haz the power, 2012-10-25 22:36:18]

Change committed as 164195