Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sync does not put Android bookmarks into folders #2447

Closed
GeetaSarvadnya opened this issue Dec 11, 2018 · 12 comments · Fixed by brave/brave-core#1079
Closed

sync does not put Android bookmarks into folders #2447

GeetaSarvadnya opened this issue Dec 11, 2018 · 12 comments · Fixed by brave/brave-core#1079

Comments

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Dec 11, 2018

Description

Empty folders are synced in win, no bookmarks inside the folders.

Devices

Device 1: Windows 10 (Sync chain creator) - 0.58.11
Device 2: Android ARM - Samsung Galaxy J3 - 1.0.71 (sync2)

Steps to Reproduce

See also #2447 (comment) for STR.

  1. Create sync chain on device 1
  2. Open QR code on Windows and scan QR code from Android
  3. Verify that sync is established between Win and Android
  4. Add few bookmarks in Win ( Add couple of BM's into the folder and leave few BM's on Toolbar)
  5. Add 4 BM's in Android
  6. Edit bookmarks 1 and add it to folder A
  7. Edit bookmark 2 and add it to folder B
  8. leave 2 BM's outside the folders
  9. Make sure all the Windows BM's are syched in Android
  10. Verify that all the BM's from Android are synched in Win

Actual result:

All the Android BM's are imported outside the folders, folder A and folder B are empty, no BM's inside the folders - Waited for almost 15 mins

Console log error:

[2548:2356:1211/161535.544:ERROR:CONSOLE(0)] "Error in event handler: TypeError: Cannot read property 'url' of undefined
at Object.NodeState.editBookmark (chrome://bookmarks/crisper.js:36:3143)
at Object.NodeState.updateNodes (chrome://bookmarks/crisper.js:36:4691)
at Object.reduceAction (chrome://bookmarks/crisper.js:36:7120)
at Store.reduce_ (chrome://bookmarks/crisper.js:44:1112)
at chrome://bookmarks/crisper.js:44:960
at Store.dispatchInternal_ (chrome://bookmarks/crisper.js:44:1015)
at Store.dispatchAsync (chrome://bookmarks/crisper.js:44:869)
at Store.dispatch (chrome://bookmarks/crisper.js:44:927)
at dispatch (chrome://bookmarks/crisper.js:48:726)
at onBookmarkChanged (chrome://bookmarks/crisper.js:48:785)", source: chrome://bookmarks/?id=3 (0)
[2548:2356:1211/161535.544:ERROR:CONSOLE(0)] "Error in event handler: TypeError: Cannot read property 'url' of undefined
at Object.NodeState.editBookmark (chrome://bookmarks/crisper.js:36:3143)
at Object.NodeState.updateNodes (chrome://bookmarks/crisper.js:36:4691)
at Object.reduceAction (chrome://bookmarks/crisper.js:36:7120)
at Store.reduce_ (chrome://bookmarks/crisper.js:44:1112)
at chrome://bookmarks/crisper.js:44:960
at Store.dispatchInternal_ (chrome://bookmarks/crisper.js:44:1015)
at Store.dispatchAsync (chrome://bookmarks/crisper.js:44:869)
at Store.dispatch (chrome://bookmarks/crisper.js:44:927)
at dispatch (chrome://bookmarks/crisper.js:48:726)
at onBookmarkChanged (chrome://bookmarks/crisper.js:48:785)", source: chrome://bookmarks/?id=3 (0)

Expected result:

Folders should not be empty

Reproduces how often:

Always

Brave version (brave://version info)

Brave 0.58.11 Chromium: 71.0.3578.80 (Official Build) beta (64-bit)
Revision 2ac50e7249fbd55e6f517a28131605c9fb9fe897-refs/branch-heads/3578@{#860}
OS Windows

Reproducible on current release:

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

Website problems only:

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

Additional Information

@LaurenWags
Copy link
Member

LaurenWags commented Dec 12, 2018

Reproduced with Device 1 as macOS (using build below) and Device 2 is Android

Brave 0.58.11 Chromium: 71.0.3578.80 (Official Build) beta(64-bit)
Revision 2ac50e7249fbd55e6f517a28131605c9fb9fe897-refs/branch-heads/3578@{#860}
OS Mac OS X

@bbondy
Copy link
Member

bbondy commented Dec 12, 2018

I think empty folders should be synced, closing. LMK if I'm wrong though.

@bbondy bbondy closed this as completed Dec 12, 2018
@bbondy bbondy modified the milestones: 0.58.x - Release, Dupe / Invalid / Not actionable Dec 12, 2018
@LaurenWags
Copy link
Member

LaurenWags commented Dec 12, 2018

@bbondy I think we should change the issue title - what i'm seeing (and I think @GeetaSarvadnya is too based on what she wrote in Actual Result: All the Android BM's are imported outside the folders, folder A and folder B are empty, no BM's inside the folders - Waited for almost 15 mins) is that if you have a folder on android which contains bookmarks, when that folder is synced to my laptop the folder is empty. the bookmarks are not inside the folder. If you take a look at my screenshots in the comment above, hopefully you can see what i mean. On the laptop the folder and bookmarks are there, but the bookmarks are not in the folder.

@LaurenWags LaurenWags changed the title Empty folders are synced in win, no bookmarks inside the folders. sync does not put Android bookmarks into folders Dec 12, 2018
@bbondy bbondy reopened this Dec 12, 2018
@LaurenWags LaurenWags modified the milestones: Dupe / Invalid / Not actionable, 0.58.x - Release Dec 12, 2018
@LaurenWags LaurenWags added the priority/P2 A bad problem. We might uplift this to the next planned release. label Dec 12, 2018
@LaurenWags
Copy link
Member

Additional Steps to Reproduce:

  1. Clean install 0.58.11.
  2. Import some bookmarks (I imported about 45 from an HTML file).
  3. Create Sync chain.
  4. Install 1.0.71 sync2 on Android device.
  5. Join sync chain.
  6. Add bookmarks (and put some of those bookmarks into folders) on Android device.
  7. Wait for bookmarks to be synced to laptop.
  8. When Sync occurs, the bookmarks are not in the folder(s) you created on Android. (screenshots in sync does not put Android bookmarks into folders #2447 (comment))

I'm unable to test this with adding bookmarks to Android device prior to joining sync chain (essentially reverse steps 5 and 6 above) due to a different issue: https://github.com/brave/browser-android-tabs/issues/924

@LaurenWags
Copy link
Member

seems like maybe the bookmark(s) on Android are being synced before I get them into a folder on my device? Then they get to laptop (Device 1) that way - meaning, out of the folder. Then this incorrect placement gets synced back to the Android device (Device 2).

@btlechowski
Copy link

Reproduced on Windows 7 x64(0.58.11) and Samsungs s7 (1.0.71(sync2)) using #2447 (comment) Steps.

image

@AlexeyBarabash
Copy link
Contributor

Just verified PR brave/brave-core#1079 fixed issue.

@GeetaSarvadnya
Copy link
Author

GeetaSarvadnya commented Dec 13, 2018

Issue is still not fixed, all the bookmarks are synced outside the folders on Windows (device1). Folders A and B are still empty in device1. Also, there is no Mobile Bookmarks folder on windows, now the folder A and B are displayed under Bookmarks bar which is not correct.
image

Android (device 2):

image
Console log error:
image

@darkdh
Copy link
Member

darkdh commented Dec 13, 2018

@GeetaSarvadnya , the fix hasn't released with official build. @AlexeyBarabash was using his own build to verify

@AlexeyBarabash
Copy link
Contributor

@GeetaSarvadnya , sorry for confusing. I was using own build from the latest master with merged PR.

@kjozwiak
Copy link
Member

@GeetaSarvadnya thanks for checking this again! There were a few issues fixed relating to sync that went into 0.58.12 so I thought this might have been fixed. Looks like brave/brave-core#1079 needs to land before we can re-check. We'll need to wait for new builds 👍 Hopefully we'll have a new build by tomorrow AM.

@GeetaSarvadnya
Copy link
Author

GeetaSarvadnya commented Dec 14, 2018

Verification passed on.

Brave 0.59.8 Chromium: 71.0.3578.98 (Official Build) beta (64-bit)
Revision 15234034d19b85dcd9a03b164ae89d04145d8368-refs/branch-heads/3578@{#897}
OS Windows

image

Verified passed with

Brave 0.59.14 Chromium: 72.0.3626.28 (Official Build) beta(64-bit)
Revision 997b1040b63bac324e815797ba52be0cd8f616ed-refs/branch-heads/3626@{#461}
OS Mac OS X

Verification passed on

Brave 0.59.14 Chromium: 72.0.3626.28 (Official Build) beta(64-bit)
Revision 997b1040b63bac324e815797ba52be0cd8f616ed-refs/branch-heads/3626@{#461}
OS Linux

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment