1309255 - Ignore spurious records on server

Status: RESOLVED INVALID

(Firefox for iOS :: Sync, defect)

Description

[Richard Newman [:rnewman]]

In some circumstances, desktop has historically uploaded roots (including the Places root) when it shouldn't.

IIRC the Places root won't do any harm, and is already ignored (but double-check as part of this bug).

Other records will cause validation to fail.

This bug is to make sure that records on the server that:

* Are known to be spurious
* Have parents that are on an 'ignore' list

are (a) dropped from existing buffers, and (b) are dropped in future downloads.

Mark, could you add a pointer and/or description of which kinds of records need to be ignored?

Comment 1

[Richard Newman [:rnewman]]

Clarifying that this means roots, special places queries, and more besides.

Comment 2

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

(In reply to Richard Newman [:rnewman] from comment #1)
> Clarifying that this means roots, special places queries, and more besides.

To steal Kit's comment from bug 1258127: I think it's easier to list what is syncable: menu, toolbar, unfiled, mobile,
and their descendants. Everything else is non-syncable.

The underlying problem is that in the past, desktop could well have synced almost *any* bookmark record, regardless of where it lived in the hierarchy. If we consider that addons might have created their own hierarchy under a different "root" and that we might have synced them, then it's effectively impossible to define the set of things that should be ignored.

Assuming we can consider the entire tree as known by the server, and assuming that tree is complete and valid, then the definition above probably is useful - we can simply walk up to the root and see if it qualifies. Sadly though, I expect we will find such records on the server that form part of an orphaned branch, which would make it difficult to guess whether it is orphaned from a branch that would mean it can be synced, or one that isn't.

That said though, I expect the vast majority to be exactly as described by Richard:
* The root itself.
* The left-pane root and the items directly under that root which places automatigically creates as necessary - these are the "special queries" shown as top-level items in the bookmarks sidebar (but note that this does *not* include copies of those items - it's perfectly legal to copy the "All Bookmarks" left-pane item and paste a copy into any other folder. I *think* we can develop heuristics to identify them, but I haven't looked closely at that.)

The most recent analysis of the bookmark validation results implies that desktop will need to instigate a tree-wide repair of most users before we can sanely enable bidirectional sync for iOS, and that effort will necessarily need to face these same questions - so I suspect it will turn out that this burden is going to fall on desktop anyway (but having iOS skip the obviously-bad items might make sense regardless)

Comment 3

[Richard Newman [:rnewman]]

> Sadly though, I expect we will find such records on the
> server that form part of an orphaned branch…

Yeah, that's the problem I'm trying to solve.

iOS already knows which records aren't connected to the four roots it cares about… it just can't tell if those are the result of a partial write/missed delete/erroneous upload, or if they're something it can safely ignore.

Deliberately ignoring the Places root record (if it arrives) actually makes our job a little harder, because we can't tell what the server things is outside our synced tree.

We need some kind of heuristic we can use to recognize the root (fixed GUID?), the left-pane root (?), left-pane queries (are they all correctly pointing to the left-pane root?), and anything else you think might be hanging around.

I know the ones from my profile:

sqlite> select id, title, guid from moz_bookmarks where parent = 1;
id          title       guid
----------  ----------  ------------
4941                    uTqj0oRQ_Lck
5065                    iHjZXah7uyyG
4743        Reading Li  readinglist
2           Bookmarks   menu________
3           Bookmarks   toolbar_____
4           Tags        tags________
5           Other Book  unfiled_____
474         mobile      UjAHxFOGEqU8
4364                    BxfRgGiNeITG


Reading list is easy. Are these the left-pane queries?


sqlite> select id, title, guid, (select url from moz_places where id = fk) from moz_bookmarks where parent = 5065 OR parent = 4941 OR parent = 4364;
id          title       guid          (select url from moz_places where id = fk)
----------  ----------  ------------  ------------------------------------------
4365        History     5KiFoUj2pdYI  place:type=3&sort=4
4366        Downloads   Bl3n3gpKag3s  place:transition=7&sort=4
4367        Tags        0_VQZv0ADDYK  place:type=6&sort=1
4368        All Bookma  NzOTUjQyXrY6
4939        History     mpGbqa4W-qXO  place:type=3&sort=4
4936        Downloads   3UHG3mTtOY5F  place:transition=7&sort=4
4934        Tags        -odCcmbgqK81  place:type=6&sort=1
4940        All Bookma  sfXEBqQN7q0Z
5066        History     k-DpvqH9TnVU  place:type=3&sort=4
5064        Downloads   bW_8_GEplDsh  place:transition=7&sort=4
5061        Tags        0i6uBFPIzs7J  place:type=6&sort=1
5068        All Bookma  xnsv1Y4n3IDG

Comment 4

[Lina Butler [:lina]]

Those are, indeed, the left pane queries thrice duplicated. :-( If we buffer bookmarks on Desktop before applying them, we can filter out roots we don't care about. Of course, in case of a partial write or missed delete, we might end up with orphans that we need to ignore.

Comment 5

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

Attachment: Snapshot of the bookmarks validator for my real account

> We need some kind of heuristic we can use to recognize the root (fixed GUID?),

Yep, the root has a fixed GUID.

> the left-pane root (?), left-pane queries (are they all correctly pointing to
> the left-pane root?), and anything else you think might be hanging around.

These are the tricky ones :(

The attached screenshot is what about-sync is showing for my "real" account. Note that in this case the validator is saying these records appear on the client and not the server, but that's simply good luck - if this device was running 49-ish and ever ended up doing a full sync I'm confident they would be uploaded to the server. (I also just opened bug 1309774 to say the validator shouldn't actually complain about them in this way - if all goes well, newer firefoxes will *never* upload them)

What we seem to be seeing here is multiple copies of the left-pane special items. For some reason, I believe places decided my old left-pane items were invalid so it recreated them - my current, real left-pane root is *not* shown here - these are old ones.

For context, look at your "Show All Bookmarks" rather than the bookmarks sidebar or popup.

At index 0 is a "left pane root" - the parent GUID is "places" - this is the (invisible) root which holds all the top-level folders in "Show All Bookmarks".

At indexes 1-4 are the "left pane items" - the top-level items in "Show All Bookmarks" - ie, "Downloads", "History", "Tags" and "All Bookmarks". They all point to the item at index 0 as the parent and have URLs place:type=3&sort=4, place:transition=7&sort=4, place:type=6&sort=1 while "All Bookmarks" is listed as a folder rather than a bookmark.

At indexes 5-7 there are bookmarks with urls place:folder=TOOLBAR, place:folder=BOOKMARKS_MENU and place:folder=UNFILED_BOOKMARKS. All have their parent as the one at index 4 "All Bookmarks". These are, basically, shortcuts to our roots. The fact they are shortcuts is why places can nuke and recreate them at will - it's not touching the *actual* roots.

At index 8, the above all repeat - it is a "left pane root" with the parent as "places", then the same funky bookmarks as listed above. IOW, it looks as though my client has 2 stale copies of the left-pane special items, which never made it to the server.

Note also that if you manually create a shortcut, the URLs are different. For example, right-click "Bookmarks Toolbar", select "Copy", then paste it into, say "Bookmarks Menu". In that case, the copy of the bookmarks toolbar has a URL of "place:folder=TOOLBAR&excludeItems=1&expandQueries=0", whereas the original left-pane shortcut for that just has "place:folder=TOOLBAR".

So, without checking the validity of this, I'm vaguely hoping the heuristics can look something like:

* Any folder with a parent of "places" and no title is probably a left-pane root.
* Any bookmark (not folder) which has a left-pane root as a parent is a left-pane folder. In the absence of that signal (eg, the parent missing), a place:type= or place:transition= item is almost certainly a left-pane folder.
* Any folder which has a left-pane root as a parent is almost certainly the "All Bookmarks" left-pane folder. If the parent is missing I'm not sure what to do here.
* Any folder with place:folder=XXXX and without additional trailing query params is almost certainly a "left pane shortcut" inside of "All Bookmarks" (whereas ones *with* trailing query params are likely to be legitimate shortcuts created by the user)

Or something like that :/ This is all off the top of my head and I need to spend some time doing further analysis of PlacesUIUtils (which is the code that manages these special items)

Comment 6

[Mark Hammond (PTO; back Oct-3) [:markh] [:mhammond]]

app-services now does iOS bookmark sync.