Fixes bug 1499444 - Save to pocket button doesn't work on first click after a startup r=adw
authorScottDowne <scott.downe@gmail.com>
Thu, 18 Oct 2018 21:23:57 +0000
changeset 490425 8594386886d1e8b9b0a6216ec32c1ab86f32662b
parent 490424 6aeee70256ad371b974717f50c400bc1091373d3
child 490426 225546ee5bd2c9190cce2bf673000146bdd77952
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersadw
bugs1499444
milestone64.0a1
Fixes bug 1499444 - Save to pocket button doesn't work on first click after a startup r=adw Differential Revision: https://phabricator.services.mozilla.com/D8886
browser/components/pocket/content/Pocket.jsm
browser/components/pocket/content/main.js
--- a/browser/components/pocket/content/Pocket.jsm
+++ b/browser/components/pocket/content/Pocket.jsm
@@ -29,23 +29,18 @@ var Pocket = {
    * Functions related to the Pocket panel UI.
    */
   onShownInPhotonPageActionPanel(panel, iframe) {
     let window = panel.ownerGlobal;
     window.pktUI.setPhotonPageActionPanelFrame(iframe);
     Pocket._initPanelView(window);
   },
 
-  onPanelViewShowing(event) {
-    Pocket._initPanelView(event.target.ownerGlobal);
-  },
-
   _initPanelView(window) {
     let document = window.document;
-    let iframe = window.pktUI.getPanelFrame();
 
     let libraryButton = document.getElementById("library-button");
     if (libraryButton) {
       BrowserUtils.setToolbarButtonHeightProperty(libraryButton);
     }
 
     let urlToSave = Pocket._urlToSave;
     let titleToSave = Pocket._titleToSave;
@@ -54,46 +49,19 @@ var Pocket = {
     // ViewShowing fires immediately before it creates the contents,
     // in lieu of an AfterViewShowing event, just spin the event loop.
     window.setTimeout(function() {
       if (urlToSave) {
         window.pktUI.tryToSaveUrl(urlToSave, titleToSave);
       } else {
         window.pktUI.tryToSaveCurrentPage();
       }
-
-      // pocketPanelDidHide in main.js set iframe to about:blank when it was
-      // hidden, make sure we're loading the save panel.
-      if (iframe.contentDocument &&
-          iframe.contentDocument.readyState == "complete" &&
-          iframe.contentDocument.documentURI != "about:blank") {
-        window.pktUI.pocketPanelDidShow();
-      } else {
-        // iframe didn't load yet. This seems to always be the case when in
-        // the toolbar panel, but never the case for a subview.
-        // XXX this only being fired when it's a _capturing_ listener!
-        iframe.addEventListener("load", Pocket.onFrameLoaded, true);
-      }
     }, 0);
   },
 
-  onFrameLoaded(event) {
-    let document = event.currentTarget.ownerDocument;
-    let window = document.defaultView;
-    let iframe = window.pktUI.getPanelFrame();
-
-    iframe.removeEventListener("load", Pocket.onFrameLoaded, true);
-    window.pktUI.pocketPanelDidShow();
-  },
-
-  onPanelViewHiding(event) {
-    let window = event.target.ownerGlobal;
-    window.pktUI.pocketPanelDidHide(event);
-  },
-
   _urlToSave: null,
   _titleToSave: null,
   savePage(browser, url, title) {
     if (this.pageAction) {
       this._urlToSave = url;
       this._titleToSave = title;
       this.pageAction.doCommand(browser.ownerGlobal);
     }
--- a/browser/components/pocket/content/main.js
+++ b/browser/components/pocket/content/main.js
@@ -50,90 +50,37 @@ ChromeUtils.defineModuleGetter(this, "Pr
 ChromeUtils.defineModuleGetter(this, "ReaderMode",
   "resource://gre/modules/ReaderMode.jsm");
 ChromeUtils.defineModuleGetter(this, "pktApi",
   "chrome://pocket/content/pktApi.jsm");
 
 var pktUI = (function() {
 
     // -- Initialization (on startup and new windows) -- //
-    var _currentPanelDidShow;
-    var _currentPanelDidHide;
 
     // Init panel id at 0. The first actual panel id will have the number 1 so
     // in case at some point any panel has the id 0 we know there is something
     // wrong
     var _panelId = 0;
 
     var overflowMenuWidth = 230;
     var overflowMenuHeight = 475;
     var savePanelWidth = 350;
     var savePanelHeights = {collapsed: 153, expanded: 272};
 
-    var _lastAddSucceeded = false;
-
-    // -- Event Handling -- //
-
-    /**
-     * Event handler when Pocket toolbar button is pressed
-     */
-
-    function pocketPanelDidShow(event) {
-        if (_currentPanelDidShow) {
-            _currentPanelDidShow(event);
-        }
-
-    }
-
-    function pocketPanelDidHide(event) {
-        if (_currentPanelDidHide) {
-            _currentPanelDidHide(event);
-        }
-
-        // clear the panel
-        getPanelFrame().setAttribute("src", "about:blank");
-
-        if (_lastAddSucceeded) {
-            var libraryButton = document.getElementById("library-button");
-            if (!Services.prefs.getBoolPref("toolkit.cosmeticAnimations.enabled") ||
-                !libraryButton ||
-                libraryButton.getAttribute("cui-areatype") == "menu-panel" ||
-                libraryButton.getAttribute("overflowedItem") == "true" ||
-                !libraryButton.closest("#nav-bar")) {
-                return;
-            }
-            libraryButton.removeAttribute("fade");
-            libraryButton.setAttribute("animate", "pocket");
-            libraryButton.addEventListener("animationend", onLibraryButtonAnimationEnd);
-        }
-    }
-
-    function onLibraryButtonAnimationEnd(event) {
-        let doc = event.target.ownerDocument;
-        let libraryButton = doc.getElementById("library-button");
-        if (event.animationName.startsWith("library-pocket-animation")) {
-            libraryButton.setAttribute("fade", "true");
-        } else if (event.animationName == "library-pocket-fade") {
-            libraryButton.removeEventListener("animationend", onLibraryButtonAnimationEnd);
-            libraryButton.removeAttribute("animate");
-            libraryButton.removeAttribute("fade");
-        }
-    }
-
     // -- Communication to API -- //
 
     /**
      * Either save or attempt to log the user in
      */
     function tryToSaveCurrentPage() {
         tryToSaveUrl(getCurrentUrl(), getCurrentTitle());
     }
 
     function tryToSaveUrl(url, title) {
-
         // If the user is logged in, go ahead and save the current page
         if (pktApi.isUserLoggedIn()) {
             saveAndShowConfirmation(url, title);
             return;
         }
 
         // If the user is not logged in, show the logged-out state to prompt them to authenticate
         showSignUp();
@@ -190,19 +137,16 @@ var pktUI = (function() {
                 + "&variant="
                 + variant
                 + "&controlvariant="
                 + controlvariant
                 + "&inoverflowmenu="
                 + inOverflowMenu
                 + "&locale="
                 + getUILocale(), {
-                    onShow() {
-                    },
-                    onHide: panelDidHide,
                     width: inOverflowMenu ? overflowMenuWidth : 300,
                     height: startheight,
             });
         });
     }
 
     /**
      * Show the logged-out state / sign-up panel
@@ -226,17 +170,16 @@ var pktUI = (function() {
             var panelId = showPanel("about:pocket-saved?pockethost="
                 + Services.prefs.getCharPref("extensions.pocket.site")
                 + "&premiumStatus=" + (pktApi.isPremiumUser() ? "1" : "0")
                 + "&fxasignedin=" + ((typeof userdata == "object" && userdata !== null) ? "1" : "0")
                 + "&inoverflowmenu=" + inOverflowMenu
                 + "&locale=" + getUILocale(), {
                 onShow() {
                     var saveLinkMessageId = "saveLink";
-                    _lastAddSucceeded = false;
                     getPanelFrame().setAttribute("itemAdded", "false");
 
                     // Send error message for invalid url
                     if (!isValidURL) {
                         // TODO: Pass key for localized error in error object
                         let error = {
                             message: "Only links can be saved",
                             localizedKey: "onlylinkssaved",
@@ -265,17 +208,16 @@ var pktUI = (function() {
                             var successResponse = {
                                 status: "success",
                                 accountState,
                                 displayName,
                                 item,
                                 ho2,
                             };
                             pktUIMessaging.sendMessageToPanel(panelId, saveLinkMessageId, successResponse);
-                            _lastAddSucceeded = true;
                             getPanelFrame().setAttribute("itemAdded", "true");
                         },
                         error(error, request) {
                             // If user is not authorized show singup page
                             if (request.status === 401) {
                                 showSignUp();
                                 return;
                             }
@@ -293,52 +235,52 @@ var pktUI = (function() {
                     // Add title if given
                     if (typeof title !== "undefined") {
                         options.title = title;
                     }
 
                     // Send the link
                     pktApi.addLink(url, options);
                 },
-                onHide: panelDidHide,
                 width: inOverflowMenu ? overflowMenuWidth : savePanelWidth,
                 height: startheight,
             });
         });
     }
 
     /**
      * Open a generic panel
      */
     function showPanel(url, options) {
-
         // Add new panel id
         _panelId += 1;
         url += ("&panelId=" + _panelId);
 
         // We don't have to hide and show the panel again if it's already shown
         // as if the user tries to click again on the toolbar button the overlay
         // will close instead of the button will be clicked
         var iframe = getPanelFrame();
+        options.onShow = options.onShow || (() => {});
 
         // Register event handlers
         registerEventMessages();
 
         // Load the iframe
         iframe.setAttribute("src", url);
 
-        // Uncomment to leave panel open -- for debugging
-        // panel.setAttribute('noautohide', true);
-        // panel.setAttribute('consumeoutsideclicks', false);
-        //
-
-        // For some reason setting onpopupshown and onpopuphidden on the panel directly didn't work, so
-        // do it this hacky way for now
-        _currentPanelDidShow = options.onShow;
-        _currentPanelDidHide = options.onHide;
+        if (iframe.contentDocument &&
+            iframe.contentDocument.readyState == "complete" &&
+            iframe.contentDocument.documentURI != "about:blank") {
+          options.onShow();
+        } else {
+          // iframe didn't load yet. This seems to always be the case when in
+          // the toolbar panel, but never the case for a subview.
+          // XXX this only being fired when it's a _capturing_ listener!
+          iframe.addEventListener("load", options.onShow, { once: true, capture: true });
+        }
 
         resizePanel({
             width: options.width,
             height: options.height,
         });
         return _panelId;
     }
 
@@ -354,25 +296,16 @@ var pktUI = (function() {
         var iframe = getPanelFrame();
 
         // Set an explicit size, panel will adapt.
         iframe.style.width  = options.width + "px";
         iframe.style.height = options.height + "px";
     }
 
     /**
-     * Called when the signup and saved panel was hidden
-     */
-    function panelDidHide() {
-        // clear the onShow and onHide values
-        _currentPanelDidShow = null;
-        _currentPanelDidHide = null;
-    }
-
-    /**
      * Register all of the messages needed for the panels
      */
     function registerEventMessages() {
         var iframe = getPanelFrame();
 
         // Only register the messages once
         var didInitAttributeKey = "did_init";
         var didInitMessageListener = iframe.getAttribute(didInitAttributeKey);
@@ -512,17 +445,16 @@ var pktUI = (function() {
 
         // Based on clicking "remove page" CTA, and passed unique item id, remove the item
         var _deleteItemMessageId = "deleteItem";
         pktUIMessaging.addMessageListener(iframe, _deleteItemMessageId, function(panelId, data) {
             pktApi.deleteItem(data.itemId, {
                 success(data, response) {
                     var successResponse = {status: "success"};
                     pktUIMessaging.sendResponseMessageToPanel(panelId, _deleteItemMessageId, successResponse);
-                    _lastAddSucceeded = false;
                     getPanelFrame().setAttribute("itemAdded", "false");
                 },
                 error(error, response) {
                     pktUIMessaging.sendErrorResponseMessageToPanel(panelId, _deleteItemMessageId, error);
                 },
             });
         });
 
@@ -631,19 +563,16 @@ var pktUI = (function() {
      * Public functions
      */
     return {
         setPhotonPageActionPanelFrame,
         getPanelFrame,
 
         openTabWithUrl,
 
-        pocketPanelDidShow,
-        pocketPanelDidHide,
-
         tryToSaveUrl,
         tryToSaveCurrentPage,
     };
 }());
 
 // -- Communication to Background -- //
 // https://developer.mozilla.org/en-US/Add-ons/Code_snippets/Interaction_between_privileged_and_non-privileged_pages
 var pktUIMessaging = (function() {