From b5785ef7725aaa6e930a9bb695e360eca4be1730 Mon Sep 17 00:00:00 2001
From: Gijs Kruitbosch <gijskruitbosch@gmail.com>
Date: Fri, 28 Dec 2018 11:27:00 +0000
Subject: [PATCH] Bug 1514724 - move pocket page action into markup, r=jaws

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

--HG--
extra : rebase_source : bc5d116af660342b069f39a1b3ccbd04dedcd6f9
---
 browser/base/content/browser.xul              | 12 ++++++
 .../pocket/content/SaveToPocket.jsm           | 41 +++----------------
 .../pocket/test/browser_pocket_ui_check.js    |  6 ++-
 3 files changed, 22 insertions(+), 37 deletions(-)

diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul
index 34e2080ccdf1..872a5455ddb2 100644
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@@ -1047,6 +1047,18 @@ xmlns="http://www.w3.org/1999/xhtml"
                            role="presentation"/>
                   </hbox>
                 </hbox>
+                <hbox id="pocket-button-box"
+                      hidden="true"
+                      class="urlbar-icon-wrapper urlbar-page-action"
+                      onclick="BrowserPageActions.doCommandForAction(PageActions.actionForID('pocket'), event, this);">
+                  <image id="pocket-button"
+                         class="urlbar-icon"
+                         role="button"/>
+                  <hbox id="pocket-button-animatable-box">
+                    <image id="pocket-button-animatable-image"
+                           role="presentation"/>
+                  </hbox>
+                </hbox>
               </hbox>
             </textbox>
         </toolbaritem>
diff --git a/browser/components/pocket/content/SaveToPocket.jsm b/browser/components/pocket/content/SaveToPocket.jsm
index 9e18107ef9d9..525a52dbbc15 100644
--- a/browser/components/pocket/content/SaveToPocket.jsm
+++ b/browser/components/pocket/content/SaveToPocket.jsm
@@ -85,46 +85,17 @@ var PocketPageAction = {
         _urlbarNodeInMarkup: true,
         onBeforePlacedInWindow(window) {
           let doc = window.document;
-
-          if (doc.getElementById("pocket-button-box")) {
-            return;
-          }
-
-          let wrapper = doc.createXULElement("hbox");
-          wrapper.id = "pocket-button-box";
-          wrapper.classList.add("urlbar-icon-wrapper", "urlbar-page-action");
-          let animatableBox = doc.createXULElement("hbox");
-          animatableBox.id = "pocket-animatable-box";
-          let animatableImage = doc.createXULElement("image");
-          animatableImage.id = "pocket-animatable-image";
-          animatableImage.setAttribute("role", "presentation");
           let tooltip =
             gPocketBundle.GetStringFromName("pocket-button.tooltiptext");
-          animatableImage.setAttribute("tooltiptext", tooltip);
-          let pocketButton = doc.createXULElement("image");
-          pocketButton.id = "pocket-button";
-          pocketButton.classList.add("urlbar-icon");
-          pocketButton.setAttribute("role", "button");
-          pocketButton.setAttribute("tooltiptext", tooltip);
-
-          wrapper.appendChild(pocketButton);
-          wrapper.appendChild(animatableBox);
-          animatableBox.appendChild(animatableImage);
-          let iconBox = doc.getElementById("page-action-buttons");
-          iconBox.appendChild(wrapper);
-          wrapper.hidden = true;
-
-          wrapper.addEventListener("click", event => {
-            let {BrowserPageActions} = wrapper.ownerGlobal;
-            BrowserPageActions.doCommandForAction(this, event, wrapper);
-          });
+          doc.getElementById("pocket-button").setAttribute("tooltiptext", tooltip);
+          doc.getElementById("pocket-button-animatable-image").setAttribute("tooltiptext", tooltip);
         },
         onIframeShowing(iframe, panel) {
           Pocket.onShownInPhotonPageActionPanel(panel, iframe);
 
           let doc = panel.ownerDocument;
           let urlbarNode = doc.getElementById("pocket-button-box");
-          if (!urlbarNode || urlbarNode.hidden) {
+          if (!urlbarNode) {
             return;
           }
 
@@ -196,7 +167,7 @@ var PocketPageAction = {
     let urlbarNode = browserWindow.document.getElementById(
       BrowserPageActions.urlbarButtonNodeIDForActionID(this.pageAction.id)
     );
-    if (!urlbarNode) {
+    if (!urlbarNode || urlbarNode.hidden) {
       return;
     }
     let browser = browserWindow.gBrowser.selectedBrowser;
@@ -218,9 +189,7 @@ var PocketPageAction = {
     for (let win of browserWindows()) {
       let doc = win.document;
       let pocketButtonBox = doc.getElementById("pocket-button-box");
-      if (pocketButtonBox) {
-        pocketButtonBox.remove();
-      }
+      pocketButtonBox.setAttribute("hidden", "true");
     }
 
     this.pageAction.remove();
diff --git a/browser/components/pocket/test/browser_pocket_ui_check.js b/browser/components/pocket/test/browser_pocket_ui_check.js
index 81218a0977a2..208f9ee142e3 100644
--- a/browser/components/pocket/test/browser_pocket_ui_check.js
+++ b/browser/components/pocket/test/browser_pocket_ui_check.js
@@ -17,6 +17,8 @@ add_task(async function() {
 
   checkWindowProperties(true, ["Pocket", "pktUI", "pktUIMessaging"]);
   checkElements(true, ["pocket-button", "appMenu-library-pocket-button"]);
+  let buttonBox = document.getElementById("pocket-button-box");
+  is(buttonBox.hidden, false, "Button should not have been hidden");
 
   // check context menu exists
   info("checking content context menu");
@@ -40,8 +42,10 @@ add_task(async function() {
   await promisePocketDisabled();
 
   checkWindowProperties(false, ["Pocket", "pktUI", "pktUIMessaging"]);
-  checkElements(false, ["pocket-button", "appMenu-library-pocket-button",
+  checkElements(false, ["appMenu-library-pocket-button",
                         "context-pocket", "context-savelinktopocket"]);
+  buttonBox = document.getElementById("pocket-button-box");
+  is(buttonBox.hidden, true, "Button should have been hidden");
 
   await promisePocketReset();
 });