From 4c27f9fc7d6520aeab69d6cdf6f185c16d5c2b50 Mon Sep 17 00:00:00 2001
From: Jared Wein <jwein@mozilla.com>
Date: Fri, 7 Feb 2020 21:02:09 +0000
Subject: [PATCH] Bug 1613731 - Move the 'fill login' menuitem up to the top of
 the context menu. r=MattN

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

--HG--
extra : moz-landing-system : lando
---
 browser/base/content/browser-context.inc      | 45 +++++++++----------
 browser/base/content/nsContextMenu.js         |  6 ++-
 .../contextMenu/browser_contextmenu_input.js  | 26 +++++------
 ...browser_context_menu_generated_password.js | 22 +++++----
 .../passwordmgr/test/browser/head.js          |  2 +-
 5 files changed, 54 insertions(+), 47 deletions(-)

diff --git a/browser/base/content/browser-context.inc b/browser/base/content/browser-context.inc
index 98790f82638d..bf762797dc04 100644
--- a/browser/base/content/browser-context.inc
+++ b/browser/base/content/browser-context.inc
@@ -233,11 +233,32 @@
                 data-l10n-id="main-context-menu-view-background-image"
                 oncommand="gContextMenu.viewBGImage(event);"
                 onclick="checkForMiddleClick(this, event);"/>
+      <menu id="fill-login"
+            label="&fillLoginMenu.label;"
+            label-login="&fillLoginMenu.label;"
+            label-password="&fillPasswordMenu.label;"
+            label-username="&fillUsernameMenu.label;"
+            accesskey="&fillLoginMenu.accesskey;"
+            accesskey-login="&fillLoginMenu.accesskey;"
+            accesskey-password="&fillPasswordMenu.accesskey;"
+            accesskey-username="&fillUsernameMenu.accesskey;"
+            hidden="true">
+        <menupopup id="fill-login-popup">
+          <menuitem id="fill-login-no-logins"
+                    label="&noLoginSuggestions.label;"
+                    disabled="true"
+                    hidden="true"/>
+          <menuseparator id="saved-logins-separator"/>
+          <menuitem id="fill-login-saved-passwords"
+                    label="&viewSavedLogins.label;"
+                    oncommand="gContextMenu.openPasswordManager();"/>
+        </menupopup>
+      </menu>
      <menuitem id="fill-login-generated-password"
                data-l10n-id="main-context-menu-generate-new-password"
                hidden="true"
                oncommand="gContextMenu.useGeneratedPassword();"/>
-      <menuseparator id="generated-password-separator"/>
+      <menuseparator id="fill-login-and-generated-password-separator"/>
       <menuitem id="context-undo"
                 data-l10n-id="text-action-undo"
                 command="cmd_undo"/>
@@ -354,28 +375,6 @@
       <menuitem hidden="true" id="context-bidi-page-direction-toggle"
                 data-l10n-id="main-context-menu-bidi-switch-page"
                 oncommand="gContextMenu.switchPageDirection();"/>
-      <menuseparator id="fill-login-separator" hidden="true"/>
-      <menu id="fill-login"
-            label="&fillLoginMenu.label;"
-            label-login="&fillLoginMenu.label;"
-            label-password="&fillPasswordMenu.label;"
-            label-username="&fillUsernameMenu.label;"
-            accesskey="&fillLoginMenu.accesskey;"
-            accesskey-login="&fillLoginMenu.accesskey;"
-            accesskey-password="&fillPasswordMenu.accesskey;"
-            accesskey-username="&fillUsernameMenu.accesskey;"
-            hidden="true">
-        <menupopup id="fill-login-popup">
-          <menuitem id="fill-login-no-logins"
-                    label="&noLoginSuggestions.label;"
-                    disabled="true"
-                    hidden="true"/>
-          <menuseparator id="saved-logins-separator"/>
-          <menuitem id="fill-login-saved-passwords"
-                    label="&viewSavedLogins.label;"
-                    oncommand="gContextMenu.openPasswordManager();"/>
-        </menupopup>
-      </menu>
       <menuseparator id="inspect-separator" hidden="true"/>
       <menuitem id="context-inspect"
                 hidden="true"
diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js
index 523a5f35435e..1ce736c64f44 100644
--- a/browser/base/content/nsContextMenu.js
+++ b/browser/base/content/nsContextMenu.js
@@ -984,10 +984,12 @@ class nsContextMenu {
       let insertBeforeElement = document.getElementById("fill-login-no-logins");
       popup.insertBefore(fragment, insertBeforeElement);
     } finally {
-      this.showItem("fill-login-separator", showFill);
       this.showItem("fill-login", showFill);
       this.showItem("fill-login-generated-password", showGenerate);
-      this.showItem("generated-password-separator", showGenerate);
+      this.showItem(
+        "fill-login-and-generated-password-separator",
+        showFill || showGenerate
+      );
     }
   }
 
diff --git a/browser/base/content/test/contextMenu/browser_contextmenu_input.js b/browser/base/content/test/contextMenu/browser_contextmenu_input.js
index 05fefaf85b4f..543bda10af6e 100644
--- a/browser/base/content/test/contextMenu/browser_contextmenu_input.js
+++ b/browser/base/content/test/contextMenu/browser_contextmenu_input.js
@@ -229,6 +229,19 @@ add_task(async function test_password_input() {
   await test_contextmenu(
     "#input_password",
     [
+      "fill-login",
+      null,
+      [
+        "fill-login-no-logins",
+        false,
+        "---",
+        null,
+        "fill-login-saved-passwords",
+        true,
+      ],
+      null,
+      "---",
+      null,
       "context-undo",
       false,
       "---",
@@ -245,19 +258,6 @@ add_task(async function test_password_input() {
       null,
       "context-selectall",
       null,
-      "---",
-      null,
-      "fill-login",
-      null,
-      [
-        "fill-login-no-logins",
-        false,
-        "---",
-        null,
-        "fill-login-saved-passwords",
-        true,
-      ],
-      null,
     ],
     {
       skipFocusChange: true,
diff --git a/toolkit/components/passwordmgr/test/browser/browser_context_menu_generated_password.js b/toolkit/components/passwordmgr/test/browser/browser_context_menu_generated_password.js
index 106c3d40ed09..94ed9cb41d91 100644
--- a/toolkit/components/passwordmgr/test/browser/browser_context_menu_generated_password.js
+++ b/toolkit/components/passwordmgr/test/browser/browser_context_menu_generated_password.js
@@ -52,17 +52,20 @@ add_task(async function test_hidden_by_prefs() {
       let generatedPasswordItem = document.getElementById(
         "fill-login-generated-password"
       );
+      let fillLoginItem = document.getElementById("fill-login");
       let generatedPasswordSeparator = document.getElementById(
-        "generated-password-separator"
+        "fill-login-and-generated-password-separator"
       );
 
       ok(
         !BrowserTestUtils.is_visible(generatedPasswordItem),
         "generated password item is hidden"
       );
-      ok(
-        !BrowserTestUtils.is_visible(generatedPasswordSeparator),
-        "separator is hidden"
+      is(
+        BrowserTestUtils.is_visible(fillLoginItem) ||
+          BrowserTestUtils.is_visible(generatedPasswordItem),
+        BrowserTestUtils.is_visible(generatedPasswordSeparator),
+        "separator should only be visible if one of the login items is visible"
       );
 
       CONTEXT_MENU.hidePopup();
@@ -89,17 +92,20 @@ add_task(async function test_fill_hidden_by_login_saving_disabled() {
       let generatedPasswordItem = document.getElementById(
         "fill-login-generated-password"
       );
+      let fillLoginItem = document.getElementById("fill-login");
       let generatedPasswordSeparator = document.getElementById(
-        "generated-password-separator"
+        "fill-login-and-generated-password-separator"
       );
 
       ok(
         !BrowserTestUtils.is_visible(generatedPasswordItem),
         "generated password item is hidden"
       );
-      ok(
-        !BrowserTestUtils.is_visible(generatedPasswordSeparator),
-        "separator is hidden"
+      is(
+        BrowserTestUtils.is_visible(fillLoginItem) ||
+          BrowserTestUtils.is_visible(generatedPasswordItem),
+        BrowserTestUtils.is_visible(generatedPasswordSeparator),
+        "separator should only be visible if one of the login items is visible"
       );
 
       CONTEXT_MENU.hidePopup();
diff --git a/toolkit/components/passwordmgr/test/browser/head.js b/toolkit/components/passwordmgr/test/browser/head.js
index 8b2df6c7bbbf..3131a1eb2278 100644
--- a/toolkit/components/passwordmgr/test/browser/head.js
+++ b/toolkit/components/passwordmgr/test/browser/head.js
@@ -689,7 +689,7 @@ async function doFillGeneratedPasswordContextMenuItem(browser, passwordInput) {
     "fill-login-generated-password"
   );
   let generatedPasswordSeparator = document.getElementById(
-    "generated-password-separator"
+    "fill-login-and-generated-password-separator"
   );
 
   ok(