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

Folder created on Android shows up without name #2144

Closed
srirambv opened this issue Nov 14, 2018 · 5 comments · Fixed by brave/brave-core#954
Closed

Folder created on Android shows up without name #2144

srirambv opened this issue Nov 14, 2018 · 5 comments · Fixed by brave/brave-core#954

Comments

@srirambv
Copy link
Contributor

Description

Folder created on Android shows up without name

Devices

Device 1: Desktop Beta (sync chain creator)
Device 2: Samsung Tab (Android 8.1)

Steps to Reproduce

  1. Enable Sync on desktop
  2. Sync Android using QR code
  3. Create a bookmark folder on Android and wait for it to sync on desktop
  4. Creates a folder without name and contains any bookmarks added on Android

Actual result:

image
image

Expected result:

Should show the folder name correctly that was created on Android

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?

Website problems only:

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

Additional Information

@AlexeyBarabash
Copy link
Contributor

Here are the reason why this happened:

##brave-core##
Bookmark send ->       
Folder send ->	       
			BookmarkChangeProcessor::BookmarkNodeToSyncBookmark
			bookmark->site.title = base::UTF16ToUTF8(node->GetTitledUrlNodeTitle());
			bookmark->site.customTitle = base::UTF16ToUTF8(node->GetTitle());
		
Bookmark recieve ->    
Folder receive ->      
			model->SetTitle(node, base::UTF8ToUTF16(bookmark.site.title));

##Android##
Bookmark send -> 
			formRequestByBookmarkItem
			title:bookmarkItem.getTitle()
			customTitle:""

Folder send -> 		CreateBookmarkRecord
			title:""
			customTitle:<>
			

Bookmark receive -> 	
Folder receive ->
			BookmarkResolver uses first non-empty
			1) title 
			2) custom 

According to
https://cs.chromium.org/chromium/src/components/bookmarks/browser/bookmark_node.cc?sq=package:chromium&g=0&l=102
BookmarkNode::GetTitledUrlNodeTitle
and
BookmarkNode::GetTitle
return the same data.

I thought in brave-sync bookmark->site.title means the title received from html/head field, but bookmark->site.customTitle is the value, which is set when bookmark info is edited manually. But I see I cannot diffirentiate it in Chromium bookmark. So going to apply into brave-core fix to use either bookmark.site.title or bookmark->site.customTitle.
@SergeyZhukovsky @samartnik

@AlexeyBarabash
Copy link
Contributor

Made PR brave/brave-core#954 .

@LaurenWags
Copy link
Member

Attempted to test with 0.57.8 beta on macOS however, since #2211 was implemented on b-c, b-c points to prod and the latest Android Sync build available points to Staging, this cannot be tested until we have an updated Android build pointing to production for Sync. Confirmed with @srirambv and marking as QA/Blocked until we have an Android build we can test with.
cc @kjozwiak

@bbondy
Copy link
Member

bbondy commented Nov 29, 2018

Removed 0.58.x and 0.59.x labels since only the smallest version it was merged to should be indicated.

@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Dec 12, 2018

Verification passed on

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

Windows:
image

Android:

image

Verified passed with

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

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