Bug 1673402 - Don't show the bookmarks toolbar on the new tab page if there are no contents. r=Gijs
authorJared Wein <jwein@mozilla.com>
Tue, 27 Oct 2020 18:23:31 +0000
changeset 554768 ddfb6f0d4fc5a5334f348f3115b14c23c94c0b4b
parent 554767 9bc0aec298c2afa12122b7c562d62ef474802ee5
child 554769 a947b98efd10380133d3b0fab9db672743fd5d19
push id37898
push userabutkovits@mozilla.com
push dateWed, 28 Oct 2020 09:24:21 +0000
treeherdermozilla-central@83bf4fd3b1fb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1673402
milestone84.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1673402 - Don't show the bookmarks toolbar on the new tab page if there are no contents. r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D94775
browser/base/content/browser-places.js
browser/base/content/browser.js
browser/base/content/test/about/browser.ini
browser/base/content/test/about/browser_aboutNewTab_bookmarksToolbarEmpty.js
browser/base/content/test/about/head.js
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -1605,16 +1605,36 @@ var BookmarkingUI = {
       menuItem.dataset.bookmarksToolbarVisibility = true;
       menuItem.dataset.visibilityEnum = visibilityEnum;
       menuItem.addEventListener("command", onViewToolbarCommand);
     });
 
     return menu;
   },
 
+  bookmarksToolbarHasVisibleChildren() {
+    let bookmarksToolbarWidgets = CustomizableUI.getWidgetsInArea(
+      CustomizableUI.AREA_BOOKMARKS
+    );
+
+    const BOOKMARKS_TOOLBAR_ITEMS_ID = "personal-bookmarks";
+    if (
+      bookmarksToolbarWidgets.find(w => w.id == BOOKMARKS_TOOLBAR_ITEMS_ID) &&
+      PlacesUtils.getChildCountForFolder(PlacesUtils.bookmarks.toolbarGuid)
+    ) {
+      return true;
+    }
+
+    // The bookmarks items may not have any children, but if there are
+    // other widgets present then treat them as visible.
+    return bookmarksToolbarWidgets.some(
+      w => w.id != BOOKMARKS_TOOLBAR_ITEMS_ID
+    );
+  },
+
   attachPlacesView(event, node) {
     // If the view is already there, bail out early.
     if (node.parentNode._placesView) {
       return;
     }
 
     new PlacesMenu(event, `place:parent=${PlacesUtils.bookmarks.menuGuid}`, {
       extraClasses: {
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@@ -6616,16 +6616,19 @@ function setToolbarVisibility(
       case "never":
         isVisible = false;
         break;
       case "newtab":
         isVisible = BookmarkingUI.isOnNewTabPage({
           currentURI: gBrowser.currentURI,
           isNullPrincipal: gBrowser.contentPrincipal.isNullPrincipal,
         });
+        // If there is nothing visible in the toolbar, then don't show
+        // it on the New Tab page.
+        isVisible &&= BookmarkingUI.bookmarksToolbarHasVisibleChildren();
         break;
     }
   }
 
   if (toolbar.getAttribute(hidingAttribute) == (!isVisible).toString()) {
     // If this call will not result in a visibility change, return early
     // since dispatching toolbarvisibilitychange will cause views to get rebuilt.
     return;
--- a/browser/base/content/test/about/browser.ini
+++ b/browser/base/content/test/about/browser.ini
@@ -30,16 +30,17 @@ skip-if = os == "mac" || (os == "linux" 
 support-files =
   iframe_page_csp.html
   csp_iframe.sjs
 [browser_aboutNetError_xfo_iframe.js]
 support-files =
   iframe_page_xfo.html
   xfo_iframe.sjs
 [browser_aboutNewTab_bookmarksToolbar.js]
+[browser_aboutNewTab_bookmarksToolbarEmpty.js]
 [browser_aboutNewTab_bookmarksToolbarPrefs.js]
 [browser_aboutNewTab_defaultBrowserNotification.js]
 skip-if = debug || asan || ccov # Default browser checks are skipped on debug builds, bug 1660723
 [browser_aboutStopReload.js]
 [browser_aboutSupport.js]
 [browser_aboutSupport_newtab_security_state.js]
 [browser_bug435325.js]
 skip-if = verify && !debug && os == 'mac'
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/about/browser_aboutNewTab_bookmarksToolbarEmpty.js
@@ -0,0 +1,134 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const bookmarksInfo = [
+  {
+    title: "firefox",
+    url: "http://example.com",
+  },
+  {
+    title: "rules",
+    url: "http://example.com/2",
+  },
+  {
+    title: "yo",
+    url: "http://example.com/2",
+  },
+];
+
+add_task(async function setup() {
+  // Move all existing bookmarks in the Bookmarks Toolbar and
+  // Other Bookmarks to the Bookmarks Menu so they don't affect
+  // the visibility of the Bookmarks Toolbar. Restore them at
+  // the end of the test.
+  let Bookmarks = PlacesUtils.bookmarks;
+  let toolbarBookmarks = [];
+  let unfiledBookmarks = [];
+  let guidBookmarkTuples = [
+    [Bookmarks.toolbarGuid, toolbarBookmarks],
+    [Bookmarks.unfiledGuid, unfiledBookmarks],
+  ];
+  for (let [parentGuid, arr] of guidBookmarkTuples) {
+    await Bookmarks.fetch({ parentGuid }, bookmark => arr.push(bookmark));
+  }
+  await Promise.all(
+    [...toolbarBookmarks, ...unfiledBookmarks].map(async bookmark => {
+      bookmark.parentGuid = Bookmarks.menuGuid;
+      return Bookmarks.update(bookmark);
+    })
+  );
+  registerCleanupFunction(async () => {
+    for (let [parentGuid, arr] of guidBookmarkTuples) {
+      await Promise.all(
+        arr.map(async bookmark => {
+          bookmark.parentGuid = parentGuid;
+          return Bookmarks.update(bookmark);
+        })
+      );
+    }
+  });
+});
+
+add_task(async function bookmarks_toolbar_shown_on_newtab() {
+  for (let featureEnabled of [true, false]) {
+    info(
+      "Testing with the feature " + (featureEnabled ? "enabled" : "disabled")
+    );
+    await SpecialPowers.pushPrefEnv({
+      set: [["browser.toolbars.bookmarks.2h2020", featureEnabled]],
+    });
+    let bookmarks = await PlacesUtils.bookmarks.insertTree({
+      guid: PlacesUtils.bookmarks.toolbarGuid,
+      children: bookmarksInfo,
+    });
+    let example = await BrowserTestUtils.openNewForegroundTab({
+      gBrowser,
+      opening: "https://example.com",
+    });
+    let newtab = await BrowserTestUtils.openNewForegroundTab({
+      gBrowser,
+      opening: "about:newtab",
+      waitForLoad: false,
+    });
+
+    // 1: Test that the toolbar is shown in a newly opened foreground about:newtab
+    if (featureEnabled) {
+      await waitForBookmarksToolbarVisibility({
+        visible: true,
+        message: "Toolbar should be visible on newtab if enabled",
+      });
+    }
+
+    // 2: Toolbar should get hidden when switching tab to example.com
+    await BrowserTestUtils.switchTab(gBrowser, example);
+    await waitForBookmarksToolbarVisibility({
+      visible: false,
+      message: "Toolbar should be hidden on example.com",
+    });
+
+    // 3: Remove all children of the Bookmarks Toolbar and confirm that
+    // the toolbar should not become visible when switching to newtab
+    CustomizableUI.addWidgetToArea(
+      "personal-bookmarks",
+      CustomizableUI.AREA_TABSTRIP
+    );
+    CustomizableUI.removeWidgetFromArea("import-button");
+    await BrowserTestUtils.switchTab(gBrowser, newtab);
+    await waitForBookmarksToolbarVisibility({
+      visible: false,
+      message:
+        "Toolbar is not visible when there are no items in the toolbar area",
+    });
+
+    // 4: Put personal-bookmarks back in the toolbar and confirm the toolbar is visible now
+    CustomizableUI.addWidgetToArea(
+      "personal-bookmarks",
+      CustomizableUI.AREA_BOOKMARKS
+    );
+    await BrowserTestUtils.switchTab(gBrowser, example);
+    await BrowserTestUtils.switchTab(gBrowser, newtab);
+    if (featureEnabled) {
+      await waitForBookmarksToolbarVisibility({
+        visible: true,
+        message:
+          "Toolbar should be visible with Bookmarks Toolbar Items restored",
+      });
+    }
+
+    // 5: Remove all the bookmarks in the toolbar and confirm that the toolbar
+    // is hidden on the New Tab now
+    await PlacesUtils.bookmarks.remove(bookmarks);
+    await BrowserTestUtils.switchTab(gBrowser, example);
+    await BrowserTestUtils.switchTab(gBrowser, newtab);
+    await waitForBookmarksToolbarVisibility({
+      visible: false,
+      message:
+        "Toolbar is not visible when there are no items or nested bookmarks in the toolbar area",
+    });
+
+    await BrowserTestUtils.removeTab(newtab);
+    await BrowserTestUtils.removeTab(example);
+  }
+});
--- a/browser/base/content/test/about/head.js
+++ b/browser/base/content/test/about/head.js
@@ -236,18 +236,20 @@ async function promiseNewEngine(basename
   return engine;
 }
 
 async function waitForBookmarksToolbarVisibility({
   win = window,
   visible,
   message,
 }) {
-  return TestUtils.waitForCondition(() => {
+  let result = await TestUtils.waitForCondition(() => {
     let toolbar = win.gNavToolbox.querySelector("#PersonalToolbar");
     return visible ? !toolbar.collapsed : toolbar.collapsed;
   }, message || "waiting for toolbar to become " + (visible ? "visible" : "hidden"));
+  ok(result, message);
+  return result;
 }
 
 function isBookmarksToolbarVisible(win = window) {
   let toolbar = win.gNavToolbox.querySelector("#PersonalToolbar");
   return !toolbar.collapsed;
 }