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

Issue 837260 link

Starred by 1 user
Status: Fixed
Owner:
treib@chromium.org
Closed: May 2018
Cc:
mastiz@chromium.org
█ tschumann@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Sync master toggle doesn't turn off bookmarks sub-toggle

Project Member Reported by tnagel@chromium.org, Apr 26 2018 
Chrome Version: ToT e57cac2093f53775cea49b6f848a402487e4cee0
OS: Linux

What steps will reproduce the problem?
(1) chrome --enable-logging=stderr --user-data-dir=/tmp/blah
(2) Click "SIGN IN" and sign in
--> "You've signed in and turned on Sync"
(3) Click "Visit Settings"
(4) Turn off master toggle ("Sync everything")

What is the expected result?
All sub-toggles should turn off, too.

What happens instead?
The "Bookmarks" toggle stays on.

Comment 1 by treib@chromium.org, Apr 26 2018

Labels: -ReleaseBlock-Beta OS-Chrome OS-Mac OS-Windows
Owner: treib@chromium.org
Status: Assigned (was: Untriaged)
Some background: There's a "preferred" pref for every sync data type. The "sync everything" toggle shadows those, but doesn't overwrite them. So if you have previously turned off "sync everything", then enabled bookmarks, then turned on "sync everything" again, then the observed behavior would be expected and WAI.

What's weird is that the pref for bookmarks gets enabled by default [1]. It's clearly done on purpose, but with no justification, and I have no idea what the reason might be. I'd suggest just removing that special casing to see if anything breaks.

Independent of that, given that this seems to have been the case since ~forever, I don't think this should be a release blocker.

[1] https://cs.chromium.org/chromium/src/components/sync/base/sync_prefs.cc?rcl=73661571dc1c1023571aac4f2a8e637eb2c21abd&l=66

Comment 2 by treib@chromium.org, Apr 30 2018

Status: Started (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, May 2 2018 

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

commit 5b782f79defe165ac655a9b6f5aa795bdc39f35e
Author: Marc Treib <treib@chromium.org>
Date: Wed May 02 10:41:13 2018

SyncPrefs: stop treating BOOKMARKS as enabled by default

SyncPrefs has a global "sync everything" pref, plus an individual pref
for every datatype. The datatype-specific prefs are all disabled by
default (which is fine because the global pref is enabled), except for
BOOKMARKS and DEVICE_INFO which were enabled by default for unknown
reasons. This CL removes the special treatment for BOOKMARKS.

Bug:  837260 
Change-Id: Ia5369596f8d9a5142eb51659d4418e492464b924
Reviewed-on: https://chromium-review.googlesource.com/1032443
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555341}
[modify] https://crrev.com/5b782f79defe165ac655a9b6f5aa795bdc39f35e/components/sync/base/sync_prefs.cc
[modify] https://crrev.com/5b782f79defe165ac655a9b6f5aa795bdc39f35e/components/sync/base/sync_prefs_unittest.cc

Comment 4 by treib@chromium.org, May 2 2018

Status: Fixed (was: Started)
This should fix the issue. Note that it will not change anything for existing profiles if you've ever toggles the "Sync bookmarks" switch.
►

Sign in to add a comment