New issue
Advanced search Search tips

Issue 664048 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug

Blocking:
issue 671375



Sign in to add a comment

md settings - prefetch has bad state

Reported by geki...@gmail.com, Nov 10 2016

Issue description

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

Steps to reproduce the problem:
1. disable prefetch in (normal) settings
2. enable md settings in flags
3. look at the prefetch setting (it's enabled)

What is the expected behavior?
should have the setting of (normal) settings

What went wrong?
if you change the settings page it's enabled

Did this work before? N/A 

Chrome version: 56.0.2906.0  Channel: dev
OS Version: 10.0
Flash Version: Shockwave Flash 24.0 r0
 

Comment 1 by ajha@chromium.org, Dec 21 2016

Components: -UI UI>Settings
Labels: M-57 Proj-MaterialDesign-WebUI OS-Linux OS-Mac
Able to reproduce the issue on the latest canary(57.0.2959.0) on Windows-10, Linux Ubuntu 14.04 and Mac OS 10.12.2 as per the above test steps. Prefetch settings(Use a prediction service to load pages more quickly) checkbox is checked under chrome://settings page.

Leaving the bug Unconfirmed as per the update from  Issue 628065 , C#4.

Comment 2 by shrike@chromium.org, Jan 13 2017

Owner: dbeam@chromium.org
Status: Untriaged (was: Unconfirmed)
-> dbeam@ for triage.
Blocking: 671375
Cc: dbeam@chromium.org
Labels: -Pri-2 Hotlist-MD-Settings-Privacy Pri-1
Owner: ----
Status: Available (was: Untriaged)
I'm not sure what's happening here, but seems like something we should fix before beta.
Owner: tommycli@chromium.org
I'll take a look at this. Thanks!
This is a legit bug. The underlying pref is numeric, and has special code to translate it to a checkbox in Old Options.

MD Settings will need to have that logic ported over. I'll start on this soon.
Status: Fixed (was: Available)
Fixed by https://codereview.chromium.org/2681373002/

Not sure why bot hasn't updated this bug yet.
Project Member

Comment 7 by bugdroid1@chromium.org, Feb 14 2017

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

commit 72af2d7297c841948cff5a1961634422c39ff51e
Author: dbeam <dbeam@chromium.org>
Date: Tue Feb 14 05:21:18 2017

Revert of MD Settings: Fix the Network Prediction toggle box. (patchset #4 id:60001 of https://codereview.chromium.org/2681373002/ )

Reason for revert:
Causing stack overflow errors on chrome://md-settings

Original issue's description:
> MD Settings: Fix the Network Prediction toggle box.
>
> In order to fix this toggle, I had to update the
> SettingsBooleanControlBehavior to support non 0/1 values for numeric
> prefs.
>
> BUG= 664048 
> CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
>
> Review-Url: https://codereview.chromium.org/2681373002
> Cr-Commit-Position: refs/heads/master@{#450063}
> Committed: https://chromium.googlesource.com/chromium/src/+/898ecc48fb6440230a819bcbd113f90c0d0df11e

TBR=stevenjb@chromium.org,tommycli@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 664048 

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

[modify] https://crrev.com/72af2d7297c841948cff5a1961634422c39ff51e/chrome/browser/resources/settings/controls/settings_boolean_control_behavior.js
[modify] https://crrev.com/72af2d7297c841948cff5a1961634422c39ff51e/chrome/browser/resources/settings/privacy_page/privacy_page.html
[modify] https://crrev.com/72af2d7297c841948cff5a1961634422c39ff51e/chrome/browser/resources/settings/privacy_page/privacy_page.js
[modify] https://crrev.com/72af2d7297c841948cff5a1961634422c39ff51e/chrome/test/data/webui/settings/settings_toggle_button_tests.js

Comment 8 by dbeam@chromium.org, Feb 14 2017

Status: Started (was: Fixed)
Status: Fixed (was: Started)

Comment 11 by dbeam@chromium.org, Feb 18 2017

 Issue 693701  has been merged into this issue.

Comment 12 by jleedev@gmail.com, Mar 13 2017

This still seems broken:

My machine's policy has the value forced to "2", but it shows as enabled.

In Chromium, which ignores policy, incorrect behavior is seen:
- Load md-settings and settings-frame side by side.
- Disabling in one will enable it in the other.

Comment 13 by dbeam@chromium.org, Mar 13 2017

jleedev@: which version of chrome are you using?  canary?

Comment 14 Deleted

Comment 15 Deleted

Comment 16 by jleedev@gmail.com, Mar 14 2017

Yes, this is canary. On windows.

(My comments are showing as deleted. Maybe not writing the exact same comment will help.)

Comment 17 by jleedev@gmail.com, Mar 14 2017

Here's the screenshot I took yesterday showing the static state with the conflicting checkboxes. The version was 59.0.3040.0 / r456312.


chrome_2017-03-13_11-28-18.png
216 KB View Download

Comment 18 by dbeam@chromium.org, Mar 14 2017

Status: Assigned (was: Fixed)
tommycli@: something about this doesn't seem to be working with policy
Okay cool, I'll take a second look.

Comment 20 by jleedev@gmail.com, Mar 15 2017

The same behavior -- disabling one enables the other -- is seen on Mac. And there's no policy here.
Project Member

Comment 21 by bugdroid1@chromium.org, Mar 15 2017

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

commit 7d113d193eb693c2c8d2d60e61744fe3c3e5dec5
Author: tommycli <tommycli@chromium.org>
Date: Wed Mar 15 17:50:19 2017

MD Settings: Fix broken numeric prefs using boolean controls

We put readOnly: true on the numeric unchecked value property. However,
this prevents the host from binding the initial value, and prevents
the functionality from working at all.

BUG= 664048 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/7d113d193eb693c2c8d2d60e61744fe3c3e5dec5/chrome/browser/resources/settings/controls/settings_boolean_control_behavior.js
[modify] https://crrev.com/7d113d193eb693c2c8d2d60e61744fe3c3e5dec5/chrome/test/data/webui/settings/settings_toggle_button_tests.js

Comment 22 by dbeam@chromium.org, Mar 15 2017

Status: Fixed (was: Assigned)

Sign in to add a comment