Bug 1913976 - mobileBookmarks folder is missing in Bookmarks menu. a=…

…pascalc Original Revision: https://phabricator.services.mozilla.com/D220383 Differential Revision: https://phabricator.services.mozilla.com/D220438
i3roly · Aug 29, 2024 · f0d9363 · f0d9363
1 parent cd60239
commit f0d9363
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 15 deletions.
diff --git a/browser/base/content/browser-places.js b/browser/base/content/browser-places.js
@@ -25,6 +25,15 @@ XPCOMUtils.defineLazyPreferenceGetter(
     }, console.error);
   }
 );
+
+// Set by sync after syncing bookmarks successfully once.
+XPCOMUtils.defineLazyPreferenceGetter(
+  this,
+  "SHOW_MOBILE_BOOKMARKS",
+  "browser.bookmarks.showMobileBookmarks",
+  false
+);
+
 ChromeUtils.defineESModuleGetters(this, {
   PanelMultiView: "resource:///modules/PanelMultiView.sys.mjs",
   RecentlyClosedTabsAndWindowsMenuUtils:
@@ -1343,7 +1352,7 @@ var BookmarkingUI = {
 
   onPopupShowing: function BUI_onPopupShowing(event) {
     // Don't handle events for submenus.
-    if (event.target != event.currentTarget) {
+    if (event.target.id != "BMB_bookmarksPopup") {
       return;
     }
 
@@ -1374,7 +1383,8 @@ var BookmarkingUI = {
       return;
     }
 
-    this._initMobileBookmarks(document.getElementById("BMB_mobileBookmarks"));
+    document.getElementById("BMB_mobileBookmarks").hidden =
+      !SHOW_MOBILE_BOOKMARKS;
 
     this.updateLabel(
       "BMB_viewBookmarksSidebar",
@@ -1582,17 +1592,6 @@ var BookmarkingUI = {
     }
   },
 
-  // Set by sync after syncing bookmarks successfully once.
-  MOBILE_BOOKMARKS_PREF: "browser.bookmarks.showMobileBookmarks",
-
-  _shouldShowMobileBookmarks() {
-    return Services.prefs.getBoolPref(this.MOBILE_BOOKMARKS_PREF, false);
-  },
-
-  _initMobileBookmarks(mobileMenuItem) {
-    mobileMenuItem.hidden = !this._shouldShowMobileBookmarks();
-  },
-
   _uninitView: function BUI__uninitView() {
     // When an element with a placesView attached is removed and re-inserted,
     // XBL reapplies the binding causing any kind of issues and possible leaks,
@@ -1938,11 +1937,12 @@ var BookmarkingUI = {
 
   onMainMenuPopupShowing: function BUI_onMainMenuPopupShowing(event) {
     // Don't handle events for submenus.
-    if (event.target != event.currentTarget) {
+    if (event.target.id != "bookmarksMenuPopup") {
       return;
     }
 
-    this._initMobileBookmarks(document.getElementById("menu_mobileBookmarks"));
+    document.getElementById("menu_mobileBookmarks").hidden =
+      !SHOW_MOBILE_BOOKMARKS;
   },
 
   showSubView(anchor) {

diff --git a/browser/components/places/tests/browser/browser.toml b/browser/components/places/tests/browser/browser.toml
@@ -189,6 +189,8 @@ skip-if = ["verify && debug && os == 'win'"]
 
 ["browser_markPageAsFollowedLink.js"]
 
+["browser_mobile_root_on_bookmark_menus.js"]
+
 ["browser_panelview_bookmarks_delete.js"]
 
 ["browser_paste_bookmarks.js"]

diff --git a/browser/components/places/tests/browser/browser_mobile_root_on_bookmark_menus.js b/browser/components/places/tests/browser/browser_mobile_root_on_bookmark_menus.js
@@ -0,0 +1,58 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+let { PlacesSyncUtils } = ChromeUtils.importESModule(
+  "resource://gre/modules/PlacesSyncUtils.sys.mjs"
+);
+
+add_setup(async function () {
+  // There should be at least one bookmark in the mobile root.
+  await PlacesUtils.bookmarks.insert({
+    url: "https://www.mozilla.org/",
+    parentGuid: PlacesUtils.bookmarks.mobileGuid,
+  });
+  await PlacesSyncUtils.bookmarks.ensureMobileQuery();
+  CustomizableUI.addWidgetToArea(
+    "bookmarks-menu-button",
+    CustomizableUI.AREA_NAVBAR,
+    4
+  );
+
+  registerCleanupFunction(async function () {
+    CustomizableUI.reset();
+    await PlacesUtils.bookmarks.eraseEverything();
+  });
+});
+
+add_task(async function test() {
+  for (let { popup, mobileMenuitem } of [
+    {
+      popup: document.getElementById("BMB_bookmarksPopup"),
+      mobileMenuitem: document.getElementById("BMB_mobileBookmarks"),
+    },
+    {
+      popup: document.getElementById("bookmarksMenuPopup"),
+      mobileMenuitem: document.getElementById("menu_mobileBookmarks"),
+    },
+  ]) {
+    info("Open bookmarks popup.");
+    let shownPromise = BrowserTestUtils.waitForEvent(
+      popup,
+      "popupshowing",
+      false,
+      e => e.target == popup
+    );
+    // Simulates popup opening causing it to populate. We cannot just open
+    // normally since it would not work on Mac native menubar.
+    popup.dispatchEvent(
+      new MouseEvent("popupshowing", {
+        bubbles: true,
+      })
+    );
+    await shownPromise;
+
+    Assert.ok(!mobileMenuitem.hidden, "Check mobile root is not hidden.");
+  }
+});