747699 - Bookmark reconciling uses parent name when parent ID would yield better results

Status: RESOLVED DUPLICATE of bug 1305563

(Firefox :: Sync, defect, P3)

Description

[Richard Newman [:rnewman]]

I noticed that bookmarks I created in Fennec were being duplicated on desktop.

Here's the reason.

If we map an item by GUID, all is well. Of course, this doesn't always occur.

If that doesn't happen, we search in guidMap. We do this by searching under the parent name (which can collide, of course!) and then by a concatenated search string.

If a record arrives like this (forgive abbreviated JSON):

  {id: "abcdefghijkl",
   parentId: "mobile",
   parentName: "Mobile Bookmarks",
   bmkUri: "http://slashdot.org/",
   title: "Slashdot"}

and locally we have

  {id: "gggggggggggg",
   parentId: "mobile",
   parentName: "mobile",
   bmkUri: "http://slashdot.org/",
   title: "Slashdot"}

-- because Places doesn't store a real name for Mobile Bookmarks -- then our guidMap search *will yield nothing* because there is no folder named "Mobile Bookmarks".

We'll deduce that this is a new record, and yet we'll store it in the folder with ID "mobile".

Now you have two Slashdot bookmarks in the same folder, despite not being able to find one during reconciling!

Bug.

Comment 1

[Richard Newman [:rnewman]]

I believe this *only* applies to the "mobile" folder.

Comment 2

[Richard Newman [:rnewman]]

Fixed on Android (Bug 748898) by translating the names of special folders inbound, to the local expected value, and outbound, to the same (which happens to be what desktop expects).

Desktop could do something similar here.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

CC mak as this bug is relevant to the discussion in bug 1012597.

Comment 4

[Mark Hammond [:markh] [:mhammond]]

(In reply to Richard Newman [:rnewman] from comment #0)
> If a record arrives like this (forgive abbreviated JSON):
> 
>   {id: "abcdefghijkl",
>    parentId: "mobile",
>    parentName: "Mobile Bookmarks",
>    bmkUri: "http://slashdot.org/",
>    title: "Slashdot"}
> 
> and locally we have
> 
>   {id: "gggggggggggg",
>    parentId: "mobile",
>    parentName: "mobile",
>    bmkUri: "http://slashdot.org/",
>    title: "Slashdot"}
> 
> -- because Places doesn't store a real name for Mobile Bookmarks -- then our
> guidMap search *will yield nothing* because there is no folder named "Mobile
> Bookmarks".

I don't quite understand this - how did the name get different? Because they were created on different devices, or due to the user renaming? I guess the former, but if the latter, then this sounds like bug 1276969.

> I believe this *only* applies to the "mobile" folder.

Is this just because Fennec doesn't touch the tree structure for any other folders, or some other reason? ie, it's not immediately obvious why this isn't a problem for the other top-level folders we care about.

Comment 5

[Richard Newman [:rnewman]]

I've never renamed. I don't know why desktop just uses "mobile":

sqlite> select guid, title from moz_bookmarks where id = 474;
guid          title
------------  ----------
UjAHxFOGEqU8  mobile

Mobile platforms upload records with descriptive names ("Mobile Bookmarks").

New desktop profiles might create Mobile Bookmarks with a better name -- I see `mobile.label` = "Mobile Bookmarks" in fx-team.


Presumably this would also happen when syncing devices in different locales; my Armenian device sure isn't going to have "Bookmarks Toolbar", but it'll have the same GUID as my English device! So I was wrong: this probably affects _any_ default folder in that case.


Checking for GUID before we check for parent name might be tricky with guidMap as it stands, but it does get us closer to how iOS does content-based reconciling: top-down, same parent folder only, not parent folder of the same name!

Comment 6

[Thom Chiovoloni [:tcsc] (ex-moco)]

Changing the component in an effort to make this show up on the sync triage list.

Comment 7

[Lina Butler [:lina]]

Added a test to bug 1305563.