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 500529 8594386886d1e8b9b0a6216ec32c1ab86f32662b
parent 500528 6aeee70256ad371b974717f50c400bc1091373d3
child 500530 225546ee5bd2c9190cce2bf673000146bdd77952
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersadw
bugs1499444
milestone64.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
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() {