Bug 1854056 - Moving secondary action from MozAutocompleteLoginRichli…

…stitem to MozAutocompleteTwoLineRichlistitem r=mtigley,desktop-theme-reviewers,credential-management-reviewers,reusable-components-reviewers,dao,hjones Depends on D191190 Differential Revision: https://phabricator.services.mozilla.com/D191193
dothq · Oct 30, 2023 · d9e076e · d9e076e
1 parent 63b0a25
commit d9e076e
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 50 deletions.
diff --git a/browser/themes/shared/autocomplete.css b/browser/themes/shared/autocomplete.css
@@ -24,23 +24,6 @@
   width: 100%;
 }
 
-#PopupAutoComplete > richlistbox > richlistitem > .ac-login-item > .ac-settings-button {
-  visibility: hidden;
-  height: 16px;
-  width: 16px;
-  border: 0;
-  color: inherit;
-  -moz-context-properties: fill;
-  fill: currentColor;
-  margin-inline: 8px;
-  cursor: pointer;
-  background: url("chrome://global/skin/icons/settings.svg") center no-repeat;
-}
-
-#PopupAutoComplete > richlistbox > richlistitem:is(:hover, [selected]) > .ac-login-item > .ac-settings-button {
-  visibility: visible;
-}
-
 #PopupAutoComplete > richlistbox > richlistitem > .ac-title {
   font: icon;
   margin-inline-start: 4px;
@@ -77,6 +60,22 @@
   min-width: 0;
 }
 
+#PopupAutoComplete > richlistbox > richlistitem > .two-line-wrapper > .ac-secondary-action {
+  height: 16px;
+  width: 16px;
+  border: 0;
+  color: inherit;
+  -moz-context-properties: fill;
+  fill: currentColor;
+  margin-inline: 8px;
+  cursor: pointer;
+  background: url("chrome://global/skin/icons/settings.svg") center no-repeat;
+}
+
+#PopupAutoComplete > richlistbox > richlistitem:not(:hover, [selected]) > .two-line-wrapper > .ac-secondary-action {
+  visibility: collapse;
+}
+
 #PopupAutoComplete > richlistbox > richlistitem[originaltype="login"] > .ac-site-icon,
 #PopupAutoComplete > richlistbox > richlistitem[originaltype="possible-username"] > .ac-site-icon,
 #PopupAutoComplete > richlistbox > richlistitem > .two-line-wrapper > .ac-site-icon {

diff --git a/toolkit/components/passwordmgr/test/browser/browser.toml b/toolkit/components/passwordmgr/test/browser/browser.toml
@@ -216,7 +216,7 @@ skip-if = [
 ["browser_openPasswordManager.js"]
 
 ["browser_preselect_login.js"]
-fail-if = ["a11y_checks"] # Bug 1854452 clicked ac-settings-button may not be labeled
+fail-if = ["a11y_checks"] # Bug 1854452 clicked ac-secondary-action may not be labeled
 skip-if = [
   "os == 'linux' && (asan || tsan || debug)", # Bug 1840479
   "os == 'win' && (asan || debug)", # Bug 1840479

diff --git a/toolkit/components/passwordmgr/test/browser/browser_preselect_login.js b/toolkit/components/passwordmgr/test/browser/browser_preselect_login.js
@@ -99,15 +99,15 @@ add_task(
         await openACPopup(popup, browser, "#form-basic-username");
 
         const secondLoginItem = popup.firstChild.getItemAtIndex(1);
-        const secondLoginItemSettingsIcon = secondLoginItem.querySelector(
-          ".ac-settings-button"
+        const secondLoginItemSecondaryAction = secondLoginItem.querySelector(
+          ".ac-secondary-action"
         );
 
         Assert.ok(
-          !secondLoginItemSettingsIcon.checkVisibility({
+          !secondLoginItemSecondaryAction.checkVisibility({
             checkVisibilityCSS: true,
           }),
-          "Gear icon should not be visible initially"
+          "Secondary action should not be visible initially"
         );
 
         await EventUtils.synthesizeKey("KEY_ArrowDown");
@@ -119,10 +119,10 @@ add_task(
         );
 
         Assert.ok(
-          secondLoginItemSettingsIcon.checkVisibility({
+          secondLoginItemSecondaryAction.checkVisibility({
             checkVisibilityCSS: true,
           }),
-          "Gear icon should be visible when login item is active"
+          "Secondary action should be visible when item is active"
         );
 
         const aboutLoginsTabPromise = BrowserTestUtils.waitForNewTab(
@@ -131,7 +131,7 @@ add_task(
           true
         );
 
-        EventUtils.synthesizeMouseAtCenter(secondLoginItemSettingsIcon, {});
+        EventUtils.synthesizeMouseAtCenter(secondLoginItemSecondaryAction, {});
         const aboutLoginsTab = await aboutLoginsTabPromise;
 
         await SpecialPowers.spawn(

diff --git a/toolkit/content/widgets/autocomplete-richlistitem.js b/toolkit/content/widgets/autocomplete-richlistitem.js
@@ -634,9 +634,23 @@
       this.textContent = "";
       this.appendChild(this.constructor.fragment);
       this.initializeAttributeInheritance();
+      this.initializeSecondaryAction();
       this._adjustAcItem();
     }
 
+    initializeSecondaryAction() {
+      const button = this.querySelector(".ac-secondary-action");
+
+      if (this.onSecondaryAction) {
+        button.addEventListener("mousedown", event => {
+          event.stopPropagation();
+          this.onSecondaryAction();
+        });
+      } else {
+        button?.remove();
+      }
+    }
+
     static get inheritedAttributes() {
       return {
         // getLabelAt:
@@ -656,6 +670,7 @@
           <div class="label-row line1-label"></div>
           <div class="label-row line2-label"></div>
         </div>
+        <button class="ac-secondary-action"></button>
       </div>
     `;
     }
@@ -692,16 +707,14 @@
     connectedCallback() {
       super.connectedCallback();
 
-      this.querySelector(".ac-settings-button").addEventListener(
-        "mousedown",
-        event => {
-          event.stopPropagation();
-          const details = JSON.parse(this.getAttribute("ac-label"));
-          LoginHelper.openPasswordManager(window, {
-            loginGuid: details?.guid,
-          });
-        }
-      );
+      this.firstChild.classList.add("ac-login-item");
+    }
+
+    onSecondaryAction() {
+      const details = JSON.parse(this.getAttribute("ac-label"));
+      LoginHelper.openPasswordManager(window, {
+        loginGuid: details?.guid,
+      });
     }
 
     static get inheritedAttributes() {
@@ -712,21 +725,6 @@
       };
     }
 
-    static get markup() {
-      return `
-        <div xmlns="http://www.w3.org/1999/xhtml"
-             xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
-             class="two-line-wrapper ac-login-item">
-          <xul:image class="ac-site-icon"></xul:image>
-          <div class="labels-wrapper">
-            <div class="label-row line1-label"></div>
-            <div class="label-row line2-label"></div>
-          </div>
-          <button class="ac-settings-button"></button>
-        </div>
-      `;
-    }
-
     _adjustAcItem() {
       super._adjustAcItem();