Bug 1388835 - Hide page action urlbar buttons on about pages (about:preferences, etc.). r=Gijs
authorDrew Willcoxon <adw@mozilla.com>
Sat, 26 Aug 2017 12:47:33 -0700
changeset 377091 cce829b3379fdf36894ae70ed0161cd354a0e85a
parent 377090 c659172882b4a952864bb14491340f941f05fa01
child 377092 c423d7c2998a580d7ffaf597c646065e3e349ea4
push id49738
push userdwillcoxon@mozilla.com
push dateSat, 26 Aug 2017 21:26:45 +0000
treeherderautoland@cce829b3379f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1388835
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -660,19 +660,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
@@ -858,36 +858,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);