New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 711061 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug

Blocking:
issue 671375



Sign in to add a comment

Chrome Default Browser setting not allowed

Reported by weaver.j...@gmail.com, Apr 12 2017

Issue description

UserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36

Steps to reproduce the problem:
1. Set the policy to block a user from making chrome the default browser.
2. Go to chrome://settings
3. Also go to chrome://md-settings

What is the expected behavior?
Making chrome the default browser should be blocked through both of those settings pages

What went wrong?
On the chrome://md-settings page, it allows to set chrome as the default browser which should not be allowed when the policy is set.

Did this work before? N/A 

Chrome version: 57.0.2987.133  Channel: stable
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
Flash Version: Shockwave Flash 25.0 r0
 
Cc: nyerramilli@chromium.org ligim...@chromium.org ajha@chromium.org
Cc: pastarmovj@chromium.org
Status: Untriaged (was: Unconfirmed)
Thanks for the report.

Able to reproduce the issue on Win7 (Client) & Win Server using Stable 57.0.2987.133.
chrome://settings - unable to change the default browser setting
chrome://md-settings - Able to change the default browser setting

attached screen cast for reference.

adding pastarmovj@/blumberg@, could you please check the issue.
711061.jpg
619 KB View Download
Cc: dbeam@chromium.org
Adding dbeam who is working on md-settings in CC.

Dan is this something you can look into?

Comment 4 by dbeam@chromium.org, Apr 25 2017

Blocking: 671375
Labels: -Pri-2 M-59 Pri-1
Owner: dschuyler@chromium.org
Status: Assigned (was: Untriaged)
is this still happening on canary?

Comment 5 by dbeam@chromium.org, Apr 25 2017

Cc: -dbeam@chromium.org tommycli@chromium.org dschuyler@chromium.org
Owner: dbeam@chromium.org
Status: Started (was: Assigned)
found the issue
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/21f439f4fe7a4589c9a0405bc6a385d34a1f3394

commit 21f439f4fe7a4589c9a0405bc6a385d34a1f3394
Author: dbeam <dbeam@chromium.org>
Date: Tue Apr 25 22:14:08 2017

MD Settings: fix policy control detection on default browser section

All settings/ code was migrated from the older options/ directory. In
this case, the old options default browser code used BooleanPrefMember,
which is a typed wrapper around a PrefService.  The code retrieved the
effective value of the preference (whether it's true or false, for a
bool) via BooleanPrefMember::GetValue().

The new code used a PrefService::Preference instead of a
BooleanPrefMember, and this also has a GetValue() method that returns a
base::Value* (NOT a boolean).  New code checked Preference::GetValue()
as an effective boolean value.  This is effectively dead code (it will
always succeed assuming the preference is registered correctly) and it
did not successfully detect when the preference was false (meaning, when
a policy was set to disable Chrome from setting the default browser).

This patch updates the new code to actually check the bool within the
base::Value* (rather than null check it in a way that always succeeds).

R=tommycli@chromium.org
BUG= 711061 

Review-Url: https://codereview.chromium.org/2838933002
Cr-Commit-Position: refs/heads/master@{#467133}

[modify] https://crrev.com/21f439f4fe7a4589c9a0405bc6a385d34a1f3394/chrome/browser/ui/webui/settings/settings_default_browser_handler.cc

Comment 7 by dbeam@chromium.org, Apr 25 2017

Labels: Merge-Request-59
Status: Fixed (was: Started)
Project Member

Comment 8 by sheriffbot@chromium.org, Apr 26 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 9 by bugdroid1@chromium.org, Apr 27 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9d10431610e8f8ee9c2cc4c8db4c80bc4b14d401

commit 9d10431610e8f8ee9c2cc4c8db4c80bc4b14d401
Author: Dan Beam <dbeam@chromium.org>
Date: Thu Apr 27 15:45:22 2017

MD Settings: fix policy control detection on default browser section

All settings/ code was migrated from the older options/ directory. In
this case, the old options default browser code used BooleanPrefMember,
which is a typed wrapper around a PrefService.  The code retrieved the
effective value of the preference (whether it's true or false, for a
bool) via BooleanPrefMember::GetValue().

The new code used a PrefService::Preference instead of a
BooleanPrefMember, and this also has a GetValue() method that returns a
base::Value* (NOT a boolean).  New code checked Preference::GetValue()
as an effective boolean value.  This is effectively dead code (it will
always succeed assuming the preference is registered correctly) and it
did not successfully detect when the preference was false (meaning, when
a policy was set to disable Chrome from setting the default browser).

This patch updates the new code to actually check the bool within the
base::Value* (rather than null check it in a way that always succeeds).

R=tommycli@chromium.org
BUG= 711061 

Review-Url: https://codereview.chromium.org/2838933002
Cr-Commit-Position: refs/heads/master@{#467133}
(cherry picked from commit 21f439f4fe7a4589c9a0405bc6a385d34a1f3394)

Review-Url: https://codereview.chromium.org/2846043002 .
Cr-Commit-Position: refs/branch-heads/3071@{#263}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/9d10431610e8f8ee9c2cc4c8db4c80bc4b14d401/chrome/browser/ui/webui/settings/settings_default_browser_handler.cc

Cc: kkaluri@chromium.org
Labels: TE-Verified-M59 TE-Verified-59.0.3071.36
Verified this issue in SkyTap Env "Windows GPO Testing - Beta - A" on Windows 10 with chrome #59.0.3071.36

Observed user is not able to change the default browser setting in chrome://md-settings

Attaching the screen-cast for reference.

Adding TE-Verified labels
Issue 711061.mp4
3.4 MB View Download

Sign in to add a comment