Bug 1397790 - add 'open' active state to urlbar buttons, use a more distinct background colour for it, r?adw draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 19 Sep 2017 15:13:25 +0100
changeset 667194 7c457c92fd5a5403ab587b010382d8e48036ab83
parent 666884 e4261f5b96ebfd63e7cb8af3035ff9fea90c74a5
child 732324 ae1fdb534e5a82f059f750b6a06adc47d8499f30
push id80645
push usergijskruitbosch@gmail.com
push dateTue, 19 Sep 2017 20:34:55 +0000
reviewersadw
bugs1397790
milestone57.0a1
Bug 1397790 - add 'open' active state to urlbar buttons, use a more distinct background colour for it, r?adw MozReview-Commit-ID: 7QrhAB79yNw
browser/base/content/browser-pageActions.js
browser/base/content/browser-places.js
browser/base/content/test/urlbar/browser_page_action_menu.js
browser/themes/shared/urlbar-searchbar.inc.css
--- a/browser/base/content/browser-pageActions.js
+++ b/browser/base/content/browser-pageActions.js
@@ -236,27 +236,29 @@ var BrowserPageActions = {
       }
       panelNode.remove();
     }, { once: true });
 
     panelNode.addEventListener("popuphiding", () => {
       if (iframeNode) {
         action.onIframeHiding(iframeNode, panelNode);
       }
+      anchorNode.removeAttribute("open");
     }, { once: true });
 
     if (panelViewNode) {
       action.subview.onPlaced(panelViewNode);
       action.subview.onShowing(panelViewNode);
     }
 
     // Hide the main page action panel before showing the activated-action
     // panel.
     this.panelNode.hidePopup();
     panelNode.openPopup(anchorNode, "bottomcenter topright");
+    anchorNode.setAttribute("open", "true");
 
     if (iframeNode) {
       action.onIframeShown(iframeNode, panelNode);
     }
 
     return panelNode;
   },
 
@@ -588,16 +590,20 @@ var BrowserPageActions = {
   showPanel(event = null) {
     for (let action of PageActions.actions) {
       let buttonNodeID = this._panelButtonNodeIDForActionID(action.id);
       let buttonNode = document.getElementById(buttonNodeID);
       action.onShowingInPanel(buttonNode);
     }
 
     this.panelNode.hidden = false;
+    this.panelNode.addEventListener("popuphiding", () => {
+      this.mainButtonNode.removeAttribute("open");
+    }, {once: true});
+    this.mainButtonNode.setAttribute("open", "true");
     this.panelNode.openPopup(this.mainButtonNode, {
       position: "bottomcenter topright",
       triggerEvent: event,
     });
   },
 
   /**
    * Call this on the contextmenu event.  Note that this is called before
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -330,16 +330,22 @@ var StarUI = {
       }, {"capture": true, "once": true});
     };
     gEditItemOverlay.initPanel({ node: aNode,
                                  onPanelReady,
                                  hiddenRows: ["description", "location",
                                               "loadInSidebar", "keyword"],
                                  focusedElement: "preferred"});
 
+    if (aAnchorElement && aAnchorElement.id == BookmarkingUI.STAR_BOX_ID) {
+      aAnchorElement.setAttribute("open", "true");
+      this.panel.addEventListener("popuphiding", () => {
+        aAnchorElement.removeAttribute("open");
+      });
+    }
     this.panel.openPopup(aAnchorElement, aPosition);
   },
 
   panelShown:
   function SU_panelShown(aEvent) {
     if (aEvent.target == this.panel) {
       if (this._element("editBookmarkPanelContent").hidden) {
         // Note this isn't actually used anymore, we should remove this
--- a/browser/base/content/test/urlbar/browser_page_action_menu.js
+++ b/browser/base/content/test/urlbar/browser_page_action_menu.js
@@ -35,17 +35,21 @@ add_task(async function bookmark() {
     // Make sure the edit-bookmark panel opens, then hide it.
     await new Promise(resolve => {
       if (StarUI.panel.state == "open") {
         resolve();
         return;
       }
       StarUI.panel.addEventListener("popupshown", resolve, { once: true });
     });
+    Assert.equal(BookmarkingUI.starBox.getAttribute("open"), "true",
+      "Star has open attribute");
     StarUI.panel.hidePopup();
+    Assert.ok(!BookmarkingUI.starBox.hasAttribute("open"),
+      "Star no longer has open attribute");
 
     // Open the panel again.
     await promisePageActionPanelOpen();
 
     // The bookmark button should now read "Edit This Bookmark" and be starred.
     Assert.equal(bookmarkButton.label, "Edit This Bookmark");
     Assert.ok(bookmarkButton.hasAttribute("starred"));
     Assert.equal(bookmarkButton.getAttribute("starred"), "true");
@@ -126,22 +130,26 @@ add_task(async function sendToDevice_non
       window.getComputedStyle(BrowserPageActions.mainButtonNode).display,
       "none",
       "Main button should be hidden on about:home"
     );
     BrowserPageActions.mainButtonNode.style.display = "-moz-box";
     await promiseSyncReady();
     // Open the panel.  Send to Device should be disabled.
     await promisePageActionPanelOpen();
+    Assert.equal(BrowserPageActions.mainButtonNode.getAttribute("open"),
+      "true", "Main button has 'open' attribute");
     let sendToDeviceButton =
       document.getElementById("pageAction-panel-sendToDevice");
     Assert.ok(sendToDeviceButton.disabled);
     let hiddenPromise = promisePageActionPanelHidden();
     BrowserPageActions.panelNode.hidePopup();
     await hiddenPromise;
+    Assert.ok(!BrowserPageActions.mainButtonNode.hasAttribute("open"),
+      "Main button no longer has 'open' attribute");
     // Remove the `display` style set above.
     BrowserPageActions.mainButtonNode.style.removeProperty("display");
   });
 });
 
 add_task(async function sendToDevice_syncNotReady_other_states() {
   // Open a tab that's sendable.
   await BrowserTestUtils.withNewTab("http://example.com/", async () => {
@@ -491,16 +499,18 @@ add_task(async function sendToDevice_inU
     let urlbarButton = document.getElementById(
       BrowserPageActions._urlbarButtonNodeIDForActionID(action.id)
     );
     Assert.ok(!urlbarButton.disabled);
     let panelPromise =
       promisePanelShown(BrowserPageActions._activatedActionPanelID);
     EventUtils.synthesizeMouseAtCenter(urlbarButton, {});
     await panelPromise;
+    Assert.equal(urlbarButton.getAttribute("open"), "true",
+      "Button has open attribute");
 
     // The devices should be shown in the subview.
     let expectedItems = [
       {
         id: "pageAction-urlbar-sendToDevice-notReady",
         display: "none",
         disabled: true,
       },
@@ -541,16 +551,18 @@ add_task(async function sendToDevice_inU
     }, "Waiting for first device menu item to appear");
 
     // Click it, which should cause the panel to close.
     let hiddenPromise =
       promisePanelHidden(BrowserPageActions._activatedActionPanelID);
     EventUtils.synthesizeMouseAtCenter(deviceMenuItem, {});
     info("Waiting for Send to Device panel to close after clicking a device");
     await hiddenPromise;
+    Assert.ok(!urlbarButton.hasAttribute("open"),
+      "URL bar button no longer has open attribute");
 
     // And then the "Sent!" notification panel should open and close by itself
     // after a moment.
     info("Waiting for the Sent! notification panel to open");
     await promisePanelShown(BrowserPageActionFeedback.panelNode.id);
     Assert.equal(
       BrowserPageActionFeedback.panelNode.anchorNode.id,
       urlbarButton.id
--- a/browser/themes/shared/urlbar-searchbar.inc.css
+++ b/browser/themes/shared/urlbar-searchbar.inc.css
@@ -181,21 +181,24 @@
   padding: 7px;
 }
 
 .urlbar-icon:hover,
 .urlbar-icon-wrapper:hover {
   background-color: hsla(0,0%,80%,.4);
 }
 
+.urlbar-icon[open],
+.urlbar-icon-wrapper[open],
 .urlbar-icon:hover:active,
 .urlbar-icon-wrapper:hover:active {
-  background-color: hsla(0,0%,80%,.45);
+  background-color: hsla(0,0%,80%,.6);
 }
 
+.urlbar-icon-wrapper[open] > .urlbar-icon,
 .urlbar-icon-wrapper > .urlbar-icon:hover,
 .urlbar-icon-wrapper > .urlbar-icon:hover:active {
   background-color: transparent;
 }
 
 .urlbar-go-button,
 .search-go-button {
   list-style-image: url("chrome://browser/skin/back.svg");