Bug 1372927 - Show mobile bookmarks folder in places organizer for sync users even if they have no mobile bookmarks. r?markh draft
authorThom Chiovoloni <tchiovoloni@mozilla.com>
Tue, 08 Aug 2017 17:56:33 -0400
changeset 643512 9c0859a18eef2720f0be740ba9f8811c7da3f021
parent 642829 9a678b3efcd23bbe3a62b76c8113384ec7714cec
child 725319 c5326e49839347a173de3988076339f59c201fc8
push id73117
push userbmo:tchiovoloni@mozilla.com
push dateWed, 09 Aug 2017 20:32:17 +0000
reviewersmarkh
bugs1372927
milestone57.0a1
Bug 1372927 - Show mobile bookmarks folder in places organizer for sync users even if they have no mobile bookmarks. r?markh This commit also makes a change to test_ensureMobileQuery in test_sync_utils.js so that it actually runs (previously it had a typo in an import path that was imported in a try/catch). MozReview-Commit-ID: Kj8vqKpFi51
toolkit/components/places/PlacesSyncUtils.jsm
toolkit/components/places/tests/unit/test_sync_utils.js
--- a/toolkit/components/places/PlacesSyncUtils.jsm
+++ b/toolkit/components/places/PlacesSyncUtils.jsm
@@ -1027,49 +1027,33 @@ const BookmarkSyncUtils = PlacesSyncUtil
     let db = await PlacesUtils.promiseDBConnection();
 
     let maybeAllBookmarksGuids = await fetchGuidsWithAnno(db,
       ORGANIZER_QUERY_ANNO, ORGANIZER_ALL_BOOKMARKS_ANNO_VALUE);
     if (!maybeAllBookmarksGuids.length) {
       return;
     }
 
-    let hasMobileBookmarks = (await db.executeCached(`SELECT EXISTS(
-      SELECT 1 FROM moz_bookmarks b
-      JOIN moz_bookmarks p ON p.id = b.parent
-      WHERE p.guid = :mobileGuid
-    ) AS hasMobile`, {
-      mobileGuid: PlacesUtils.bookmarks.mobileGuid,
-    }))[0].getResultByName("hasMobile");
-
     let allBookmarksGuid = maybeAllBookmarksGuids[0];
     let mobileTitle = PlacesUtils.getString("MobileBookmarksFolderTitle");
 
     let maybeMobileQueryGuids = await fetchGuidsWithAnno(db,
       ORGANIZER_QUERY_ANNO, ORGANIZER_MOBILE_QUERY_ANNO_VALUE);
     if (maybeMobileQueryGuids.length) {
       let mobileQueryGuid = maybeMobileQueryGuids[0];
-      if (hasMobileBookmarks) {
-        // We have a left pane query for mobile bookmarks, and at least one
-        // mobile bookmark. Make sure the query title is correct.
-        await PlacesUtils.bookmarks.update({
-          guid: mobileQueryGuid,
-          url: "place:folder=MOBILE_BOOKMARKS",
-          title: mobileTitle,
-          source: SOURCE_SYNC,
-        });
-      } else {
-        // We have a left pane query for mobile bookmarks, but no mobile
-        // bookmarks. Remove the query.
-        await PlacesUtils.bookmarks.remove(mobileQueryGuid, {
-          source: SOURCE_SYNC,
-        });
-      }
-    } else if (hasMobileBookmarks) {
-      // We have mobile bookmarks, but no left pane query. Create the query.
+      // We have a left pane query for mobile bookmarks, make sure the
+      // query title is correct.
+      await PlacesUtils.bookmarks.update({
+        guid: mobileQueryGuid,
+        url: "place:folder=MOBILE_BOOKMARKS",
+        title: mobileTitle,
+        source: SOURCE_SYNC,
+      });
+    } else {
+      // We have no left pane query. Create the query.
       let mobileQuery = await PlacesUtils.bookmarks.insert({
         parentGuid: allBookmarksGuid,
         url: "place:folder=MOBILE_BOOKMARKS",
         title: mobileTitle,
         source: SOURCE_SYNC,
       });
 
       let mobileQueryId = await PlacesUtils.promiseItemId(mobileQuery.guid);
--- a/toolkit/components/places/tests/unit/test_sync_utils.js
+++ b/toolkit/components/places/tests/unit/test_sync_utils.js
@@ -2840,19 +2840,20 @@ add_task(async function test_migrateOldT
 
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesSyncUtils.bookmarks.reset();
 });
 
 add_task(async function test_ensureMobileQuery() {
   do_print("Ensure we correctly create the mobile query");
 
-  const PlacesUIUtils = {};
+  let PlacesUIUtils;
   try {
-    Cu.import("resource://gre/modules/PlacesUIUtils.jsm", PlacesUIUtils);
+    PlacesUIUtils = Cu.import("resource:///modules/PlacesUIUtils.jsm", {}).PlacesUIUtils;
+    PlacesUIUtils.maybeRebuildLeftPane();
   } catch (ex) {
     do_print("Can't build left pane roots; skipping test");
     return;
   }
 
   // Creates the organizer queries as a side effect.
   let leftPaneId = PlacesUIUtils.leftPaneFolderId;
   do_print(`Left pane root ID: ${leftPaneId}`);
@@ -2861,27 +2862,17 @@ add_task(async function test_ensureMobil
     "PlacesOrganizer/OrganizerQuery", "AllBookmarks");
   equal(allBookmarksGuids.length, 1, "Should create folder with all bookmarks queries");
   let allBookmarkGuid = allBookmarksGuids[0];
 
   do_print("Try creating query after organizer is ready");
   await PlacesSyncUtils.bookmarks.ensureMobileQuery()
   let queryGuids = await PlacesSyncUtils.bookmarks.fetchGuidsWithAnno(
     "PlacesOrganizer/OrganizerQuery", "MobileBookmarks");
-  equal(queryGuids.length, 0, "Should not create query without any mobile bookmarks");
-
-  do_print("Insert mobile bookmark, then create query");
-  let mozBmk = await PlacesUtils.bookmarks.insert({
-    parentGuid: PlacesUtils.bookmarks.mobileGuid,
-    url: "https://mozilla.org",
-  });
-  await PlacesSyncUtils.bookmarks.ensureMobileQuery()
-  queryGuids = await PlacesSyncUtils.bookmarks.fetchGuidsWithAnno(
-    "PlacesOrganizer/OrganizerQuery", "MobileBookmarks");
-  equal(queryGuids.length, 1, "Should create query once mobile bookmarks exist");
+  equal(queryGuids.length, 1, "Should create query without any mobile bookmarks");
 
   let queryGuid = queryGuids[0];
 
   let queryInfo = await PlacesUtils.bookmarks.fetch(queryGuid);
   equal(queryInfo.url, `place:folder=MOBILE_BOOKMARKS`, "Query should point to mobile root");
   equal(queryInfo.title, "Mobile Bookmarks", "Query title should be localized");
   equal(queryInfo.parentGuid, allBookmarkGuid, "Should append mobile query to all bookmarks queries");
 
@@ -2908,14 +2899,13 @@ add_task(async function test_ensureMobil
   await PlacesSyncUtils.bookmarks.ensureMobileQuery()
   queryInfo = await PlacesUtils.bookmarks.fetch(queryGuid);
   equal(queryInfo.url.href, `place:folder=MOBILE_BOOKMARKS`,
     "Should fix query URL to point to mobile root");
 
   do_print("We shouldn't track the query or the left pane root");
 
   let changes = await PlacesSyncUtils.bookmarks.pullChanges();
-  deepEqual(Object.keys(changes).sort(), [mozBmk.guid, "mobile"],
-    "Should not track mobile query");
+  ok(!(queryGuid in changes), "Should not track mobile query");
 
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesSyncUtils.bookmarks.reset();
 });