Bug 1388835 - Hide page action urlbar buttons on about pages (about:preferences, etc.). r?Gijs draft
authorDrew Willcoxon <adw@mozilla.com>
Sat, 26 Aug 2017 12:47:33 -0700
changeset 653617 5abf14837493a7efff2eaa0b1d567ba987f12033
parent 653365 03d7b6dd65b93afaa6981269f69e9f7cd34224bc
child 728378 a2f99de58ac8267a817ddec967167935070858c9
push id76370
push userdwillcoxon@mozilla.com
push dateSat, 26 Aug 2017 19:47:57 +0000
reviewersGijs
bugs1388835
milestone57.0a1
Bug 1388835 - Hide page action urlbar buttons on about pages (about:preferences, etc.). r?Gijs MozReview-Commit-ID: 9lid8VpPkJE
browser/base/content/browser-pageActions.js
browser/base/content/browser.css
browser/base/content/browser.xul
browser/base/content/test/general/browser_bookmark_popup.js
browser/base/content/test/urlbar/browser_page_action_menu.js
browser/base/content/test/urlbar/head.js
browser/extensions/pocket/bootstrap.js
browser/modules/test/browser/browser_PageActions.js
--- a/browser/base/content/browser-pageActions.js
+++ b/browser/base/content/browser-pageActions.js
@@ -333,17 +333,17 @@ var BrowserPageActions = {
       }
     }
 
     return node;
   },
 
   _makeUrlbarButtonNode(action) {
     let buttonNode = document.createElement("image");
-    buttonNode.classList.add("urlbar-icon");
+    buttonNode.classList.add("urlbar-icon", "urlbar-page-action");
     if (action.tooltip) {
       buttonNode.setAttribute("tooltiptext", action.tooltip);
     }
     if (action.iconURL) {
       buttonNode.style.listStyleImage = `url('${action.iconURL}')`;
     }
     buttonNode.setAttribute("context", "pageActionPanelContextMenu");
     buttonNode.addEventListener("contextmenu", event => {
--- a/browser/base/content/browser.css
+++ b/browser/base/content/browser.css
@@ -659,19 +659,18 @@ html|input.urlbar-input[textoverflow]:no
 #PopupAutoCompleteRichResult.showSearchSuggestionsNotification > richlistbox {
   transition: none;
 }
 
 #DateTimePickerPanel[active="true"] {
   -moz-binding: url("chrome://global/content/bindings/datetimepopup.xml#datetime-popup");
 }
 
-#urlbar[pageproxystate="invalid"] > #page-action-buttons > .urlbar-icon,
-#urlbar[pageproxystate="invalid"] > #page-action-buttons > .urlbar-icon-wrapper,
-#urlbar[pageproxystate="invalid"] > #page-action-buttons > #pageActionSeparator,
+#urlbar[pageproxystate=invalid] > #page-action-buttons > .urlbar-page-action,
+#identity-box.chromeUI ~ #page-action-buttons > .urlbar-page-action,
 .urlbar-go-button[pageproxystate="valid"],
 .urlbar-go-button:not([parentfocused="true"]),
 #urlbar[pageproxystate="invalid"] > #identity-box > #blocked-permissions-container,
 #urlbar[pageproxystate="invalid"] > #identity-box > #notification-popup-box,
 #urlbar[pageproxystate="invalid"] > #identity-box > #identity-icon-labels {
   visibility: collapse;
 }
 
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -856,36 +856,36 @@
                 <label id="extension" class="urlbar-display urlbar-display-extension" value="&urlbar.extension.label;"/>
               </box>
               <hbox id="page-action-buttons">
                 <hbox id="userContext-icons" hidden="true">
                   <label id="userContext-label"/>
                   <image id="userContext-indicator"/>
                 </hbox>
                 <image id="page-report-button"
-                       class="urlbar-icon"
+                       class="urlbar-icon urlbar-page-action"
                        hidden="true"
                        tooltiptext="&pageReportIcon.tooltip;"
                        onmousedown="gPopupBlockerObserver.onReportButtonMousedown(event);"/>
                 <image id="reader-mode-button"
-                       class="urlbar-icon"
+                       class="urlbar-icon urlbar-page-action"
                        hidden="true"
                        onclick="ReaderParent.buttonClick(event);"/>
                 <toolbarbutton id="urlbar-zoom-button"
                        onclick="FullZoom.reset();"
                        tooltip="dynamic-shortcut-tooltip"
                        hidden="true"/>
-                <box id="pageActionSeparator"/>
+                <box id="pageActionSeparator" class="urlbar-page-action"/>
                 <image id="pageActionButton"
-                       class="urlbar-icon"
+                       class="urlbar-icon urlbar-page-action"
                        tooltiptext="&pageActionButton.tooltip;"
                        onclick="BrowserPageActions.mainButtonClicked(event);"/>
                 <hbox id="star-button-box"
                       hidden="true"
-                      class="urlbar-icon-wrapper"
+                      class="urlbar-icon-wrapper urlbar-page-action"
                       context="pageActionPanelContextMenu"
                       oncontextmenu="BrowserPageActions.onContextMenu(event);"
                       onclick="BrowserPageActions.bookmark.onUrlbarNodeClicked(event);">
                   <image id="star-button"
                          class="urlbar-icon"
                          observes="bookmarkThisPageBroadcaster"/>
                   <hbox id="star-button-animatable-box">
                     <image id="star-button-animatable-image"
--- a/browser/base/content/test/general/browser_bookmark_popup.js
+++ b/browser/base/content/test/general/browser_bookmark_popup.js
@@ -9,33 +9,35 @@
  * Test opening and closing the bookmarks panel.
  */
 
 let bookmarkPanel = document.getElementById("editBookmarkPanel");
 let bookmarkStar = BookmarkingUI.star;
 let bookmarkPanelTitle = document.getElementById("editBookmarkPanelTitle");
 let editBookmarkPanelRemoveButtonRect;
 
+const TEST_URL = "data:text/html,<html><body></body></html>";
+
 StarUI._closePanelQuickForTesting = true;
 
 add_task(async function setup() {
   bookmarkPanel.setAttribute("animate", false);
   registerCleanupFunction(() => {
     bookmarkPanel.removeAttribute("animate");
   });
 })
 
 async function test_bookmarks_popup({isNewBookmark, popupShowFn, popupEditFn,
                                 shouldAutoClose, popupHideFn, isBookmarkRemoved}) {
-  await BrowserTestUtils.withNewTab({gBrowser, url: "about:home"}, async function(browser) {
+  await BrowserTestUtils.withNewTab({gBrowser, url: TEST_URL}, async function(browser) {
     try {
       if (!isNewBookmark) {
         await PlacesUtils.bookmarks.insert({
           parentGuid: PlacesUtils.bookmarks.unfiledGuid,
-          url: "about:home",
+          url: TEST_URL,
           title: "Home Page"
         });
       }
 
       info(`BookmarkingUI.status is ${BookmarkingUI.status}`);
       await BrowserTestUtils.waitForCondition(
         () => BookmarkingUI.status != BookmarkingUI.STATUS_UPDATING,
         "BookmarkingUI should not be updating");
@@ -50,46 +52,46 @@ async function test_bookmarks_popup({isN
 
     editBookmarkPanelRemoveButtonRect =
       document.getElementById("editBookmarkPanelRemoveButton").getBoundingClientRect();
 
       if (popupEditFn) {
         await popupEditFn();
       }
       let bookmarks = [];
-      await PlacesUtils.bookmarks.fetch({url: "about:home"}, bm => bookmarks.push(bm));
+      await PlacesUtils.bookmarks.fetch({url: TEST_URL}, bm => bookmarks.push(bm));
       Assert.equal(bookmarks.length, 1, "Only one bookmark should exist");
       Assert.equal(bookmarkStar.getAttribute("starred"), "true", "Page is starred");
       Assert.equal(bookmarkPanelTitle.value,
         isNewBookmark ?
           gNavigatorBundle.getString("editBookmarkPanel.pageBookmarkedTitle") :
           gNavigatorBundle.getString("editBookmarkPanel.editBookmarkTitle"),
         "title should match isEditingBookmark state");
 
       if (!shouldAutoClose) {
         await new Promise(resolve => setTimeout(resolve, 400));
         Assert.equal(bookmarkPanel.state, "open", "Panel should still be 'open' for non-autoclose");
       }
 
       let onItemRemovedPromise = Promise.resolve();
       if (isBookmarkRemoved) {
         onItemRemovedPromise = PlacesTestUtils.waitForNotification("onItemRemoved",
-          (id, parentId, index, type, itemUrl) => "about:home" == itemUrl.spec);
+          (id, parentId, index, type, itemUrl) => TEST_URL == itemUrl.spec);
       }
 
       let hiddenPromise = promisePopupHidden(bookmarkPanel);
       if (popupHideFn) {
         await popupHideFn();
       }
       await Promise.all([hiddenPromise, onItemRemovedPromise]);
 
       Assert.equal(bookmarkStar.hasAttribute("starred"), !isBookmarkRemoved,
          "Page is starred after closing");
     } finally {
-      let bookmark = await PlacesUtils.bookmarks.fetch({url: "about:home"});
+      let bookmark = await PlacesUtils.bookmarks.fetch({url: TEST_URL});
       Assert.equal(!!bookmark, !isBookmarkRemoved,
          "bookmark should not be present if a panel action should've removed it");
       if (bookmark) {
         await PlacesUtils.bookmarks.remove(bookmark);
       }
     }
   });
 }
--- a/browser/base/content/test/urlbar/browser_page_action_menu.js
+++ b/browser/base/content/test/urlbar/browser_page_action_menu.js
@@ -110,28 +110,40 @@ add_task(async function emailLink() {
     EventUtils.synthesizeMouseAtCenter(emailLinkButton, {});
     await hiddenPromise;
 
     Assert.ok(fnCalled);
   });
 });
 
 add_task(async function sendToDevice_nonSendable() {
-  // Open a tab that's not sendable -- but that's also actionable so that the
-  // main page action button appears.
+  // Open a tab that's not sendable.  An about: page like about:home is
+  // convenient.
   await BrowserTestUtils.withNewTab("about:home", async () => {
+    // ... but the page actions should be hidden on about:home, including the
+    // main button.  (It's not easy to load a page that's both actionable and
+    // not sendable.)  So first check that that's the case, and then unhide the
+    // main button so that this test can continue.
+    Assert.equal(
+      window.getComputedStyle(BrowserPageActions.mainButtonNode).visibility,
+      "collapse",
+      "Main button should be hidden on about:home"
+    );
+    BrowserPageActions.mainButtonNode.style.visibility = "visible";
     await promiseSyncReady();
     // Open the panel.  Send to Device should be disabled.
     await promisePageActionPanelOpen();
     let sendToDeviceButton =
       document.getElementById("pageAction-panel-sendToDevice");
     Assert.ok(sendToDeviceButton.disabled);
     let hiddenPromise = promisePageActionPanelHidden();
     BrowserPageActions.panelNode.hidePopup();
     await hiddenPromise;
+    // Remove the `visibility` style set above.
+    BrowserPageActions.mainButtonNode.style.removeProperty("visibility");
   });
 });
 
 add_task(async function sendToDevice_syncNotReady_other_states() {
   // Open a tab that's sendable.
   await BrowserTestUtils.withNewTab("http://example.com/", async () => {
     await promiseSyncReady();
     const sandbox = sinon.sandbox.create();
--- a/browser/base/content/test/urlbar/head.js
+++ b/browser/base/content/test/urlbar/head.js
@@ -222,19 +222,29 @@ function promiseNewSearchEngine(basename
     });
   });
 }
 
 function promisePageActionPanelOpen() {
   if (BrowserPageActions.panelNode.state == "open") {
     return Promise.resolve();
   }
-  let shownPromise = promisePageActionPanelShown();
-  EventUtils.synthesizeMouseAtCenter(BrowserPageActions.mainButtonNode, {});
-  return shownPromise;
+  // The main page action button is hidden for some URIs, so make sure it's
+  // visible before trying to click it.
+  let dwu = window.QueryInterface(Ci.nsIInterfaceRequestor)
+                  .getInterface(Ci.nsIDOMWindowUtils);
+  return BrowserTestUtils.waitForCondition(() => {
+    info("Waiting for main page action button to have non-0 size");
+    let bounds = dwu.getBoundsWithoutFlushing(BrowserPageActions.mainButtonNode);
+    return bounds.width > 0 && bounds.height > 0;
+  }).then(() => {
+    let shownPromise = promisePageActionPanelShown();
+    EventUtils.synthesizeMouseAtCenter(BrowserPageActions.mainButtonNode, {});
+    return shownPromise;
+  });
 }
 
 function promisePageActionPanelShown() {
   return promisePanelShown(BrowserPageActions.panelNode);
 }
 
 function promisePageActionPanelHidden() {
   return promisePanelHidden(BrowserPageActions.panelNode);
--- a/browser/extensions/pocket/bootstrap.js
+++ b/browser/extensions/pocket/bootstrap.js
@@ -104,17 +104,17 @@ var PocketPageAction = {
           let doc = window.document;
 
           if (doc.getElementById("pocket-button-box")) {
             return;
           }
 
           let wrapper = doc.createElement("hbox");
           wrapper.id = "pocket-button-box";
-          wrapper.classList.add("urlbar-icon-wrapper");
+          wrapper.classList.add("urlbar-icon-wrapper", "urlbar-page-action");
           wrapper.setAttribute("context", "pageActionPanelContextMenu");
           wrapper.addEventListener("contextmenu", event => {
             window.BrowserPageActions.onContextMenu(event);
           });
           let animatableBox = doc.createElement("hbox");
           animatableBox.id = "pocket-animatable-box";
           let animatableImage = doc.createElement("image");
           animatableImage.id = "pocket-animatable-image";
--- a/browser/modules/test/browser/browser_PageActions.js
+++ b/browser/modules/test/browser/browser_PageActions.js
@@ -759,19 +759,29 @@ add_task(async function nonBuiltFirst() 
     initialActions.map(a => BrowserPageActions._panelButtonNodeIDForActionID(a.id)),
     "Action should no longer be in panel"
   );
 });
 
 
 function promisePageActionPanelOpen() {
   let button = document.getElementById("pageActionButton");
-  let shownPromise = promisePageActionPanelShown();
-  EventUtils.synthesizeMouseAtCenter(button, {});
-  return shownPromise;
+  // The main page action button is hidden for some URIs, so make sure it's
+  // visible before trying to click it.
+  let dwu = window.QueryInterface(Ci.nsIInterfaceRequestor)
+                  .getInterface(Ci.nsIDOMWindowUtils);
+  return BrowserTestUtils.waitForCondition(() => {
+    info("Waiting for main page action button to have non-0 size");
+    let bounds = dwu.getBoundsWithoutFlushing(button);
+    return bounds.width > 0 && bounds.height > 0;
+  }).then(() => {
+    let shownPromise = promisePageActionPanelShown();
+    EventUtils.synthesizeMouseAtCenter(button, {});
+    return shownPromise;
+  });
 }
 
 function promisePageActionPanelShown() {
   return promisePanelShown(BrowserPageActions.panelNode);
 }
 
 function promisePageActionPanelHidden() {
   return promisePanelHidden(BrowserPageActions.panelNode);