Closed Bug 1285324 Opened 8 years ago Closed 8 years ago

Bookmarks engine shouldn't change root GUIDs

Categories

(Firefox :: Sync, defect, P2)

Product:
Firefox
Component:
Sync
Type:
defect

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: lina, Unassigned)

Details

(Whiteboard: [data-integrity])

It's possible for the reconciliation logic to get confused and try to reconcile a root (http://searchfox.org/mozilla-central/rev/a7c8e9f3cc323fd707659175a46826ad12899cd1/services/sync/tests/unit/test_bookmark_engine.js#480-542). In this case, `nsINavBookmarkService.moveItem` will throw, since roots can't be moved...but Sync will still change the root's GUID! This will break the bookmarks service on the next launch, since it uses the GUIDs to find the root IDs (http://searchfox.org/mozilla-central/rev/a7c8e9f3cc323fd707659175a46826ad12899cd1/toolkit/components/places/nsNavBookmarks.cpp#185-226). It'll also break calls in `Bookmarks.jsm` that check for root GUIDs, and probably more. Even though it's a semantic change, we shouldn't let Sync change root GUIDs, because nothing good can come from that. I added some notes in bug 1274108, comment 38, too.
Kit, I believe this already landed in PlacesSyncUtils at https://dxr.mozilla.org/mozilla-central/rev/1a5b53a831e5a6c20de1b081c774feb3ff76756c/toolkit/components/places/PlacesSyncUtils.jsm#236 - can we close this, or is there more to do?
Flags: needinfo?(kcambridge)
Oops, yes, this can be closed. We'll do the right thing now.
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(kcambridge)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.