Skip to content

Commit

Permalink
Backed out changeset 5e08d0841b10 (bug 1739348) for causing browser-c…
Browse files Browse the repository at this point in the history
…hrome failures on browser_pdfjs_download_button.js, browser_tempfilename.js
  • Loading branch information
Norisz Fay committed Jun 2, 2022
1 parent 65b0c46 commit dc40d2a
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 281 deletions.
14 changes: 5 additions & 9 deletions browser/components/downloads/DownloadsCommon.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -1003,18 +1003,14 @@ DownloadsDataCtor.prototype = {
browserWin == Services.focus.activeWindow &&
gAlwaysOpenPanel;

// For new downloads after the first one, don't show the panel
// automatically, but provide a visible notification in the topmost browser
// window, if the status indicator is already visible. Also ensure that if
// openDownloadsListOnStart = false is passed, we always skip opening the
// panel. That's because this will only be passed if the download is started
// without user interaction or if a dialog was previously opened in the
// process of the download (e.g. unknown content type dialog).
if (
this.panelHasShownBefore &&
aType != "error" &&
((this.panelHasShownBefore && !shouldOpenDownloadsPanel) ||
!openDownloadsListOnStart)
!shouldOpenDownloadsPanel
) {
// For new downloads after the first one, don't show the panel
// automatically, but provide a visible notification in the topmost
// browser window, if the status indicator is already visible.
DownloadsCommon.log("Showing new download notification.");
browserWin.DownloadsIndicatorView.showEventNotification(aType);
return;
Expand Down
3 changes: 0 additions & 3 deletions browser/components/downloads/test/browser/browser.ini
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,6 @@ support-files =
foo.txt^headers^
[browser_downloads_panel_height.js]
[browser_downloads_panel_opens.js]
support-files =
foo.txt
foo.txt^headers^
[browser_downloads_autohide.js]
[browser_go_to_download_page.js]
[browser_pdfjs_preview.js]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
/* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/publicdomain/zero/1.0/ */

let { MockFilePicker } = SpecialPowers;
MockFilePicker.init(window);
registerCleanupFunction(() => MockFilePicker.cleanup());

/**
* Check that the downloads panel opens when a download is spoofed.
*/
Expand Down Expand Up @@ -89,164 +85,6 @@ function clickCheckbox(checkbox) {
checkbox.parentElement.hidePopup();
}

/**
* Test that the downloads panel correctly opens or doesn't open based on
* whether the download triggered a dialog already. If askWhereToSave is true,
* we should get a file picker dialog. If preferredAction is alwaysAsk, we
* should get an unknown content type dialog. If neither of those is true, we
* should get no dialog at all, and expect the downloads panel to open.
* @param {boolean} [expectPanelToOpen] true - fail if panel doesn't open
* false (default) - fail if it opens
* @param {number} [preferredAction] Default download action:
* 0 (default) - save download to disk
* 1 - open UCT dialog first
* @param {boolean} [askWhereToSave] true - open file picker dialog
* false (default) - use download dir
*/
async function testDownloadsPanelAfterDialog({
expectPanelToOpen = false,
preferredAction,
askWhereToSave = false,
} = {}) {
const { saveToDisk, alwaysAsk } = Ci.nsIHandlerInfo;
if (![saveToDisk, alwaysAsk].includes(preferredAction)) {
preferredAction = saveToDisk;
}
const openUCT = preferredAction === alwaysAsk;
const TEST_PATH = getRootDirectory(gTestPath).replace(
"chrome://mochitests/content",
"https://example.com"
);
const MimeSvc = Cc["@mozilla.org/mime;1"].getService(Ci.nsIMIMEService);
const HandlerSvc = Cc["@mozilla.org/uriloader/handler-service;1"].getService(
Ci.nsIHandlerService
);
let publicList = await Downloads.getList(Downloads.PUBLIC);

for (let download of await publicList.getAll()) {
await publicList.remove(download);
}

// We need to test the changes from bug 1739348, where the helper app service
// sets a flag based on whether a file picker dialog was opened, and this flag
// determines whether the downloads panel will be opened as the download
// starts. We need to actually hit "Save" for the download to start, but we
// can't interact with the real file picker dialog. So this temporarily
// replaces it with a barebones component that plugs into the helper app
// service and tells it to start saving the file to the default path.
if (askWhereToSave) {
MockFilePicker.returnValue = MockFilePicker.returnOK;
MockFilePicker.showCallback = function(fp) {
// Get the default location from the helper app service.
let testFile = MockFilePicker.displayDirectory.clone();
testFile.append(fp.defaultString);
info("File picker download path: " + testFile.path);
MockFilePicker.setFiles([testFile]);
MockFilePicker.filterIndex = 0; // kSaveAsType_Complete
MockFilePicker.showCallback = null;
// Confirm that saving should proceed. The helper app service uses this
// value to determine whether to invoke launcher.saveDestinationAvailable
return MockFilePicker.returnOK;
};
}

await SpecialPowers.pushPrefEnv({
set: [
["browser.download.useDownloadDir", !askWhereToSave],
["browser.download.always_ask_before_handling_new_types", openUCT],
["browser.download.improvements_to_download_panel", true],
["security.dialog_enable_delay", 0],
],
});

// Configure the handler for the file according to parameters.
let mimeInfo = MimeSvc.getFromTypeAndExtension("text/plain", "txt");
let existed = HandlerSvc.exists(mimeInfo);
mimeInfo.alwaysAskBeforeHandling = openUCT;
mimeInfo.preferredAction = preferredAction;
HandlerSvc.store(mimeInfo);
registerCleanupFunction(async () => {
// Reset the handler to its original state.
if (existed) {
HandlerSvc.store(mimeInfo);
} else {
HandlerSvc.remove(mimeInfo);
}
await publicList.removeFinished();
BrowserTestUtils.removeTab(loadingTab);
});

let dialogWindowPromise = BrowserTestUtils.domWindowOpenedAndLoaded();
let downloadFinishedPromise = new Promise(resolve => {
publicList.addView({
onDownloadChanged(download) {
info("Download changed!");
if (download.succeeded || download.error) {
info("Download succeeded or failed.");
publicList.removeView(this);
resolve(download);
}
},
});
});

// Open the tab that will trigger the download.
let loadingTab = await BrowserTestUtils.openNewForegroundTab({
gBrowser,
opening: TEST_PATH + "foo.txt",
waitForLoad: false,
waitForStateStop: true,
});

// Wait for a UCT dialog if the handler was set up to open one.
if (openUCT) {
let dialogWindow = await dialogWindowPromise;
is(
dialogWindow.location.href,
"chrome://mozapps/content/downloads/unknownContentType.xhtml",
"Should have seen the unknown content dialogWindow."
);
let doc = dialogWindow.document;
let dialog = doc.getElementById("unknownContentType");
let radio = doc.getElementById("save");
let button = dialog.getButton("accept");

await TestUtils.waitForCondition(
() => !button.disabled,
"Waiting for the UCT dialog's Accept button to be enabled."
);
ok(!radio.hidden, "The Save option should be visible");
// Make sure we aren't opening the file.
radio.click();
ok(radio.selected, "The Save option should be selected");
button.disabled = false;
dialog.acceptDialog();
}

info("Waiting for download to finish.");
let download = await downloadFinishedPromise;
ok(!download.error, "There should be no error.");
ok(
DownloadsPanel.isPanelShowing === expectPanelToOpen,
`Panel should${expectPanelToOpen ? " " : " not "}be showing.`
);
if (download?.target.exists) {
try {
info("Removing test file: " + download.target.path);
if (Services.appinfo.OS === "WINNT") {
await IOUtils.setPermissions(download.target.path, 0o600);
}
await IOUtils.remove(download.target.path);
} catch (ex) {
/* ignore */
}
}
for (let dl of await publicList.getAll()) {
await publicList.remove(dl);
}
BrowserTestUtils.removeTab(loadingTab);
}

/**
* Make sure the downloads panel opens automatically with a new download.
*/
Expand All @@ -255,7 +93,6 @@ add_task(async function test_downloads_panel_opens() {
set: [
["browser.download.improvements_to_download_panel", true],
["browser.download.always_ask_before_handling_new_types", false],
["browser.download.alwaysOpenPanel", true],
],
});
await checkPanelOpens();
Expand All @@ -266,7 +103,6 @@ add_task(async function test_customizemode_doesnt_wreck_things() {
set: [
["browser.download.improvements_to_download_panel", true],
["browser.download.always_ask_before_handling_new_types", false],
["browser.download.alwaysOpenPanel", true],
],
});

Expand All @@ -278,7 +114,7 @@ add_task(async function test_customizemode_doesnt_wreck_things() {
gCustomizeMode.enter();
await customizationReadyPromise;

info("Try to open the panel (will not work, in customize mode)");
info("try to open the panel (will not work, in customize mode)");
let promise = promisePanelOpened();
DownloadsCommon.getData(window)._notifyDownloadEvent("start");
await TestUtils.waitForCondition(
Expand All @@ -298,16 +134,10 @@ add_task(async function test_customizemode_doesnt_wreck_things() {
gCustomizeMode.exit();
await afterCustomizationPromise;

// Avoid a failure on Linux where the window isn't active for some reason,
// which prevents the window's downloads panel from opening.
if (Services.focus.activeWindow != window) {
info("Main window is not active, trying to focus.");
await SimpleTest.promiseFocus(window);
is(Services.focus.activeWindow, window, "Main window should be active.");
}
DownloadsCommon.getData(window)._notifyDownloadEvent("start");
await TestUtils.waitForCondition(
() => DownloadsPanel.isPanelShowing,
is(
DownloadsPanel.isPanelShowing,
true,
"Panel state should indicate a preparation to be opened"
);
await promise;
Expand Down Expand Up @@ -626,50 +456,3 @@ add_task(async function test_alwaysOpenPanel_menuitem() {

await checkPanelOpens();
});

/**
* Verify that the downloads panel opens if the download did not open a file
* picker or UCT dialog
*/
add_task(async function test_downloads_panel_after_no_dialogs() {
await testDownloadsPanelAfterDialog({ expectPanelToOpen: true });
ok(true, "Downloads panel opened because no dialogs were opened.");
});

/**
* Verify that the downloads panel doesn't open if the download opened an
* unknown content type dialog (e.g. action = always ask)
*/
add_task(async function test_downloads_panel_after_UCT_dialog() {
await testDownloadsPanelAfterDialog({
expectPanelToOpen: false,
preferredAction: Ci.nsIHandlerInfo.alwaysAsk,
});
ok(true, "Downloads panel suppressed after UCT dialog.");
});

/**
* Verify that the downloads panel doesn't open if the download opened a file
* picker dialog (e.g. useDownloadDir = false)
*/
add_task(async function test_downloads_panel_after_file_picker_dialog() {
await testDownloadsPanelAfterDialog({
expectPanelToOpen: false,
preferredAction: Ci.nsIHandlerInfo.saveToDisk,
askWhereToSave: true,
});
ok(true, "Downloads panel suppressed after file picker dialog.");
});

/**
* Verify that the downloads panel doesn't open if the download opened both
* dialogs (e.g. default action = always ask AND useDownloadDir = false)
*/
add_task(async function test_downloads_panel_after_both_dialogs() {
await testDownloadsPanelAfterDialog({
expectPanelToOpen: false,
preferredAction: Ci.nsIHandlerInfo.alwaysAsk,
askWhereToSave: true,
});
ok(true, "Downloads panel suppressed after UCT and file picker dialogs.");
});
10 changes: 2 additions & 8 deletions toolkit/components/downloads/DownloadLegacy.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,7 @@ DownloadLegacyTransfer.prototype = {
aCancelable,
aIsPrivate,
aDownloadClassification,
aReferrerInfo,
aOpenDownloadsListOnStart
aReferrerInfo
) {
return this._nsITransferInitInternal(
aSource,
Expand All @@ -288,8 +287,7 @@ DownloadLegacyTransfer.prototype = {
aCancelable,
aIsPrivate,
aDownloadClassification,
aReferrerInfo,
aOpenDownloadsListOnStart
aReferrerInfo
);
},

Expand All @@ -305,7 +303,6 @@ DownloadLegacyTransfer.prototype = {
aIsPrivate,
aDownloadClassification,
aReferrerInfo,
aOpenDownloadsListOnStart,
aBrowsingContext,
aHandleInternally,
aHttpChannel
Expand All @@ -330,7 +327,6 @@ DownloadLegacyTransfer.prototype = {
aIsPrivate,
aDownloadClassification,
aReferrerInfo,
aOpenDownloadsListOnStart,
userContextId,
browsingContextId,
aHandleInternally,
Expand All @@ -350,7 +346,6 @@ DownloadLegacyTransfer.prototype = {
isPrivate,
aDownloadClassification,
referrerInfo,
openDownloadsListOnStart = true,
userContextId = 0,
browsingContextId = 0,
handleInternally = false,
Expand Down Expand Up @@ -402,7 +397,6 @@ DownloadLegacyTransfer.prototype = {
contentType,
launcherPath,
handleInternally,
openDownloadsListOnStart,
};

// In case the Download was classified as insecure/dangerous
Expand Down
3 changes: 1 addition & 2 deletions toolkit/mozapps/downloads/HelperAppDlg.jsm
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,7 @@ nsUnknownContentTypeDialog.prototype = {
}
}
}
// Don't pop up the downloads panel redundantly.
aLauncher.saveDestinationAvailable(result, true);
aLauncher.saveDestinationAvailable(result);
});
});
})().catch(Cu.reportError);
Expand Down
9 changes: 1 addition & 8 deletions uriloader/base/nsITransfer.idl
Original file line number Diff line number Diff line change
Expand Up @@ -62,12 +62,7 @@ interface nsITransfer : nsIWebProgressListener2 {
*
* @param aDownloadClassification Indicates wheter the download is unwanted,
* should be considered dangerous or insecure.
*
* @param aReferrerInfo The Referrer this download is started with
*
* @param aOpenDownloadsListOnStart true (default) - Open downloads panel.
* false - Only show an icon indicator.
* This parameter is optional.
*/
void init(in nsIURI aSource,
in nsIURI aSourceOriginalURI,
Expand All @@ -79,8 +74,7 @@ interface nsITransfer : nsIWebProgressListener2 {
in nsICancelable aCancelable,
in boolean aIsPrivate,
in long aDownloadClassification,
in nsIReferrerInfo aReferrerInfo,
[optional] in boolean aOpenDownloadsListOnStart);
in nsIReferrerInfo aReferrerInfo);

/**
* Same as init, but allows for passing the browsingContext
Expand All @@ -103,7 +97,6 @@ interface nsITransfer : nsIWebProgressListener2 {
in boolean aIsPrivate,
in long aDownloadClassification,
in nsIReferrerInfo aReferrerInfo,
[optional] in boolean aOpenDownloadsListOnStart,
in BrowsingContext aBrowsingContext,
in boolean aHandleInternally,
in nsIHttpChannel aHttpChannel);
Expand Down
Loading

0 comments on commit dc40d2a

Please sign in to comment.