Closed Bug 625681 Opened 13 years ago Closed 13 years ago

Bookmark sync: bookmark item has GUID "mobile"

Categories

(Firefox :: Sync, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: philikon, Assigned: rnewman)

References

Details

A user reported a bookmark sync failure and provided the following log:

2011-01-13 20:07:33	Engine.Bookmarks     TRACE	Incoming: { id: mobile
  index: 340
  modified: 1294796229.04
  payload: {"id":"mobile","type":"bookmark","title":"Serials & keys","parentName":"Cracks / Serials","bmkUri":"http://www.serials.ws/","tags":[],"keyword":null,"loadInSidebar":false,"parentid":"tL9Kxbv-Ki2d"}
  collection: bookmarks }
2011-01-13 20:07:33	Engine.Bookmarks     TRACE	Reconcile step 1: Check for conflicts
2011-01-13 20:07:33	Engine.Bookmarks     TRACE	Finding mapping: Marque-pages non classés, fSerials & keys
2011-01-13 20:07:33	Engine.Bookmarks     TRACE	Mapped dupe: mobile
2011-01-13 20:07:33	Engine.Bookmarks     TRACE	Local record: { id: mobile
  index: 1000000
  modified: undefined
  payload: {"id":"mobile","type":"folder","parentName":"Marque-pages non classés","title":"Serials & keys","children":[],"parentid":"unfiled"}
  collection: bookmarks }
2011-01-13 20:07:33	Engine.Bookmarks     TRACE	Local record is different
2011-01-13 20:07:33	Engine.Bookmarks     TRACE	Record age vs local age: 149399.03999996185,1294945653.787
2011-01-13 20:07:33	Store.Bookmarks      TRACE	Reparenting to unfiled until parent is synced
2011-01-13 20:07:33	Store.Bookmarks      TRACE	Updating mobile (1393)
2011-01-13 20:07:33	Engine.Bookmarks     TRACE	Event: weave:engine:sync:error
2011-01-13 20:07:33	Engine.Bookmarks     DEBUG	Total (ms): sync 16284, processIncoming 13483, syncStartup 2786, createRecord 9027, isEqual 9308, reconcile 10116, syncCleanup 0
2011-01-13 20:07:33	Service.Main         DEBUG	bookmarks failed: NS_ERROR_ILLEGAL_VALUE JS Stack trace: Channel_onStopRequest([object XPCWrappedNative_NoHelper],null,2147942487)@resource.js:438 < Res__request(...)@resource.js:357 < Res_get()@resource.js:376 < SyncEngine__processIncoming()@engines.js:511 < _processIncoming()@bookmarks.js:212 < ()@engines.js:203 < SyncEngine__sync()@engines.js:768 < wrappedSync(null)@util.js:168 < runInBatchMode([object Object],null)@:0 < batchedSync()@util.js:174 < ()@engines.js:203 < WrappedNotify()@util.js:147 < Engine_sync()@engines.js:213 < WeaveSvc__syncEngine([object Object])@service.js:1738 < ()@service.js:1624 < WrappedNotify()@util.js:147 < WrappedLock()@util.js:119 < WrappedCatch()@util.js:97 < sync()@service.js:1529 < (3)@sync.js:380
2011-01-13 20:07:33	Service.Main         DEBUG	Exception: Some engines did not sync correctly No traceback available

For some reason the "mobile" GUID was assigned to a folder in Unsorted Bookmarks, so unsurprisingly the update process for that record fails.
Blocks: 621584
Bug 625810 has another set of logs with pretty much the same problem.
This is just weird.

Either the creator of the server record assigned the wrong GUID to a Places ID, and thus synthesized a bookmark with GUID "mobile", or somehow a folder got turned into a bookmark. That's not supposed to happen.

When Sync sees that, it dupes on the guid ("mobile"), and then tries to apply the record. That actually doesn't fail in my test, but I can see why it would -- setting certain fields on folders is likely to cause an error.

It would be really nice to figure out how that record was created. I know the UI will allow you to move and rename the Mobile Bookmarks folder, but that still should never cause it to be duped with a bookmark so as to cause this to happen.

There are things that Sync could do to handle this better -- skip over the error, of course, but also don't dupe two items with the same GUID but obviously incompatible types.

I will mull this over more over the weekend. Maybe there's some livemark translation or something that'll cause the problem.

Changing the subject: it's OK for a folder to have GUID = "mobile"; it's not OK for a bookmark (i.e., with URI) to do so!
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Summary: Bookmark sync: local folder has GUID "mobile" → Bookmark sync: bookmark item has GUID "mobile"
So Places GUIDs must be 12 characters base64url now. I believe Places actually checks for that. The BookmarksEngine recognizes certain special Places items, however, and gives them different GUIDs from what they have assigned locally: the Unsorted Bookmarks Folder, the Bookmarks Toolbar, etc., as well as the mobile query (smart bookmark) we add.

It's definitely not OK for *anything* but our mobile query to have this GUID. I wonder if applyIncoming handles it incorrectly. We special-case those special items and don't even apply them locally (we only order their children if they have any). Perhaps that somehow messes things up.
Depends on: 627490
A summary of what we think happened here:

* Places reassigns IDs. For example, if we create Mobile Bookmarks, and the user deletes it and immediately adds a new bookmark, the new item can get the ID we assigned to Mobile Bookmarks. (At least, that's what sdwilsh says, and I'm inclined to believe him.)

* We cache (or did) those IDs, and use them when assigning special GUIDs to outgoing items.

* An item grabbed the 'wrong' ID, and then we gave it the wrong GUID on upload. On download we see any old bookmark with the "mobile" GUID.

Bug 627490 is to fix the caching.

Users can also make a copy of the Mobile Bookmarks folder, and that carries its annotation with it. They can modify that copy, so it's possible that we will see folders with arbitrary names, in arbitrary places in the Bookmarks hierarchy, with the GUID "mobile". That's OK. The copies with the same annotation aren't so OK -- that's Bug 627519.
Status: ASSIGNED → NEW
Depends on: 627519
This is resolved now, I think: by no longer caching IDs, we avoid most of the danger. Follow-on in Bug 627519 doesn't need to keep this open.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Seems also resolved for me, the problem I had (Bug 625810) is solved in 4.0b11, I can now sync correctly.
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.