Bug 1638156 - Allow key modifiers to determine how/'where' PDFs are opened when possible. r=mak

Differential Revision: https://phabricator.services.mozilla.com/D75870
This commit is contained in:
Sam Foster 2020-05-28 01:13:19 +00:00
parent 42b26c66a8
commit 5d4187bff2
9 changed files with 315 additions and 71 deletions

View File

@ -431,20 +431,21 @@ var DownloadsCommon = {
*
* @param downloadProperties
* A Download object or the initial properties of a serialized download
* @param options.openWhere
* Optional string indicating how to handle opening a download target file URI.
* One of "window", "tab", "tabshifted".
* @return {Promise}
* @resolves When the instruction to launch the file has been
* successfully given to the operating system or handled internally
* @rejects JavaScript exception if there was an error trying to launch
* the file.
*/
async openDownload(download) {
// TODO: accept an event and construct an options object here with any key modifiers
async openDownload(download, options) {
// some download objects got serialized and need reconstituting
if (typeof download.launch !== "function") {
download = await Downloads.createDownload(download);
}
return download.launch().catch(ex => Cu.reportError(ex));
return download.launch(options).catch(ex => Cu.reportError(ex));
},
/**

View File

@ -10,36 +10,15 @@ const { XPCOMUtils } = ChromeUtils.import(
"resource://gre/modules/XPCOMUtils.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"AppConstants",
"resource://gre/modules/AppConstants.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"Downloads",
"resource://gre/modules/Downloads.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"DownloadsCommon",
"resource:///modules/DownloadsCommon.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"DownloadsViewUI",
"resource:///modules/DownloadsViewUI.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"FileUtils",
"resource://gre/modules/FileUtils.jsm"
);
ChromeUtils.defineModuleGetter(
this,
"PlacesUtils",
"resource://gre/modules/PlacesUtils.jsm"
);
XPCOMUtils.defineLazyModuleGetters(this, {
AppConstants: "resource://gre/modules/AppConstants.jsm",
BrowserWindowTracker: "resource:///modules/BrowserWindowTracker.jsm",
Downloads: "resource://gre/modules/Downloads.jsm",
DownloadsCommon: "resource:///modules/DownloadsCommon.jsm",
DownloadsViewUI: "resource:///modules/DownloadsViewUI.jsm",
FileUtils: "resource://gre/modules/FileUtils.jsm",
PlacesUtils: "resource://gre/modules/PlacesUtils.jsm",
});
let gPanelViewInstances = new WeakMap();
const kRefreshBatchSize = 10;
@ -392,7 +371,7 @@ class DownloadsSubview extends DownloadsViewUI.BaseView {
* @param {DOMMouseEvent} event
*/
static onClick(event) {
// Middle clicks fall through and are regarded as left clicks.
// Handle left & middle clicks with any key modifiers
if (event.button > 1) {
return;
}
@ -407,6 +386,7 @@ class DownloadsSubview extends DownloadsViewUI.BaseView {
let item = button.closest(".subviewbutton.download");
let command = "downloadsCmd_open";
let openWhere = "current";
if (button.classList.contains("action-button")) {
command = item.hasAttribute("canShow")
? "downloadsCmd_show"
@ -419,9 +399,17 @@ class DownloadsSubview extends DownloadsViewUI.BaseView {
}
item = button.parentNode._anchorNode;
}
if (
command == "downloadsCmd_open" &&
(event.shiftKey || event.ctrlKey || event.metaKey || event.button == 1)
) {
// We adjust the command for supported modifiers to suggest where the download may
// be opened.
let topWindow = BrowserWindowTracker.getTopWindow();
openWhere = topWindow.whereToOpenLink(event, false, true);
}
if (item && item._shell.isCommandEnabled(command)) {
item._shell[command]();
item._shell[command](openWhere);
}
}
}

View File

@ -707,6 +707,10 @@ DownloadsViewUI.DownloadElementShell.prototype = {
case "downloadsCmd_cancel":
return this.download.hasPartialData || !this.download.stopped;
case "downloadsCmd_open":
case "downloadsCmd_open:current":
case "downloadsCmd_open:tab":
case "downloadsCmd_open:tabshifted":
case "downloadsCmd_open:window":
// This property is false if the download did not succeed.
return this.download.target.exists;
case "downloadsCmd_show":
@ -729,8 +733,11 @@ DownloadsViewUI.DownloadElementShell.prototype = {
},
doCommand(aCommand) {
if (DownloadsViewUI.isCommandName(aCommand)) {
this[aCommand]();
// split off an optional command "modifier" into an argument,
// e.g. "downloadsCmd_open:window"
let [command, modifier] = aCommand.split(":");
if (DownloadsViewUI.isCommandName(command)) {
this[command](modifier);
}
},
@ -748,8 +755,10 @@ DownloadsViewUI.DownloadElementShell.prototype = {
this.download.confirmBlock().catch(Cu.reportError);
},
downloadsCmd_open() {
DownloadsCommon.openDownload(this.download);
downloadsCmd_open(openWhere = "current") {
DownloadsCommon.openDownload(this.download, {
openWhere,
});
},
downloadsCmd_openReferrer() {

View File

@ -127,10 +127,26 @@ HistoryDownloadElementShell.prototype = {
);
},
// Handles return keypress on the element (the keypress listener is
// set in the DownloadsPlacesView object).
doDefaultCommand() {
// Handles double-click and return keypress on the element (the keypress
// listener is set in the DownloadsPlacesView object).
doDefaultCommand(event) {
let command = this.currentDefaultCommandName;
if (
command == "downloadsCmd_open" &&
event &&
(event.shiftKey || event.ctrlKey || event.metaKey || event.button == 1)
) {
// We adjust the command for supported modifiers to suggest where the download may
// be opened.
let browserWin = BrowserWindowTracker.getTopWindow();
let openWhere = browserWin
? browserWin.whereToOpenLink(event, false, true)
: "window";
if (["window", "tabshifted", "tab"].includes(openWhere)) {
command += ":" + openWhere;
}
}
if (command && this.isCommandEnabled(command)) {
this.doCommand(command);
}
@ -714,7 +730,7 @@ DownloadsPlacesView.prototype = {
if (selectedElements.length == 1) {
let element = selectedElements[0];
if (element._shell) {
element._shell.doDefaultCommand();
element._shell.doDefaultCommand(aEvent);
}
}
} else if (aEvent.charCode == " ".charCodeAt(0)) {
@ -739,7 +755,7 @@ DownloadsPlacesView.prototype = {
let element = selectedElements[0];
if (element._shell) {
element._shell.doDefaultCommand();
element._shell.doDefaultCommand(aEvent);
}
},

View File

@ -820,11 +820,19 @@ var DownloadsView = {
target = target.parentNode;
}
let download = DownloadsView.itemForElement(target).download;
let command = "downloadsCmd_open";
if (download.hasBlockedData) {
goDoCommand("downloadsCmd_showBlockedInfo");
} else {
goDoCommand("downloadsCmd_open");
command = "downloadsCmd_showBlockedInfo";
} else if (aEvent.shiftKey || aEvent.ctrlKey || aEvent.metaKey) {
// We adjust the command for supported modifiers to suggest where the download
// may be opened
let openWhere = target.ownerGlobal.whereToOpenLink(aEvent, false, true);
if (["tab", "window", "tabshifted"].includes(openWhere)) {
command += ":" + openWhere;
}
}
DownloadsCommon.log("onDownloadClick, resolved command: ", command);
goDoCommand(command);
}
},
@ -1008,7 +1016,11 @@ class DownloadsViewItem extends DownloadsViewUI.DownloadElementShell {
isCommandEnabled(aCommand) {
switch (aCommand) {
case "downloadsCmd_open": {
case "downloadsCmd_open":
case "downloadsCmd_open:current":
case "downloadsCmd_open:tab":
case "downloadsCmd_open:tabshifted":
case "downloadsCmd_open:window": {
if (!this.download.succeeded) {
return false;
}
@ -1044,7 +1056,10 @@ class DownloadsViewItem extends DownloadsViewUI.DownloadElementShell {
doCommand(aCommand) {
if (this.isCommandEnabled(aCommand)) {
this[aCommand]();
let [command, modifier] = aCommand.split(":");
// split off an optional command "modifier" into an argument,
// e.g. "downloadsCmd_open:window"
this[command](modifier);
}
}
@ -1065,8 +1080,8 @@ class DownloadsViewItem extends DownloadsViewUI.DownloadElementShell {
this.unblockAndOpenDownload().catch(Cu.reportError);
}
downloadsCmd_open() {
super.downloadsCmd_open();
downloadsCmd_open(openWhere) {
super.downloadsCmd_open(openWhere);
// We explicitly close the panel here to give the user the feedback that
// their click has been received, and we're handling the action.
@ -1139,7 +1154,10 @@ var DownloadsViewController = {
if (!DownloadsViewUI.isCommandName(aCommand)) {
return false;
}
if (!(aCommand in this) && !(aCommand in DownloadsViewItem.prototype)) {
// Strip off any :modifier suffix before checking if the command name is
// a method on our view
let [command] = aCommand.split(":");
if (!(command in this) && !(command in DownloadsViewItem.prototype)) {
return false;
}
// The currently supported commands depend on whether the blocked subview is

View File

@ -14,6 +14,10 @@
<command id="downloadsCmd_chooseOpen"/>
<command id="downloadsCmd_confirmBlock"/>
<command id="downloadsCmd_open"/>
<command id="downloadsCmd_open:current"/>
<command id="downloadsCmd_open:tab"/>
<command id="downloadsCmd_open:tabshifted"/>
<command id="downloadsCmd_open:window"/>
<command id="downloadsCmd_show"/>
<command id="downloadsCmd_retry"/>
<command id="downloadsCmd_openReferrer"/>

View File

@ -20,6 +20,14 @@
oncommand="goDoCommand('downloadsCmd_confirmBlock')"/>
<command id="downloadsCmd_open"
oncommand="goDoCommand('downloadsCmd_open')"/>
<command id="downloadsCmd_open:current"
oncommand="goDoCommand('downloadsCmd_open:current')"/>
<command id="downloadsCmd_open:tab"
oncommand="goDoCommand('downloadsCmd_open:tab')"/>
<command id="downloadsCmd_open:tabshifted"
oncommand="goDoCommand('downloadsCmd_open:tabshifted')"/>
<command id="downloadsCmd_open:window"
oncommand="goDoCommand('downloadsCmd_open:window')"/>
<command id="downloadsCmd_show"
oncommand="goDoCommand('downloadsCmd_show')"/>
<command id="downloadsCmd_retry"

View File

@ -21,7 +21,7 @@ const TestCases = [
expected: {
downloadCount: 1,
newWindow: false,
opensTab: true,
opensTab: false,
tabSelected: true,
},
},
@ -33,6 +33,38 @@ const TestCases = [
itemTarget.focus();
EventUtils.synthesizeKey("VK_RETURN", {}, win);
},
expected: {
downloadCount: 1,
newWindow: false,
opensTab: false,
tabSelected: true,
},
},
{
name: "Download panel, open in new window",
whichUI: "downloadPanel",
itemSelector: "#downloadsListBox richlistitem .downloadMainArea",
async userEvents(itemTarget, win) {
EventUtils.synthesizeMouseAtCenter(itemTarget, { shiftKey: true }, win);
},
expected: {
downloadCount: 1,
newWindow: true,
opensTab: false,
tabSelected: true,
},
},
{
name: "Download panel, open foreground tab",
whichUI: "downloadPanel",
itemSelector: "#downloadsListBox richlistitem .downloadMainArea",
async userEvents(itemTarget, win) {
EventUtils.synthesizeMouseAtCenter(
itemTarget,
{ ctrlKey: true, metaKey: true },
win
);
},
expected: {
downloadCount: 1,
newWindow: false,
@ -40,6 +72,25 @@ const TestCases = [
tabSelected: true,
},
},
{
name: "Download panel, open background tab",
whichUI: "downloadPanel",
itemSelector: "#downloadsListBox richlistitem .downloadMainArea",
async userEvents(itemTarget, win) {
EventUtils.synthesizeMouseAtCenter(
itemTarget,
{ ctrlKey: true, metaKey: true, shiftKey: true },
win
);
},
expected: {
downloadCount: 1,
newWindow: false,
opensTab: true,
tabSelected: false,
},
},
{
name: "Library all downloads dialog, default click behavior",
whichUI: "allDownloads",
@ -50,17 +101,49 @@ const TestCases = [
expected: {
downloadCount: 1,
newWindow: false,
opensTab: true,
opensTab: false,
tabSelected: true,
},
},
{
name: "Library all downloads dialog, open tab from keyboard",
name: "Library all downloads dialog, open from keyboard",
whichUI: "allDownloads",
async userEvents(itemTarget, win) {
itemTarget.focus();
EventUtils.synthesizeKey("VK_RETURN", {}, win);
},
expected: {
downloadCount: 1,
newWindow: false,
opensTab: false,
tabSelected: true,
},
},
{
name: "Library all downloads dialog, open in new window",
whichUI: "allDownloads",
async userEvents(itemTarget, win) {
// double click
await triggerDblclickOn(itemTarget, { shiftKey: true }, win);
},
expected: {
downloadCount: 1,
newWindow: true,
opensTab: false,
tabSelected: true,
},
},
{
name: "Library all downloads dialog, open foreground tab",
whichUI: "allDownloads",
async userEvents(itemTarget, win) {
// double click
await triggerDblclickOn(
itemTarget,
{ ctrlKey: true, metaKey: true },
win
);
},
expected: {
downloadCount: 1,
newWindow: false,
@ -68,6 +151,24 @@ const TestCases = [
tabSelected: true,
},
},
{
name: "Library all downloads dialog, open background tab",
whichUI: "allDownloads",
async userEvents(itemTarget, win) {
// double click
await triggerDblclickOn(
itemTarget,
{ ctrlKey: true, metaKey: true, shiftKey: true },
win
);
},
expected: {
downloadCount: 1,
newWindow: false,
opensTab: true,
tabSelected: false,
},
},
{
name: "about:downloads, default click behavior",
whichUI: "aboutDownloads",
@ -77,6 +178,42 @@ const TestCases = [
is(browser.currentURI.spec, "about:downloads");
await contentTriggerDblclickOn(itemSelector, {}, browser);
},
expected: {
downloadCount: 1,
newWindow: false,
opensTab: false,
tabSelected: true,
},
},
{
name: "about:downloads, open in new window",
whichUI: "aboutDownloads",
itemSelector: "#downloadsRichListBox richlistitem .downloadContainer",
async userEvents(itemSelector, win) {
let browser = win.gBrowser.selectedBrowser;
is(browser.currentURI.spec, "about:downloads");
await contentTriggerDblclickOn(itemSelector, { shiftKey: true }, browser);
},
expected: {
downloadCount: 1,
newWindow: true,
opensTab: false,
tabSelected: true,
},
},
{
name: "about:downloads, open in foreground tab",
whichUI: "aboutDownloads",
itemSelector: "#downloadsRichListBox richlistitem .downloadContainer",
async userEvents(itemSelector, win) {
let browser = win.gBrowser.selectedBrowser;
is(browser.currentURI.spec, "about:downloads");
await contentTriggerDblclickOn(
itemSelector,
{ ctrlKey: true, metaKey: true },
browser
);
},
expected: {
downloadCount: 1,
newWindow: false,
@ -84,6 +221,43 @@ const TestCases = [
tabSelected: true,
},
},
{
name: "about:downloads, open in background tab",
whichUI: "aboutDownloads",
itemSelector: "#downloadsRichListBox richlistitem .downloadContainer",
async userEvents(itemSelector, win) {
let browser = win.gBrowser.selectedBrowser;
is(browser.currentURI.spec, "about:downloads");
await contentTriggerDblclickOn(
itemSelector,
{ ctrlKey: true, metaKey: true, shiftKey: true },
browser
);
},
expected: {
downloadCount: 1,
newWindow: false,
opensTab: true,
tabSelected: false,
},
},
{
name: "Private download in about:downloads, opens in new private window",
whichUI: "aboutDownloads",
itemSelector: "#downloadsRichListBox richlistitem .downloadContainer",
async userEvents(itemSelector, win) {
let browser = win.gBrowser.selectedBrowser;
is(browser.currentURI.spec, "about:downloads");
await contentTriggerDblclickOn(itemSelector, { shiftKey: true }, browser);
},
isPrivate: true,
expected: {
downloadCount: 1,
newWindow: true,
opensTab: false,
tabSelected: true,
},
},
];
function triggerDblclickOn(target, modifiers = {}, win) {
@ -151,6 +325,7 @@ async function addPDFDownload(itemData) {
let download = {
source: {
url: "https://example.com/some.pdf",
isPrivate: itemData.isPrivate,
},
target: {
path: pdfFile.path,
@ -198,6 +373,7 @@ async function testOpenPDFPreview({
itemSelector,
expected,
userEvents,
isPrivate,
}) {
info("Test case: " + name);
// Wait for focus first
@ -208,14 +384,21 @@ async function testOpenPDFPreview({
info("Adding download objects");
let download = await addPDFDownload({
targetFilename: "downloaded.pdf",
isPrivate,
});
info("Got download pathname:" + download.target.path);
is(
!!download.source.isPrivate,
!!isPrivate,
`Added download is ${isPrivate ? "private" : "not private"} as expected`
);
let pdfFileURI = NetUtil.newURI(new FileUtils.File(download.target.path));
info("pdfFileURI:" + pdfFileURI.spec);
let uiWindow = window;
let initialTab = gBrowser.selectedTab;
// we never want to unload the test browser by loading the file: URI into it
let initialTab = await BrowserTestUtils.openNewForegroundTab(gBrowser);
let previewWindow = window;
let previewTab;
let previewHappened;
@ -230,7 +413,7 @@ async function testOpenPDFPreview({
anyWindow: true,
url: pdfFileURI.spec,
});
} else {
} else if (expected.opensTab) {
// wait for a tab to be opened
info("previewHappened will wait for tab with URI:" + pdfFileURI.spec);
previewHappened = BrowserTestUtils.waitForNewTab(
@ -239,6 +422,17 @@ async function testOpenPDFPreview({
false, // dont wait for load
true // any tab, not just the next one
);
} else {
info(
"previewHappened will wait to load " +
pdfFileURI.spec +
" into the current tab"
);
previewHappened = BrowserTestUtils.browserLoaded(
initialTab.linkedBrowser,
false,
pdfFileURI.spec
);
}
let itemTarget;
@ -262,16 +456,22 @@ async function testOpenPDFPreview({
itemTarget = listbox.itemChildren[0];
break;
case "aboutDownloads":
if (isPrivate) {
uiWindow = await BrowserTestUtils.openNewBrowserWindow({
private: true,
});
}
info(
"in aboutDownloads, initially there are tabs: " + gBrowser.tabs.length
"in aboutDownloads, initially there are tabs: " +
uiWindow.gBrowser.tabs.length
);
await BrowserTestUtils.openNewForegroundTab(gBrowser, "about:blank");
let browser = gBrowser.selectedBrowser;
let browser = uiWindow.gBrowser.selectedBrowser;
let downloadsLoaded = BrowserTestUtils.waitForEvent(
browser,
"InitialDownloadsLoaded",
true
);
info("Loading about:downloads");
BrowserTestUtils.loadURI(browser, "about:downloads");
info("waiting for downloadsLoaded");
await downloadsLoaded;
@ -279,12 +479,8 @@ async function testOpenPDFPreview({
}
info("Executing user events");
info(
"selectedBrowser has currentURI: " +
gBrowser.selectedBrowser.currentURI.spec
);
await userEvents(itemTarget || itemSelector, uiWindow);
info("/Executing user events");
info("Waiting for previewHappened");
let results = await previewHappened;
if (expected.newWindow) {
@ -308,6 +504,12 @@ async function testOpenPDFPreview({
"previewTab has the expected currentURI"
);
is(
PrivateBrowsingUtils.isBrowserPrivate(previewTab.linkedBrowser),
!!isPrivate,
`The preview tab was ${isPrivate ? "private" : "not private"} as expected`
);
info("cleaning up");
if (whichUI == "downloadPanel") {
DownloadsPanel.hidePanel();
@ -323,9 +525,7 @@ async function testOpenPDFPreview({
} else {
await BrowserTestUtils.removeTab(previewTab);
}
if (gBrowser.selectedTab !== initialTab) {
await BrowserTestUtils.removeTab(gBrowser.selectedTab);
}
await BrowserTestUtils.removeTab(initialTab);
}
// register the tests

View File

@ -79,7 +79,7 @@ var DownloadUIHelper = {
file,
{
chromeWindow: browserWin,
openWhere = "tab",
openWhere = "current",
isPrivate,
userContextId = 0,
browsingContextId = 0,