Correct description in ADM/ADMX template and change registry key to "Chrome" from "Google Chrome" (issue2806040)

151 views
Skip to first unread message

da...@chromium.org

unread,
Jun 29, 2010, 6:01:15 AM6/29/10
to bau...@chromium.org, fma...@chromium.org, chromium...@chromium.org, ben...@chromium.org
Reviewers: Bernhard Bauer, fmantek1,

Message:
please review: frank the ADM/X changes, Bernhard for the committer's
LGTM :-)

Description:
Correct description in ADM/ADMX template and change registry key to "Chrome"
from "Google Chrome"

BUG=none
TEST=manual

Please review this at http://codereview.chromium.org/2806040/show

Affected files:
M chrome/app/policy/windows/adm/en-US/chrome.adm
M chrome/app/policy/windows/admx/en-US/chrome.adml
M chrome/browser/configuration_policy_provider_win.cc


Index: chrome/app/policy/windows/adm/en-US/chrome.adm
diff --git a/chrome/app/policy/windows/adm/en-US/chrome.adm
b/chrome/app/policy/windows/adm/en-US/chrome.adm
index
1cc79536f90f2f4ff689d3a5a3a7493cb9a3e7f2..47dc18f4faaf7984d44650862def0e4c5c9dd8c3
100644
--- a/chrome/app/policy/windows/adm/en-US/chrome.adm
+++ b/chrome/app/policy/windows/adm/en-US/chrome.adm
@@ -8,7 +8,7 @@ CLASS MACHINE
;
; For example: Software\Policies\Microsoft\Windows\Printing

- KEYNAME "Software\Policies\Google\Google Chrome"
+ KEYNAME "Software\Policies\Google\Chrome"

; This is a simple policy that sets the homepage, or if the homepage
should be
; the new tab page
@@ -153,7 +153,7 @@ into Google Chrome (such as 'page not found'.)"
SearchSuggestEnabled="Enable search suggestions."
SearchSuggestEnabled_Explain="This policy enables search suggestions in
the Omnibox."
DnsPrefetchingEnabled="Enable search suggestions."
-DnsPrefetchingEnabled_Explain="This policy enables search suggestions in
the Omnibox."
+DnsPrefetchingEnabled_Explain="This policy enables DNS prefething."
SafeBrowsingEnabled="Enable safe browsing."
SafeBrowsingEnabled_Explain="This policy enables Safe Browsing."
MetricsReportingEnabled="Enable reporting of usage and crash related data."
Index: chrome/app/policy/windows/admx/en-US/chrome.adml
diff --git a/chrome/app/policy/windows/admx/en-US/chrome.adml
b/chrome/app/policy/windows/admx/en-US/chrome.adml
index
81fd7f99b731fd88ad79cfbccd9c6db4397b5a96..57e7e95d89813eecfd233b53c9a2d000b1e893ee
100644
--- a/chrome/app/policy/windows/admx/en-US/chrome.adml
+++ b/chrome/app/policy/windows/admx/en-US/chrome.adml
@@ -6,7 +6,7 @@
<stringTable>
<string id="SUPPORTED_WINXPSP2">At least Microsoft Windows XP
SP2</string>
<string id="google">Google</string>
- <string id="googlechrome">Google Chrome</string>
+ <string id="googlechrome">Chrome</string>
<string id="HomePage">Set the default home page.</string>
<string id="HomePage_Explain">This policy setting specifies the
location of the default home page for the user.
The homepage can either be set to a URL you specify, or set to the New Tab
Page.
@@ -38,7 +38,7 @@ For detailed examples, visit:
http://www.chromium.org/developers/design-document
<string id="SearchSuggestEnabled">Enable search suggestions.</string>
<string id="SearchSuggestEnabled_Explain">This policy enables search
suggestions in the Omnibox.</string>
<string id="DnsPrefetchingEnabled">Enable search
suggestions.</string>
- <string id="DnsPrefetchingEnabled_Explain">This policy enables
search suggestions in the Omnibox.</string>
+ <string id="DnsPrefetchingEnabled_Explain">This policy enables DNS
prefething.</string>
<string id="SafeBrowsingEnabled">Enable safe browsing.</string>
<string id="SafeBrowsingEnabled_Explain">This policy enables Safe
Browsing.</string>
<string id="MetricsReportingEnabled">Enable reporting of usage and
crash related data.</string>
@@ -78,4 +78,5 @@ For detailed examples, visit:
http://www.chromium.org/developers/design-document
</presentation>
</presentationTable>
</resources>
-</policyDefinitionResources>
\ No newline at end of file
+</policyDefinitionResources>
+
Index: chrome/browser/configuration_policy_provider_win.cc
diff --git a/chrome/browser/configuration_policy_provider_win.cc
b/chrome/browser/configuration_policy_provider_win.cc
index
420ff3625780c2c2e10ff3a7ecb85690529793aa..d39adeef0dd5cdfe9ec2f287b70818d2950a14a9
100644
--- a/chrome/browser/configuration_policy_provider_win.cc
+++ b/chrome/browser/configuration_policy_provider_win.cc
@@ -16,7 +16,7 @@

#if defined(GOOGLE_CHROME_BUILD)
const wchar_t ConfigurationPolicyProviderWin::kPolicyRegistrySubKey[] =
- L"SOFTWARE\\Policies\\Google\\Google Chrome";
+ L"SOFTWARE\\Policies\\Google\\Chrome";
#else
const wchar_t ConfigurationPolicyProviderWin::kPolicyRegistrySubKey[] =
L"SOFTWARE\\Policies\\Chromium";


bau...@chromium.org

unread,
Jun 29, 2010, 6:03:43 AM6/29/10
to da...@chromium.org, fma...@chromium.org, chromium...@chromium.org, ben...@chromium.org

http://codereview.chromium.org/2806040/diff/1/2
File chrome/app/policy/windows/adm/en-US/chrome.adm (right):

http://codereview.chromium.org/2806040/diff/1/2#newcode155
chrome/app/policy/windows/adm/en-US/chrome.adm:155:
DnsPrefetchingEnabled="Enable search suggestions."
"Enable DNS prefetching"?

http://codereview.chromium.org/2806040/diff/1/2#newcode156
chrome/app/policy/windows/adm/en-US/chrome.adm:156:


DnsPrefetchingEnabled_Explain="This policy enables DNS prefething."

Nit: "prefetching".

http://codereview.chromium.org/2806040/diff/1/3
File chrome/app/policy/windows/admx/en-US/chrome.adml (right):

http://codereview.chromium.org/2806040/diff/1/3#newcode40
chrome/app/policy/windows/admx/en-US/chrome.adml:40: <string


id="DnsPrefetchingEnabled">Enable search suggestions.</string>

"Enable DNS prefetching"?

http://codereview.chromium.org/2806040/show

da...@chromium.org

unread,
Jun 29, 2010, 6:09:55 AM6/29/10
to bau...@chromium.org, fma...@chromium.org, chromium...@chromium.org, ben...@chromium.org

http://codereview.chromium.org/2806040/diff/1/2
File chrome/app/policy/windows/adm/en-US/chrome.adm (right):

http://codereview.chromium.org/2806040/diff/1/2#newcode155
chrome/app/policy/windows/adm/en-US/chrome.adm:155:
DnsPrefetchingEnabled="Enable search suggestions."

On 2010/06/29 10:03:44, Bernhard Bauer wrote:
> "Enable DNS prefetching"?

Done.

http://codereview.chromium.org/2806040/diff/1/2#newcode156
chrome/app/policy/windows/adm/en-US/chrome.adm:156:
DnsPrefetchingEnabled_Explain="This policy enables DNS prefething."

On 2010/06/29 10:03:44, Bernhard Bauer wrote:
> Nit: "prefetching".

Done.

http://codereview.chromium.org/2806040/diff/1/3
File chrome/app/policy/windows/admx/en-US/chrome.adml (right):

http://codereview.chromium.org/2806040/diff/1/3#newcode40
chrome/app/policy/windows/admx/en-US/chrome.adml:40: <string
id="DnsPrefetchingEnabled">Enable search suggestions.</string>

On 2010/06/29 10:03:44, Bernhard Bauer wrote:
> "Enable DNS prefetching"?

Done.

http://codereview.chromium.org/2806040/show

bau...@chromium.org

unread,
Jun 29, 2010, 6:11:33 AM6/29/10
to da...@chromium.org, fma...@chromium.org, chromium...@chromium.org, ben...@chromium.org

Frank Mantek

unread,
Jun 29, 2010, 6:23:06 AM6/29/10
to da...@chromium.org, bau...@chromium.org, fma...@chromium.org, chromium...@chromium.org, ben...@chromium.org
LGTM as well.

On Tue, Jun 29, 2010 at 12:11 PM, <bau...@chromium.org> wrote:
LGTM.

da...@chromium.org

unread,
Jun 29, 2010, 9:52:12 AM6/29/10
to bau...@chromium.org, fma...@chromium.org, chromium...@chromium.org, ben...@chromium.org
sorry fellas, here's another round. After finding a few further minor nits
myself, I realized that the wording, punctuation and verbiage of our
template
doesn't match the other templates that Microsoft provides. I tried my
hardest to
make the templates conform, too.

http://codereview.chromium.org/2806040/show

Frank Mantek

unread,
Jun 29, 2010, 10:01:01 AM6/29/10
to da...@chromium.org, bau...@chromium.org, fma...@chromium.org, chromium...@chromium.org, ben...@chromium.org
Shouldn't the wording match the other templates Google provides?

(not sure that there is a difference).

Why did you change the Homepage UI to have a dropdown and an edittext vs a checkbox and an edit text?

Frank

da...@chromium.org

unread,
Jun 30, 2010, 3:44:44 AM6/30/10
to bau...@chromium.org, fma...@chromium.org, chromium...@chromium.org, ben...@chromium.org, p...@chromium.org, mnis...@chromium.org, gfe...@google.com, markus...@chromium.org
Actually, I think it's better that our templates are follow the "typical
windows" style. If the Omaha templates don't, then maybe we might want to
address that separately.

Here are examples of the things I changed. I observed them in the
majority of
the random sampling of built-in windows policies that I looked at:

- No punctuation for checkbox policy elements, unless in the descriptive
text
before. Colon after label on elements for everything else (e.g. drop-downs,
edit
text fields)
- Descriptions do not start with "This policy...", rather they start with a
verb.
- Policy descriptions explicitly illustrates what the policy does or does
not do
if it is enabled, disabled or not configured.

The drop down is needed because an unchecked checkbox doesn't set the
corresponding registry value to "0", even with a VALUEOFF modifier. This
only
works for policies at the top level. This means the template didn't allow an
administrator to specify "use this homepage URL always", because it wasn't
possible to set the HomepageIsNewTab to "0", rather then just unset.

http://codereview.chromium.org/2806040/show

Frank Mantek

unread,
Jun 30, 2010, 4:56:48 AM6/30/10
to da...@chromium.org, bau...@chromium.org, fma...@chromium.org, chromium...@chromium.org, ben...@chromium.org, p...@chromium.org, mnis...@chromium.org, gfe...@google.com, markus...@chromium.org
In that case. LGTM
Reply all
Reply to author
Forward
0 new messages