Bug 1415692 - Show the bookmark toolbar in new profiles with > 3 bookmarks on it. r=mak,Gijs
MozReview-Commit-ID: C3tmqIrt5ak
--- a/browser/base/content/test/performance/browser.ini
+++ b/browser/base/content/test/performance/browser.ini
@@ -1,15 +1,18 @@
[DEFAULT]
# to avoid overhead when running the browser normally, startupRecorder.js will
# do almost nothing unless browser.startup.record is true.
# gfx.canvas.willReadFrequently.enable is just an optimization, but needs to be
# 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 =
head.js
[browser_appmenu_reflows.js]
skip-if = asan || debug # Bug 1382809, bug 1369959
[browser_favicon_load.js]
[browser_startup.js]
--- a/browser/components/migration/tests/marionette/test_refresh_firefox.py
+++ b/browser/components/migration/tests/marionette/test_refresh_firefox.py
@@ -39,24 +39,32 @@ class TestFirefoxRefresh(MarionetteTestC
arguments[0],
arguments[1],
"username",
"password"
);
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];
PlacesUtils.bookmarks.insertBookmark(PlacesUtils.bookmarks.bookmarksMenuFolder,
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.
let didReturn;
PlacesUtils.asyncHistory.updatePlaces(
[{title: arguments[1], uri: makeURI(arguments[0]), visits: [{
transitionType: Ci.nsINavHistoryService.TRANSITION_LINK,
visitDate: (Date.now() - 5000) * 1000,
@@ -195,24 +203,32 @@ class TestFirefoxRefresh(MarionetteTestC
self.assertEqual(loginInfo[0]['password'], self._password)
loginCount = self.marionette.execute_script("""
return Services.logins.getAllLogins().length;
""")
# 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), {}, {});
return bookmarkIds.length == 1 ? PlacesUtils.bookmarks.getItemTitle(bookmarkIds[0]) : "";
""", 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 => {
if (!pageInfo) {
marionetteScriptFinished("No visits found");
} else {
marionetteScriptFinished(pageInfo);
}
@@ -373,28 +389,30 @@ class TestFirefoxRefresh(MarionetteTestC
self.assertEqual(result["accountData"]["keyFetchToken"], "top-secret");
if hasMigrated:
# This test doesn't actually configure sync itself, so the username
# pref only exists after migration.
self.assertEqual(result["prefUsername"], "test@test.com");
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()
self.createCookie()
self.createSession()
self.createSync()
def setUpScriptData(self):
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@@ -1741,27 +1741,65 @@ BrowserGlue.prototype = {
let clickCallback = (subject, topic, data) => {
if (topic != "alertclickcallback")
return;
this._openPreferences("sync", { origin: "doorhanger" });
};
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;
const BROWSER_DOCURL = "chrome://browser/content/browser.xul";
let currentUIVersion;
if (Services.prefs.prefHasUserValue("browser.migration.version")) {
currentUIVersion = Services.prefs.getIntPref("browser.migration.version");
} 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;
}
if (currentUIVersion >= UI_VERSION)
return;
let xulStore = Cc["@mozilla.org/xul/xulstore;1"].getService(Ci.nsIXULStore);
--- a/browser/components/tests/browser/browser.ini
+++ b/browser/components/tests/browser/browser.ini
@@ -1,6 +1,7 @@
[DEFAULT]
[browser_bug538331.js]
skip-if = !updater
reason = test depends on update channel
[browser_contentpermissionprompt.js]
+[browser_default_bookmark_toolbar_visibility.js]
new file mode 100644
--- /dev/null
+++ b/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");
+});
--- a/dom/events/test/mochitest.ini
+++ b/dom/events/test/mochitest.ini
@@ -1,9 +1,12 @@
[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
bug299673.js
bug322588-popup.html
bug426082.html
bug545268.html
bug574663.html