Bug 1397545 - Don't update the recently bookmarks item list when it is hidden and a bookmark is deleted. r=mak
authorMark Banner <standard8@mozilla.com>
Thu, 07 Sep 2017 10:49:03 +0100
changeset 379467 3b1def512f64
parent 379466 7e0c0b8a5c13
child 379468 8da12747279b
push id50646
push usermbanner@mozilla.com
push dateThu, 07 Sep 2017 11:17:11 +0000
treeherderautoland@3b1def512f64 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1397545
milestone57.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 1397545 - Don't update the recently bookmarks item list when it is hidden and a bookmark is deleted. r=mak MozReview-Commit-ID: CnbFMONaRil
browser/base/content/browser-places.js
browser/components/places/tests/chrome/test_RecentBookmarksMenuUI.xul
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -1487,22 +1487,23 @@ var RecentBookmarksMenuUI = {
   /**
    * nsINavBookmarkObserver methods.
    */
 
   /*
    * Handles onItemRemoved notifications from the bookmarks service.
    */
   onItemRemoved(itemId, parentId, index, itemType, uri, guid) {
+    if (!this.visible) {
+      return;
+    }
     // Update the menu when a bookmark has been removed.
     // The native menubar on Mac doesn't support live update, so this is
     // unlikely to be called there.
-    if (this._recentGuids.size == 0 ||
-        (guid && this._recentGuids.has(guid))) {
-
+    if (guid && this._recentGuids.has(guid)) {
       if (this._itemRemovedTimer) {
         clearTimeout(this._itemRemovedTimer);
       }
 
       this._itemRemovedTimer = setTimeout(() => {
         this._clearExistingItems();
         this._insertRecentMenuItems();
       }, this.ITEM_REMOVED_TIMEOUT);
--- a/browser/components/places/tests/chrome/test_RecentBookmarksMenuUI.xul
+++ b/browser/components/places/tests/chrome/test_RecentBookmarksMenuUI.xul
@@ -230,16 +230,47 @@
       is(document.getElementById("fakeRecentBookmarks").hidden, true,
          "The title item should be hidden");
       is(document.getElementById("fakeNextSeparator").hidden, false,
          "Next separator should not be hidden");
 
       simulateHideMenu();
     });
 
+    add_task(async function test_remove_with_recently_bookmarked_hidden() {
+      RecentBookmarksMenuUI.init(document.getElementById("fakeRecentBookmarks"));
+      RecentBookmarksMenuUI.visible = false;
+
+      is(bmMenu.children.length, 3,
+         "There should only be the original 3 items in the menu");
+
+      let bmToRemove = await PlacesUtils.bookmarks.fetch({url: `${BASE_URL}bookmark_dummy_6.html`});
+
+      sandbox.stub(RecentBookmarksMenuUI, "_clearExistingItems");
+      sandbox.stub(RecentBookmarksMenuUI, "_insertRecentMenuItems");
+
+      const clock = sandbox.useFakeTimers();
+
+      await PlacesUtils.bookmarks.remove(bmToRemove);
+
+      // Move the clock past the timeout to ensure any update happen.
+      clock.tick(RecentBookmarksMenuUI.ITEM_REMOVED_TIMEOUT + 1);
+      clock.restore();
+
+      is(bmMenu.children.length, 3,
+         "There should only be the original 3 items in the menu");
+      is(RecentBookmarksMenuUI._clearExistingItems.notCalled, true,
+         "Should not have cleared the existing items when recently bookmarked are hidden.");
+      is(RecentBookmarksMenuUI._insertRecentMenuItems.notCalled, true,
+         "Should not have inserted new menu items when recently bookmarked are hidden.");
+
+      sandbox.restore();
+      simulateHideMenu();
+    });
+
     add_task(async function test_show_recently_bookmarked() {
       RecentBookmarksMenuUI.init(document.getElementById("fakeRecentBookmarks"));
 
       is(bmMenu.children.length, 3,
          "There should only be the original 3 items in the menu");
 
       RecentBookmarksMenuUI.visible = true;