Bug 1415692 - Show the bookmark toolbar in new profiles with > 3 bookmarks on it. r=Gijs,mak

MozReview-Commit-ID: C3tmqIrt5ak

--HG--
extra : rebase_source : 936b8cab5de587e2dde708b94f8be597f9c83e6a

browser/base/content/test/performance/browser.ini

@@ -5,6 +5,9 @@
 # set during early startup to have an impact as a canvas will be used by
 # startupRecorder.js
 prefs =
+  # Skip migration work in BG__migrateUI for browser_startup.js since it isn't
+  # representative of common startup.
+  browser.migration.version=9999999
   browser.startup.record=true
   gfx.canvas.willReadFrequently.enable=true
 support-files =

browser/components/migration/tests/marionette/test_refresh_firefox.py

@@ -44,7 +44,7 @@ class TestFirefoxRefresh(MarionetteTestCase):
           Services.logins.addLogin(myLogin)
         """, script_args=(self._username, self._password))
 
-    def createBookmark(self):
+    def createBookmarkInMenu(self):
         self.marionette.execute_script("""
           let url = arguments[0];
           let title = arguments[1];
@@ -52,6 +52,14 @@ class TestFirefoxRefresh(MarionetteTestCase):
             makeURI(url), 0, title);
         """, script_args=(self._bookmarkURL, self._bookmarkText))
 
+    def createBookmarksOnToolbar(self):
+        self.marionette.execute_script("""
+          for (let i = 1; i <= 5; i++) {
+            PlacesUtils.bookmarks.insertBookmark(PlacesUtils.toolbarFolderId,
+              makeURI(`about:rights?p=${i}`), 0, `Bookmark ${i}`);
+          }
+        """)
+
     def createHistory(self):
         error = self.runAsyncCode("""
           // Copied from PlacesTestUtils, which isn't available in Marionette tests.
@@ -200,7 +208,7 @@ class TestFirefoxRefresh(MarionetteTestCase):
         # Note that we expect 2 logins - one from us, one from sync.
         self.assertEqual(loginCount, 2, "No other logins are present")
 
-    def checkBookmark(self):
+    def checkBookmarkInMenu(self):
         titleInBookmarks = self.marionette.execute_script("""
           let url = arguments[0];
           let bookmarkIds = PlacesUtils.bookmarks.getBookmarkIdsForURI(makeURI(url), {}, {});
@@ -208,6 +216,14 @@ class TestFirefoxRefresh(MarionetteTestCase):
         """, script_args=(self._bookmarkURL,))
         self.assertEqual(titleInBookmarks, self._bookmarkText)
 
+    def checkBookmarkToolbarVisibility(self):
+        toolbarVisible = self.marionette.execute_script("""
+          const BROWSER_DOCURL = "chrome://browser/content/browser.xul";
+          let xulStore = Cc["@mozilla.org/xul/xulstore;1"].getService(Ci.nsIXULStore);
+          return xulStore.getValue(BROWSER_DOCURL, "PersonalToolbar", "collapsed")
+        """)
+        self.assertEqual(toolbarVisible, "false")
+
     def checkHistory(self):
         historyResult = self.runAsyncCode("""
           PlacesUtils.history.fetch(arguments[0]).then(pageInfo => {
@@ -378,18 +394,20 @@ class TestFirefoxRefresh(MarionetteTestCase):
 
     def checkProfile(self, hasMigrated=False):
         self.checkPassword()
-        self.checkBookmark()
+        self.checkBookmarkInMenu()
         self.checkHistory()
         self.checkFormHistory()
         self.checkFormAutofill()
         self.checkCookie()
         self.checkSync(hasMigrated);
         if hasMigrated:
+            self.checkBookmarkToolbarVisibility()
             self.checkSession()
 
     def createProfileData(self):
         self.savePassword()
-        self.createBookmark()
+        self.createBookmarkInMenu()
+        self.createBookmarksOnToolbar()
         self.createHistory()
         self.createFormHistory()
         self.createFormAutofill()

browser/components/nsBrowserGlue.js

@@ -1746,6 +1746,35 @@ BrowserGlue.prototype = {
     this.AlertsService.showAlertNotification(null, title, body, true, null, clickCallback);
   },
 
+  /**
+   * Uncollapses PersonalToolbar if its collapsed status is not
+   * persisted, and user customized it or changed default bookmarks.
+   *
+   * If the user does not have a persisted value for the toolbar's
+   * "collapsed" attribute, try to determine whether it's customized.
+   */
+  _maybeToggleBookmarkToolbarVisibility() {
+    const BROWSER_DOCURL = "chrome://browser/content/browser.xul";
+    const NUM_TOOLBAR_BOOKMARKS_TO_UNHIDE = 3;
+    let xulStore = Cc["@mozilla.org/xul/xulstore;1"].getService(Ci.nsIXULStore);
+
+    if (!xulStore.hasValue(BROWSER_DOCURL, "PersonalToolbar", "collapsed")) {
+      // We consider the toolbar customized if it has more than NUM_TOOLBAR_BOOKMARKS_TO_UNHIDE
+      // children, or if it has a persisted currentset value.
+      let toolbarIsCustomized = xulStore.hasValue(BROWSER_DOCURL, "PersonalToolbar", "currentset");
+      let getToolbarFolderCount = () => {
+        let toolbarFolder = PlacesUtils.getFolderContents(PlacesUtils.toolbarFolderId).root;
+        let toolbarChildCount = toolbarFolder.childCount;
+        toolbarFolder.containerOpen = false;
+        return toolbarChildCount;
+      };
+
+      if (toolbarIsCustomized || getToolbarFolderCount() > NUM_TOOLBAR_BOOKMARKS_TO_UNHIDE) {
+        xulStore.setValue(BROWSER_DOCURL, "PersonalToolbar", "collapsed", "false");
+      }
+    }
+  },
+
   // eslint-disable-next-line complexity
   _migrateUI: function BG__migrateUI() {
     const UI_VERSION = 59;
@@ -1757,6 +1786,15 @@ BrowserGlue.prototype = {
     } else {
       // This is a new profile, nothing to migrate.
       Services.prefs.setIntPref("browser.migration.version", UI_VERSION);
+
+      try {
+        // New profiles may have existing bookmarks (imported from another browser or
+        // copied into the profile) and we want to show the bookmark toolbar for them
+        // in some cases.
+        this._maybeToggleBookmarkToolbarVisibility();
+      } catch (ex) {
+        Cu.reportError(ex);
+      }
       return;
     }
 

browser/components/tests/browser/browser.ini

@@ -4,3 +4,4 @@
 skip-if = !updater
 reason = test depends on update channel
 [browser_contentpermissionprompt.js]
+[browser_default_bookmark_toolbar_visibility.js]

browser/components/tests/browser/browser_default_bookmark_toolbar_visibility.js

@@ -0,0 +1,18 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Test _maybeToggleBookmarkToolbarVisibility() code running for new profiles.
+ * Ensure that the bookmarks toolbar is hidden in a default configuration.
+ * If new default bookmarks are added to the toolbar then the threshold of > 3
+ * in NUM_TOOLBAR_BOOKMARKS_TO_UNHIDE may need to be adjusted there.
+ */
+add_task(async function test_default_bookmark_toolbar_visibility() {
+  const BROWSER_DOCURL = "chrome://browser/content/browser.xul";
+  let xulStore = Cc["@mozilla.org/xul/xulstore;1"].getService(Ci.nsIXULStore);
+
+  is(xulStore.getValue(BROWSER_DOCURL, "PersonalToolbar", "collapsed"), "",
+     "Check that @collapsed isn't persisted");
+  ok(document.getElementById("PersonalToolbar").collapsed,
+     "The bookmarks toolbar should be collapsed by default");
+});

dom/events/test/mochitest.ini

@@ -1,4 +1,7 @@
 [DEFAULT]
+# Skip migration work in BG__migrateUI for browser_startup.js since it increases
+# the occurrence of the leak reported in bug 1398563 with test_bug1327798.html.
+prefs = browser.migration.version=9999999
 skip-if = toolkit == 'android' #CRASH_DUMP, RANDOM
 support-files =
   bug226361_iframe.xhtml