From f37e4076597665e08b4bdf6da3768827db33d8ad Mon Sep 17 00:00:00 2001 From: Paolo Amadini Date: Thu, 22 Feb 2018 12:20:53 +0000 Subject: [PATCH] Bug 1439358 - Part 2 - Don't use the "current" property when restoring focus from the blocked downloads subview. r=Gijs MozReview-Commit-ID: ASmmdQMctr5 --HG-- extra : rebase_source : 7d990c407e63e6c5a2db3c644e84da7f243e9909 --- .../components/downloads/content/downloads.js | 54 +++++++------------ .../browser/browser_downloads_panel_block.js | 50 +++++++---------- 2 files changed, 38 insertions(+), 66 deletions(-) diff --git a/browser/components/downloads/content/downloads.js b/browser/components/downloads/content/downloads.js index 6b959a80b9e1..820bec8d1f30 100644 --- a/browser/components/downloads/content/downloads.js +++ b/browser/components/downloads/content/downloads.js @@ -1215,7 +1215,7 @@ var DownloadsViewController = { } // 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", @@ -1530,13 +1530,6 @@ XPCOMUtils.defineConstant(this, "DownloadsFooter", DownloadsFooter); * 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. */ @@ -1556,15 +1549,6 @@ var DownloadsBlockedSubview = { 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. @@ -1595,31 +1579,24 @@ var DownloadsBlockedSubview = { 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(); } }, @@ -1632,5 +1609,12 @@ var DownloadsBlockedSubview = { }, }; +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); diff --git a/browser/components/downloads/test/browser/browser_downloads_panel_block.js b/browser/components/downloads/test/browser/browser_downloads_panel_block.js index 6e1954b54f7a..b7e756e4b313 100644 --- a/browser/components/downloads/test/browser/browser_downloads_panel_block.js +++ b/browser/components/downloads/test/browser/browser_downloads_panel_block.js @@ -20,22 +20,24 @@ add_task(async function mainTest() { 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 @@ -53,19 +55,23 @@ add_task(async function mainTest() { // 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(); @@ -127,15 +133,7 @@ async function openPanel() { } 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) { @@ -152,18 +150,8 @@ function makeDownload(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) {