Bookmarks is sent via sync even when bookmark sync is disabled

[srirambv]

Description

Bookmarks is sent via sync even when bookmark sync is disabled

Devices

Device 1: Desktop b-c 0.57.6
Device 2: Samsung Tab (Android 8.1)

Steps to Reproduce

1. Import 5k bookmarks file on b-c
2. Enable sync on b-c
3. Disable bookmark sync on b-c
4. Visit any site in a new tab and add it as bookmark
5. Add an Android device to sync chain
6. Several bookmarks are sync'd on Android even though bookmark sync is disabled in step 3

Actual result:

b-c sync code

Bookmarks on b-c


Sync code on Android

Bookmarks on Android

Expected result:

Should not sync any bookmarks if b-c has bookmark sync disabled

Reproduces how often:

Easy

Brave version (brave://version info)

Brave	0.57.6 Chromium: 71.0.3578.31 (Official Build) beta (64-bit)
Revision	c88fdf2a4ce19a713615ca4fbde7a0d0b5fe2363-refs/branch-heads/3578@{#427}
OS	Windows

Reproducible on current release:

• Does it reproduce on brave-browser dev/beta builds?
  Yes on Beta

Website problems only:

• Does the issue resolve itself when disabling Brave Shields?
• Is the issue reproducible on the latest version of Chrome?

Additional Information

[darkdh]

disable bookmark sync doesn't mean delete all bookmark on sync chain.
This is expected behavior, Android pulls existing bookmarks from sync chain.
The scenario you should check is after disable bookmark sync, "NEW ADDED" bookmarks on brave-core shouldn't appear in Android.

[srirambv]

@darkdh agree that disable bookmark sync doesn't mean delete all bookmarks from chain but without it being enabled on b-c why is Android pulling in bookmarks ?

This is expected behavior, Android pulls existing bookmarks from sync chain.

This is correct if bookmark sync is enabled on b-c but in this scenario its disabled before adding the mobile device to the chain so ideally there shouldn't be any bookmarks available to be pulled in by Android. If Android is pulling in existing bookmarks even with bookmarks sync disabled in b-c that defeats the purpose of having a switch to turn off bookmark sync.

The scenario you should check is after disable bookmark sync, "NEW ADDED" bookmarks on brave-core shouldn't appear in Android.

This is correct but what happens here is the existing bookmarks are getting sync'd to Android when bookmark sync is turned off on b-c which is still a bug.

[darkdh]

if you don't want Android pulling bookmarks, disable the bookmark sync on Android.
As long as you don't delete sync chain, bookmarks will always be there.
bookmark sync switch only control brave-core send/pull new bookmarks, it will not affect other devices.

[darkdh]

and your title is not even accurate. Some bookmarks has already been sent when you enable sync.
You should revise your steps to

1. Enable sync on b-c
2. Disable bookmark sync on b-c
3. Import 5k bookmarks file on b-c

[srirambv]

I dont think the steps needs to be revised. The one you are mentioning is a different scenario. The scenario here is I already have bookmarks on my profile and then enable sync and turn off bookmark sync which is correctly mentioned in the original str.

This would be the scenario for user who go on upgrade profile from current release and already have bookmarks added before enabling sync

[srirambv]

if you don't want Android pulling bookmarks, disable the bookmark sync on Android.

Android doesn't allow you turn off only that setting similar to b-c. Its either on or off so this is invalid

[SergeyZhukovsky]

@srirambv when bc sends couple bookmarks to the chain in step #2 it's done. it cannot influence in any manner to the chain, it is just off to send new records there in step 3. All new devices that join that chain will receive the existing bookmarks in the chain and they have no idea were they sent from bc with disabled bookmarks or from another mobile and etc. The bc switch has an affect to the feature bookmarks only. The way we can control that is only like @darkdh described by having a switch on mobile for bookmarks and disabling it.

[srirambv]

Ok. I'll add a feature request for Android to have the switch to avoid this scenario

[AlexeyBarabash]

This issue has STR which are not actual since new design. At the current time it is impossible to add only one device in the chain. Also with PR brave/brave-core#2016 brave-core does not sends data to sync cloud until sync chain contains 2 or more devices.