Bug 1439358 - Part 2 - Don't use the "current" property when restoring focus from the blocked downloads subview. r=Gijs draft
authorPaolo Amadini <paolo.mozmail@amadzone.org>
Thu, 22 Feb 2018 12:20:53 +0000
changeset 758478 7a34b33ba4ffbf7c56fe2dbaf5fc35c70c3b0cc7
parent 758477 0a4e4335d953649d097749692479da16bb6202fd
child 758479 81f2c22ef023eb2bcb1332ca554a54c3e66f2c80
push id100065
push userpaolo.mozmail@amadzone.org
push dateThu, 22 Feb 2018 14:26:14 +0000
reviewersGijs
bugs1439358
milestone60.0a1
Bug 1439358 - Part 2 - Don't use the "current" property when restoring focus from the blocked downloads subview. r=Gijs MozReview-Commit-ID: ASmmdQMctr5
browser/components/downloads/content/downloads.js
browser/components/downloads/test/browser/browser_downloads_panel_block.js
--- a/browser/components/downloads/content/downloads.js
+++ b/browser/components/downloads/content/downloads.js
@@ -1210,17 +1210,17 @@ var DownloadsViewController = {
       return false;
     }
     if (!(aCommand in this) &&
         !(aCommand in DownloadsViewItem.prototype)) {
       return false;
     }
     // The currently supported commands depend on whether the blocked subview is
     // showing.  If it is, then take the following path.
-    if (DownloadsBlockedSubview.view.showingSubView) {
+    if (DownloadsView.subViewOpen) {
       let blockedSubviewCmds = [
         "downloadsCmd_unblockAndOpen",
         "cmd_delete",
       ];
       return blockedSubviewCmds.includes(aCommand);
     }
     // If the blocked subview is not showing, then determine if focus is on a
     // control in the downloads list.
@@ -1525,23 +1525,16 @@ XPCOMUtils.defineConstant(this, "Downloa
 
 
 // DownloadsBlockedSubview
 
 /**
  * Manages the blocked subview that slides in when you click a blocked download.
  */
 var DownloadsBlockedSubview = {
-
-  get subview() {
-    let subview = document.getElementById("downloadsPanel-blockedSubview");
-    delete this.subview;
-    return this.subview = subview;
-  },
-
   /**
    * Elements in the subview.
    */
   get elements() {
     let idSuffixes = [
       "title",
       "details1",
       "details2",
@@ -1552,25 +1545,16 @@ var DownloadsBlockedSubview = {
       memo[s] = document.getElementById("downloadsPanel-blockedSubview-" + s);
       return memo;
     }, {});
     delete this.elements;
     return this.elements = elements;
   },
 
   /**
-   * The multiview that contains both the main view and the subview.
-   */
-  get view() {
-    let view = document.getElementById("downloadsPanel-multiView");
-    delete this.view;
-    return this.view = view;
-  },
-
-  /**
    * The blocked-download richlistitem element that was clicked to show the
    * subview.  If the subview is not showing, this is undefined.
    */
   element: undefined,
 
   /**
    * Slides in the blocked subview.
    *
@@ -1590,47 +1574,47 @@ var DownloadsBlockedSubview = {
     e.title.textContent = title;
     e.details1.textContent = details[0];
     e.details2.textContent = details[1];
     e.openButton.label = s.unblockButtonOpen;
     e.deleteButton.label = s.unblockButtonConfirmBlock;
 
     let verdict = element.getAttribute("verdict");
     this.subview.setAttribute("verdict", verdict);
-    this.subview.addEventListener("ViewHiding", this);
 
-    this.view.showSubView(this.subview.id);
+    this.mainView.addEventListener("ViewShown", this);
+    DownloadsPanel.panel.addEventListener("popuphidden", this);
+    this.panelMultiView.showSubView(this.subview);
 
     // Without this, the mainView is more narrow than the panel once all
     // downloads are removed from the panel.
-    document.getElementById("downloadsPanel-mainView").style.minWidth =
-      window.getComputedStyle(this.subview).width;
+    this.mainView.style.minWidth = window.getComputedStyle(this.subview).width;
   },
 
   handleEvent(event) {
-    switch (event.type) {
-      case "ViewHiding":
-        this.subview.removeEventListener(event.type, this);
-        DownloadsView.subViewOpen = false;
-        // If we're going back to the main panel, use showPanel to
-        // focus the proper element.
-        if (this.view.current !== this.subview) {
-          DownloadsPanel.showPanel();
-        }
-        break;
-      default:
-        DownloadsCommon.log("Unhandled DownloadsBlockedSubview event: " +
-                            event.type);
-        break;
+    // This is called when the main view is shown or the panel is hidden.
+    DownloadsView.subViewOpen = false;
+    this.mainView.removeEventListener("ViewShown", this);
+    DownloadsPanel.panel.removeEventListener("popuphidden", this);
+    // Focus the proper element if we're going back to the main panel.
+    if (event.type == "ViewShown") {
+      DownloadsPanel.showPanel();
     }
   },
 
   /**
    * Deletes the download and hides the entire panel.
    */
   confirmBlock() {
     goDoCommand("cmd_delete");
     DownloadsPanel.hidePanel();
   },
 };
 
+XPCOMUtils.defineLazyGetter(DownloadsBlockedSubview, "panelMultiView",
+  () => document.getElementById("downloadsPanel-multiView"));
+XPCOMUtils.defineLazyGetter(DownloadsBlockedSubview, "mainView",
+  () => document.getElementById("downloadsPanel-mainView"));
+XPCOMUtils.defineLazyGetter(DownloadsBlockedSubview, "subview",
+  () => document.getElementById("downloadsPanel-blockedSubview"));
+
 XPCOMUtils.defineConstant(this, "DownloadsBlockedSubview",
                           DownloadsBlockedSubview);
--- a/browser/components/downloads/test/browser/browser_downloads_panel_block.js
+++ b/browser/components/downloads/test/browser/browser_downloads_panel_block.js
@@ -15,32 +15,34 @@ add_task(async function mainTest() {
   for (let i = 0; i < verdicts.length; i++) {
     await openPanel();
 
     // The current item is always the first one in the listbox since each
     // iteration of this loop removes the item at the end.
     let item = DownloadsView.richListBox.firstChild;
 
     // Open the panel and click the item to show the subview.
+    let viewPromise = promiseViewShown(DownloadsBlockedSubview.subview);
     EventUtils.sendMouseEvent({ type: "click" }, item);
-    await promiseSubviewShown(true);
+    await viewPromise;
 
     // Items are listed in newest-to-oldest order, so e.g. the first item's
     // verdict is the last element in the verdicts array.
     Assert.ok(DownloadsBlockedSubview.subview.getAttribute("verdict"),
               verdicts[verdicts.count - i - 1]);
 
-    // Click the sliver of the main view that's still showing on the left to go
-    // back to it.
-    EventUtils.synthesizeMouse(DownloadsPanel.panel, 10, 10, {}, window);
-    await promiseSubviewShown(false);
+    // Go back to the main view.
+    viewPromise = promiseViewShown(DownloadsBlockedSubview.mainView);
+    DownloadsBlockedSubview.panelMultiView.goBack();
+    await viewPromise;
 
     // Show the subview again.
+    viewPromise = promiseViewShown(DownloadsBlockedSubview.subview);
     EventUtils.sendMouseEvent({ type: "click" }, item);
-    await promiseSubviewShown(true);
+    await viewPromise;
 
     // Click the Open button.  The download should be unblocked and then opened,
     // i.e., unblockAndOpenDownload() should be called on the item.  The panel
     // should also be closed as a result, so wait for that too.
     let unblockOpenPromise = promiseUnblockAndOpenDownloadCalled(item);
     let hidePromise = promisePanelHidden();
     EventUtils.synthesizeMouse(DownloadsBlockedSubview.elements.openButton,
                                10, 10, {}, window);
@@ -48,29 +50,33 @@ add_task(async function mainTest() {
     await hidePromise;
 
     window.focus();
     await SimpleTest.promiseFocus(window);
 
     // Reopen the panel and show the subview again.
     await openPanel();
 
+    viewPromise = promiseViewShown(DownloadsBlockedSubview.subview);
     EventUtils.sendMouseEvent({ type: "click" }, item);
-    await promiseSubviewShown(true);
+    await viewPromise;
 
     // Click the Remove button.  The panel should close and the item should be
     // removed from it.
+    hidePromise = promisePanelHidden();
     EventUtils.synthesizeMouse(DownloadsBlockedSubview.elements.deleteButton,
                                10, 10, {}, window);
-    await promisePanelHidden();
-    await openPanel();
+    await hidePromise;
 
+    await openPanel();
     Assert.ok(!item.parentNode);
+
+    hidePromise = promisePanelHidden();
     DownloadsPanel.hidePanel();
-    await promisePanelHidden();
+    await hidePromise;
   }
 
   await task_resetState();
 });
 
 async function openPanel() {
   // This function is insane but something intermittently causes the panel to be
   // closed as soon as it's opening on Linux ASAN.  Maybe it would also happen
@@ -122,53 +128,35 @@ async function openPanel() {
           }
           DownloadsPanel.showPanel();
       }
     }, 100);
   });
 }
 
 function promisePanelHidden() {
-  return new Promise(resolve => {
-    if (!DownloadsPanel.panel || DownloadsPanel.panel.state == "closed") {
-      resolve();
-      return;
-    }
-    DownloadsPanel.panel.addEventListener("popuphidden", function() {
-      setTimeout(resolve, 0);
-    }, {once: true});
-  });
+  return BrowserTestUtils.waitForEvent(DownloadsPanel.panel, "popuphidden");
 }
 
 function makeDownload(verdict) {
   return {
     state: DownloadsCommon.DOWNLOAD_DIRTY,
     hasBlockedData: true,
     errorObj: {
       result: Components.results.NS_ERROR_FAILURE,
       message: "Download blocked.",
       becauseBlocked: true,
       becauseBlockedByReputationCheck: true,
       reputationCheckVerdict:  verdict,
     },
   };
 }
 
-function promiseSubviewShown(shown) {
-  // More terribleness, but I'm tired of fighting intermittent timeouts on try.
-  // Just poll for the subview and wait a second before resolving the promise.
-  return new Promise(resolve => {
-    let interval = setInterval(() => {
-      if (shown == DownloadsBlockedSubview.view.showingSubView &&
-          !DownloadsBlockedSubview.view._transitioning) {
-        clearInterval(interval);
-        setTimeout(resolve, 1000);
-      }
-    }, 0);
-  });
+function promiseViewShown(view) {
+  return BrowserTestUtils.waitForEvent(view, "ViewShown");
 }
 
 function promiseUnblockAndOpenDownloadCalled(item) {
   return new Promise(resolve => {
     let realFn = item._shell.unblockAndOpenDownload;
     item._shell.unblockAndOpenDownload = () => {
       item._shell.unblockAndOpenDownload = realFn;
       resolve();