Bug 1690567 - Hide some sync context menu items when not signed in. r=markh

Differential Revision: https://phabricator.services.mozilla.com/D104746

accessible/tests/browser/mac/browser_app.js

@@ -219,16 +219,16 @@ add_task(async () => {
 
       menu = await getMacAccessible(menu);
       let menuChildren = menu.getAttributeValue("AXChildren");
-      const expectedChildCount = 13 + +hasContainers + +hasInspectA11y;
+      const expectedChildCount = 12 + +hasContainers + +hasInspectA11y;
       is(
         menuChildren.length,
         expectedChildCount,
         `Context menu on link contains ${expectedChildCount} items.`
       );
-      // items at indicies 4, 10, and 12 are the splitters when containers exist
+      // items at indicies 3, 9, and 11 are the splitters when containers exist
       // everything else should be a menu item, otherwise indicies of splitters are
-      // 3, 9, and 11
+      // 3, 8, and 10
-      const splitterIndicies = hasContainers ? [4, 10, 12] : [3, 9, 11];
+      const splitterIndicies = hasContainers ? [4, 9, 11] : [3, 8, 10];
       for (let i = 0; i < menuChildren.length; i++) {
         if (splitterIndicies.includes(i)) {
           is(

browser/base/content/browser-sync.js

@@ -380,6 +380,20 @@ var gSync = {
     return UIState.get().status == UIState.STATUS_SIGNED_IN;
   },
 
+  shouldHideSendContextMenuItems(enabled) {
+    const state = UIState.get();
+    // Only show the "Send..." context menu items when sending would be possible
+    if (
+      enabled &&
+      state.status == UIState.STATUS_SIGNED_IN &&
+      state.syncEnabled &&
+      this.getSendTabTargets().length
+    ) {
+      return false;
+    }
+    return true;
+  },
+
   getSendTabTargets() {
     // If sync is not enabled, then there's no point looking for sync clients.
     // If sync is simply not ready or hasn't yet synced the clients engine, we
@@ -1566,27 +1580,33 @@ var gSync = {
       }
     }
     const enabled = !this.sendTabConfiguredAndLoading && hasASendableURI;
+    const hideItems = this.shouldHideSendContextMenuItems(enabled);
 
     let sendTabsToDevice = document.getElementById("context_sendTabToDevice");
     sendTabsToDevice.disabled = !enabled;
 
-    let tabCount = aTargetTab.multiselected
+    if (hideItems || !hasASendableURI) {
-      ? gBrowser.multiSelectedTabsCount
+      sendTabsToDevice.hidden = true;
-      : 1;
+    } else {
-    sendTabsToDevice.label = PluralForm.get(
+      let tabCount = aTargetTab.multiselected
-      tabCount,
+        ? gBrowser.multiSelectedTabsCount
-      gNavigatorBundle.getString("sendTabsToDevice.label")
+        : 1;
-    ).replace("#1", tabCount.toLocaleString());
+      sendTabsToDevice.label = PluralForm.get(
-    sendTabsToDevice.accessKey = gNavigatorBundle.getString(
+        tabCount,
-      "sendTabsToDevice.accesskey"
+        gNavigatorBundle.getString("sendTabsToDevice.label")
-    );
+      ).replace("#1", tabCount.toLocaleString());
+      sendTabsToDevice.accessKey = gNavigatorBundle.getString(
+        "sendTabsToDevice.accesskey"
+      );
+      sendTabsToDevice.hidden = false;
+    }
   },
 
   // "Send Page to Device" and "Send Link to Device" menu items
   updateContentContextMenu(contextMenu) {
     if (!this.FXA_ENABLED) {
       // These items are hidden by default. No need to do anything.
-      return;
+      return false;
     }
     // showSendLink and showSendPage are mutually exclusive
     const showSendLink =
@@ -1603,24 +1623,34 @@ var gSync = {
         contextMenu.onTextInput
       );
 
-    contextMenu.showItem("context-sendpagetodevice", showSendPage);
-    contextMenu.showItem("context-sendlinktodevice", showSendLink);
-
-    if (!showSendLink && !showSendPage) {
-      return;
-    }
-
     const targetURI = showSendLink
       ? contextMenu.getLinkURI()
       : contextMenu.browser.currentURI;
     const enabled =
       !this.sendTabConfiguredAndLoading &&
       BrowserUtils.isShareableURL(targetURI);
+    const hideItems = this.shouldHideSendContextMenuItems(enabled);
+
+    contextMenu.showItem(
+      "context-sendpagetodevice",
+      !hideItems && showSendPage
+    );
+    contextMenu.showItem(
+      "context-sendlinktodevice",
+      !hideItems && showSendLink
+    );
+
+    if (!showSendLink && !showSendPage) {
+      return false;
+    }
+
     contextMenu.setItemAttr(
       showSendPage ? "context-sendpagetodevice" : "context-sendlinktodevice",
       "disabled",
       !enabled || null
     );
+    // return true if context menu items are visible
+    return !hideItems && (showSendPage || showSendLink);
   },
 
   // Functions called by observers

browser/base/content/nsContextMenu.js

@@ -327,12 +327,12 @@ class nsContextMenu {
     this.initMiscItems();
     this.initSpellingItems();
     this.initSaveItems();
+    this.initSyncItems();
     this.initClipboardItems();
     this.initMediaPlayerItems();
     this.initLeaveDOMFullScreenItems();
     this.initClickToPlayItems();
     this.initPasswordManagerItems();
-    this.initSyncItems();
     this.initViewSourceItems();
     this.initScreenshotItem();
 
@@ -803,7 +803,7 @@ class nsContextMenu {
     // Other cases will show a divider.
     copyLinkSeparator.toggleAttribute(
       "ensureHidden",
-      this.onLink && !this.onMailtoLink && !this.onImage
+      this.onLink && !this.onMailtoLink && !this.onImage && this.syncItemsShown
     );
 
     this.showItem("context-copyvideourl", this.onVideo);
@@ -1016,7 +1016,7 @@ class nsContextMenu {
   }
 
   initSyncItems() {
-    gSync.updateContentContextMenu(this);
+    this.syncItemsShown = gSync.updateContentContextMenu(this);
   }
 
   initViewSourceItems() {

browser/base/content/test/contextMenu/browser_contextmenu.js

@@ -68,12 +68,6 @@ add_task(async function test_xul_text_link_label() {
     ...(hasPocket ? ["context-savelinktopocket", true] : []),
     "context-copylink",
     true,
-    "context-sendlinktodevice",
-    true,
-    // We need a blank entry here because the sendlinktodevice submenu is
-    // dynamically generated with no ids.
-    [],
-    null,
     "---",
     null,
     "context-searchselect",
@@ -152,12 +146,6 @@ add_task(async function test_plaintext() {
     ...(hasPocket ? ["context-pocket", true] : []),
     "---",
     null,
-    "context-sendpagetodevice",
-    true,
-    [],
-    null,
-    "---",
-    null,
     "context-selectall",
     true,
     "---",
@@ -193,10 +181,6 @@ add_task(async function test_link() {
     ...(hasPocket ? ["context-savelinktopocket", true] : []),
     "context-copylink",
     true,
-    "context-sendlinktodevice",
-    true,
-    [],
-    null,
     "---",
     null,
     "context-searchselect",
@@ -229,10 +213,6 @@ add_task(async function test_link_in_shadow_dom() {
       ...(hasPocket ? ["context-savelinktopocket", true] : []),
       "context-copylink",
       true,
-      "context-sendlinktodevice",
-      true,
-      [],
-      null,
       "---",
       null,
       "context-searchselect",
@@ -669,12 +649,6 @@ add_task(async function test_iframe() {
     ...(hasPocket ? ["context-pocket", true] : []),
     "---",
     null,
-    "context-sendpagetodevice",
-    true,
-    [],
-    null,
-    "---",
-    null,
     "context-selectall",
     true,
     "---",
@@ -1018,12 +992,6 @@ add_task(async function test_pdf_viewer_in_iframe() {
       ...(hasPocket ? ["context-pocket", true] : []),
       "---",
       null,
-      "context-sendpagetodevice",
-      true,
-      [],
-      null,
-      "---",
-      null,
       "context-selectall",
       true,
       "---",
@@ -1294,12 +1262,6 @@ add_task(async function test_pagemenu() {
       ...(hasPocket ? ["context-pocket", true] : []),
       "---",
       null,
-      "context-sendpagetodevice",
-      true,
-      [],
-      null,
-      "---",
-      null,
       "context-selectall",
       true,
       "---",
@@ -1363,12 +1325,6 @@ add_task(async function test_dom_full_screen() {
       ...(hasPocket ? ["context-pocket", true] : []),
       "---",
       null,
-      "context-sendpagetodevice",
-      true,
-      [],
-      null,
-      "---",
-      null,
       "context-selectall",
       true,
       "---",
@@ -1451,12 +1407,6 @@ add_task(async function test_pagemenu2() {
       ...(hasPocket ? ["context-pocket", true] : []),
       "---",
       null,
-      "context-sendpagetodevice",
-      true,
-      [],
-      null,
-      "---",
-      null,
       "context-selectall",
       true,
       "---",
@@ -1572,12 +1522,6 @@ add_task(async function test_select_text_link() {
       true,
       "---",
       null,
-      "context-sendlinktodevice",
-      true,
-      [],
-      null,
-      "---",
-      null,
       "context-searchselect",
       true,
       "context-searchselect-private",
@@ -1644,12 +1588,6 @@ add_task(async function test_imagelink() {
     null,
     "context-setDesktopBackground",
     true,
-    "---",
-    null,
-    "context-sendlinktodevice",
-    true,
-    [],
-    null,
   ]);
 });
 
@@ -1772,12 +1710,6 @@ add_task(async function test_srcdoc() {
     ...(hasPocket ? ["context-pocket", true] : []),
     "---",
     null,
-    "context-sendpagetodevice",
-    true,
-    [],
-    null,
-    "---",
-    null,
     "context-selectall",
     true,
     "---",
@@ -1849,10 +1781,6 @@ add_task(async function test_svg_link() {
     ...(hasPocket ? ["context-savelinktopocket", true] : []),
     "context-copylink",
     true,
-    "context-sendlinktodevice",
-    true,
-    [],
-    null,
     "---",
     null,
     "context-searchselect",
@@ -1881,10 +1809,6 @@ add_task(async function test_svg_link() {
     ...(hasPocket ? ["context-savelinktopocket", true] : []),
     "context-copylink",
     true,
-    "context-sendlinktodevice",
-    true,
-    [],
-    null,
     "---",
     null,
     "context-searchselect",
@@ -1913,10 +1837,6 @@ add_task(async function test_svg_link() {
     ...(hasPocket ? ["context-savelinktopocket", true] : []),
     "context-copylink",
     true,
-    "context-sendlinktodevice",
-    true,
-    [],
-    null,
     "---",
     null,
     "context-searchselect",
@@ -1947,10 +1867,6 @@ add_task(async function test_svg_relative_link() {
     ...(hasPocket ? ["context-savelinktopocket", true] : []),
     "context-copylink",
     true,
-    "context-sendlinktodevice",
-    true,
-    [],
-    null,
     "---",
     null,
     "context-searchselect",
@@ -1979,10 +1895,6 @@ add_task(async function test_svg_relative_link() {
     ...(hasPocket ? ["context-savelinktopocket", true] : []),
     "context-copylink",
     true,
-    "context-sendlinktodevice",
-    true,
-    [],
-    null,
     "---",
     null,
     "context-searchselect",
@@ -2011,10 +1923,6 @@ add_task(async function test_svg_relative_link() {
     ...(hasPocket ? ["context-savelinktopocket", true] : []),
     "context-copylink",
     true,
-    "context-sendlinktodevice",
-    true,
-    [],
-    null,
     "---",
     null,
     "context-searchselect",
@@ -2054,12 +1962,6 @@ add_task(async function test_background_image() {
     ...(hasPocket ? ["context-pocket", true] : []),
     "---",
     null,
-    "context-sendpagetodevice",
-    true,
-    [],
-    null,
-    "---",
-    null,
     "context-selectall",
     true,
     "---",
@@ -2092,10 +1994,6 @@ add_task(async function test_background_image() {
     true,
     "context-copylink",
     true,
-    "context-sendlinktodevice",
-    false,
-    [],
-    null,
     "---",
     null,
     "context-searchselect",

browser/base/content/test/contextMenu/browser_contextmenu_input.js

@@ -211,12 +211,6 @@ add_task(
           ...(hasPocket ? ["context-pocket", true] : []),
           "---",
           null,
-          "context-sendpagetodevice",
-          null,
-          [],
-          null,
-          "---",
-          null,
           "context-selectall",
           null,
           "---",

browser/base/content/test/sync/browser_contextmenu_sendpage.js

@@ -35,12 +35,12 @@ add_task(async function test_page_contextmenu() {
   is(
     document.getElementById("context-sendpagetodevice").hidden,
     false,
-    "Send tab to device is shown"
+    "Send page to device is shown"
   );
   is(
     document.getElementById("context-sendpagetodevice").disabled,
     false,
-    "Send tab to device is enabled"
+    "Send page to device is enabled"
   );
   checkPopup([
     { label: "Bar" },
@@ -103,23 +103,18 @@ add_task(async function test_link_contextmenu() {
 add_task(async function test_page_contextmenu_no_remote_clients() {
   const sandbox = setupSendTabMocks({ fxaDevices: [] });
 
-  await openContentContextMenu("#moztext", "context-sendpagetodevice");
+  await openContentContextMenu("#moztext");
   is(
     document.getElementById("context-sendpagetodevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send page to device is hidden"
   );
   is(
     document.getElementById("context-sendpagetodevice").disabled,
     false,
     "Send tab to device is enabled"
   );
-  checkPopup([
+  checkPopup();
-    { label: "No Devices Connected", disabled: true },
-    "----",
-    { label: "Connect Another Device..." },
-    { label: "Learn About Sending Tabs..." },
-  ]);
   await hideContentContextMenu();
 
   sandbox.restore();
@@ -142,12 +137,12 @@ add_task(async function test_page_contextmenu_one_remote_client() {
   is(
     document.getElementById("context-sendpagetodevice").hidden,
     false,
-    "Send tab to device is shown"
+    "Send page to device is shown"
   );
   is(
     document.getElementById("context-sendpagetodevice").disabled,
     false,
-    "Send tab to device is enabled"
+    "Send page to device is enabled"
   );
   checkPopup([{ label: "Foo" }]);
   await hideContentContextMenu();
@@ -161,13 +156,13 @@ add_task(async function test_page_contextmenu_not_sendable() {
   await openContentContextMenu("#moztext");
   is(
     document.getElementById("context-sendpagetodevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send page to device is hidden"
   );
   is(
     document.getElementById("context-sendpagetodevice").disabled,
     true,
-    "Send tab to device is disabled"
+    "Send page to device is disabled"
   );
   checkPopup();
   await hideContentContextMenu();
@@ -181,13 +176,13 @@ add_task(async function test_page_contextmenu_not_synced_yet() {
   await openContentContextMenu("#moztext");
   is(
     document.getElementById("context-sendpagetodevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send page to device is hidden"
   );
   is(
     document.getElementById("context-sendpagetodevice").disabled,
     true,
-    "Send tab to device is disabled"
+    "Send page to device is disabled"
   );
   checkPopup();
   await hideContentContextMenu();
@@ -201,13 +196,13 @@ add_task(async function test_page_contextmenu_sync_not_ready_configured() {
   await openContentContextMenu("#moztext");
   is(
     document.getElementById("context-sendpagetodevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send page to device is hidden"
   );
   is(
     document.getElementById("context-sendpagetodevice").disabled,
     true,
-    "Send tab to device is disabled"
+    "Send page to device is disabled"
   );
   checkPopup();
   await hideContentContextMenu();
@@ -221,22 +216,18 @@ add_task(async function test_page_contextmenu_sync_not_ready_other_state() {
     state: UIState.STATUS_NOT_VERIFIED,
   });
 
-  await openContentContextMenu("#moztext", "context-sendpagetodevice");
+  await openContentContextMenu("#moztext");
   is(
     document.getElementById("context-sendpagetodevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send page to device is hidden"
   );
   is(
     document.getElementById("context-sendpagetodevice").disabled,
     false,
-    "Send tab to device is enabled"
+    "Send page to device is enabled"
   );
-  checkPopup([
+  checkPopup();
-    { label: "Account Not Verified", disabled: true },
-    "----",
-    { label: "Verify Your Account..." },
-  ]);
   await hideContentContextMenu();
 
   sandbox.restore();
@@ -245,23 +236,18 @@ add_task(async function test_page_contextmenu_sync_not_ready_other_state() {
 add_task(async function test_page_contextmenu_unconfigured() {
   const sandbox = setupSendTabMocks({ state: UIState.STATUS_NOT_CONFIGURED });
 
-  await openContentContextMenu("#moztext", "context-sendpagetodevice");
+  await openContentContextMenu("#moztext");
   is(
     document.getElementById("context-sendpagetodevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send page to device is hidden"
   );
   is(
     document.getElementById("context-sendpagetodevice").disabled,
     false,
-    "Send tab to device is enabled"
+    "Send page to device is enabled"
   );
-  checkPopup([
+  checkPopup();
-    { label: "Not Signed In", disabled: true },
-    "----",
-    { label: "Sign in to Firefox..." },
-    { label: "Learn About Sending Tabs..." },
-  ]);
 
   await hideContentContextMenu();
 
@@ -271,22 +257,18 @@ add_task(async function test_page_contextmenu_unconfigured() {
 add_task(async function test_page_contextmenu_not_verified() {
   const sandbox = setupSendTabMocks({ state: UIState.STATUS_NOT_VERIFIED });
 
-  await openContentContextMenu("#moztext", "context-sendpagetodevice");
+  await openContentContextMenu("#moztext");
   is(
     document.getElementById("context-sendpagetodevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send page to device is hidden"
   );
   is(
     document.getElementById("context-sendpagetodevice").disabled,
     false,
-    "Send tab to device is enabled"
+    "Send page to device is enabled"
   );
-  checkPopup([
+  checkPopup();
-    { label: "Account Not Verified", disabled: true },
-    "----",
-    { label: "Verify Your Account..." },
-  ]);
 
   await hideContentContextMenu();
 
@@ -296,22 +278,18 @@ add_task(async function test_page_contextmenu_not_verified() {
 add_task(async function test_page_contextmenu_login_failed() {
   const sandbox = setupSendTabMocks({ state: UIState.STATUS_LOGIN_FAILED });
 
-  await openContentContextMenu("#moztext", "context-sendpagetodevice");
+  await openContentContextMenu("#moztext");
   is(
     document.getElementById("context-sendpagetodevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send page to device is hidden"
   );
   is(
     document.getElementById("context-sendpagetodevice").disabled,
     false,
-    "Send tab to device is enabled"
+    "Send page to device is enabled"
   );
-  checkPopup([
+  checkPopup();
-    { label: "Account Not Verified", disabled: true },
-    "----",
-    { label: "Verify Your Account..." },
-  ]);
 
   await hideContentContextMenu();
 
@@ -325,7 +303,7 @@ add_task(async function test_page_contextmenu_fxa_disabled() {
   is(
     document.getElementById("context-sendpagetodevice").hidden,
     true,
-    "Send tab to device is hidden"
+    "Send page to device is hidden"
   );
   await hideContentContextMenu();
   getter.restore();

browser/base/content/test/sync/browser_contextmenu_sendtab.js

@@ -111,8 +111,8 @@ add_task(async function test_tab_contextmenu_unconfigured() {
   updateTabContextMenu(testTab);
   is(
     document.getElementById("context_sendTabToDevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send tab to device is hidden"
   );
   is(
     document.getElementById("context_sendTabToDevice").disabled,
@@ -129,8 +129,8 @@ add_task(async function test_tab_contextmenu_not_sendable() {
   updateTabContextMenu(testTab);
   is(
     document.getElementById("context_sendTabToDevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send tab to device is hidden"
   );
   is(
     document.getElementById("context_sendTabToDevice").disabled,
@@ -147,8 +147,8 @@ add_task(async function test_tab_contextmenu_not_synced_yet() {
   updateTabContextMenu(testTab);
   is(
     document.getElementById("context_sendTabToDevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send tab to device is hidden"
   );
   is(
     document.getElementById("context_sendTabToDevice").disabled,
@@ -165,8 +165,8 @@ add_task(async function test_tab_contextmenu_sync_not_ready_configured() {
   updateTabContextMenu(testTab);
   is(
     document.getElementById("context_sendTabToDevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send tab to device is hidden"
   );
   is(
     document.getElementById("context_sendTabToDevice").disabled,
@@ -186,8 +186,8 @@ add_task(async function test_tab_contextmenu_sync_not_ready_other_state() {
   updateTabContextMenu(testTab);
   is(
     document.getElementById("context_sendTabToDevice").hidden,
-    false,
+    true,
-    "Send tab to device is shown"
+    "Send tab to device is hidden"
   );
   is(
     document.getElementById("context_sendTabToDevice").disabled,