Commit 0b49eba7 authored by yawano's avatar yawano Committed by Commit bot

Show generic file handler more modestly.

- Do not show generic file handler (file handlers which can take any type of file such as types:["*"]) as a default task.
- All file handlers are shown in context menu and menu of the combobottuon.

BUG=414566
TEST=out/Release/browser_tests --gtest_filter=GenericTask/FileManagerBrowserTest.Test/*

Review URL: https://codereview.chromium.org/700023002

Cr-Commit-Position: refs/heads/master@{#302777}
parent aeed9866
......@@ -906,6 +906,9 @@ Press any key to continue exploring.
No items match &lt;b&gt;"<ph name="SEARCH_STRING">$1<ex>abc</ex></ph>"&lt;/b&gt;
</message>
<message name="IDS_FILE_BROWSER_MORE_ACTIONS" desc="In the File Manager, the text in combobutton to show the user there exist actions which can be executed to the file.">
More actions...
</message>
<message name="IDS_FILE_BROWSER_CHANGE_DEFAULT_MENU_ITEM" desc="In the File Manager, the text in the menu item to change the default action for a particular file type.">
Change default...
</message>
......
......@@ -461,6 +461,7 @@ bool FileManagerPrivateGetStringsFunction::RunSync() {
SET_STRING("SEARCH_NO_MATCHING_FILES_HTML",
IDS_FILE_BROWSER_SEARCH_NO_MATCHING_FILES_HTML);
SET_STRING("MORE_ACTIONS", IDS_FILE_BROWSER_MORE_ACTIONS);
SET_STRING("CHANGE_DEFAULT_MENU_ITEM",
IDS_FILE_BROWSER_CHANGE_DEFAULT_MENU_ITEM);
SET_STRING("CHANGE_DEFAULT_CAPTION", IDS_FILE_BROWSER_CHANGE_DEFAULT_CAPTION);
......
......@@ -1035,6 +1035,19 @@ WRAPPED_INSTANTIATE_TEST_CASE_P(
TestParameter(IN_GUEST_MODE, "defaultActionDialogOnDownloads"),
TestParameter(NOT_IN_GUEST_MODE, "defaultActionDialogOnDrive")));
// Slow tests are disabled on debug build. http://crbug.com/327719
#if !defined(NDEBUG)
#define MAYBE_GenericTask DISABLED_GenericTask
#else
#define MAYBE_GenericTask GenericTask
#endif
WRAPPED_INSTANTIATE_TEST_CASE_P(
MAYBE_GenericTask,
FileManagerBrowserTest,
::testing::Values(
TestParameter(NOT_IN_GUEST_MODE, "genericTaskIsNotExecuted"),
TestParameter(NOT_IN_GUEST_MODE, "genericAndNonGenericTasksAreMixed")));
// Slow tests are disabled on debug build. http://crbug.com/327719
#if !defined(NDEBUG)
#define MAYBE_FolderShortcuts DISABLED_FolderShortcuts
......
......@@ -10,14 +10,17 @@
* @param {boolean} isDefault Whether the task is default or not.
* @param {string} taskId Task ID.
* @param {string} title Title of the task.
* @param {boolean=} opt_isGenericFileHandler Whether the task is a generic
* file handler.
* @constructor
*/
function FakeTask(isDefault, taskId, title) {
function FakeTask(isDefault, taskId, title, opt_isGenericFileHandler) {
this.driveApp = false;
this.iconUrl = 'chrome://theme/IDR_DEFAULT_FAVICON'; // Dummy icon
this.isDefault = isDefault;
this.taskId = taskId;
this.title = title;
this.isGenericFileHandler = opt_isGenericFileHandler || false;
Object.freeze(this);
}
......@@ -214,3 +217,38 @@ testcase.defaultActionDialogOnDownloads = function() {
testPromise(setupTaskTest(RootPath.DOWNLOADS, DOWNLOADS_FAKE_TASKS).then(
defaultActionDialog.bind(null, 'dummytaskid-2|open-with')));
};
testcase.genericTaskIsNotExecuted = function() {
var tasks = [
new FakeTask(false, 'dummytaskid|open-with', 'DummyAction1',
true /* isGenericFileHandler */),
new FakeTask(false, 'dummytaskid-2|open-with', 'DummyAction2',
true /* isGenericFileHandler */)
];
// When default task is not set, executeDefaultInternal_ in file_tasks.js
// tries to show it in a browser tab. By checking the view-in-browser task is
// executed, we check that default task is not set in this situation.
//
// See: src/ui/file_manager/file_manager/foreground/js/file_tasks.js&l=404
testPromise(setupTaskTest(RootPath.DOWNLOADS, tasks)
.then(function(windowId) {
return executeDefaultTask(
FILE_MANAGER_EXTENSIONS_ID + '|file|view-in-browser',
windowId);
}));
};
testcase.genericAndNonGenericTasksAreMixed = function() {
var tasks = [
new FakeTask(false, 'dummytaskid|open-with', 'DummyAction1',
true /* isGenericFileHandler */),
new FakeTask(false, 'dummytaskid-2|open-with', 'DummyAction2',
false /* isGenericFileHandler */),
new FakeTask(false, 'dummytaskid-3|open-with', 'DummyAction3',
true /* isGenericFileHandler */)
];
testPromise(setupTaskTest(RootPath.DOWNLOADS, tasks).then(
executeDefaultTask.bind(null, 'dummytaskid-2|open-with')));
}
......@@ -8,12 +8,15 @@
}
.buttonbar .combobutton > .action {
-webkit-padding-start: 21px;
background-position: left center;
background-repeat: no-repeat;
background-size: 16px 16px;
}
.buttonbar .combobutton > .with-icon {
-webkit-padding-start: 21px;
}
html[dir='rtl'] .buttonbar .combobutton > .action {
background-position: right center;
}
......
......@@ -288,11 +288,11 @@ function FileManager() {
this.hostedButton = null;
/**
* The menu item for doing default action.
* The menu item for doing an action.
* @type {HTMLMenuItemElement}
* @private
*/
this.defaultActionMenuItem_ = null;
this.actionMenuItem_ = null;
/**
* The button to open gear menu.
......@@ -1387,14 +1387,14 @@ var BOTTOM_MARGIN_FOR_PREVIEW_PANEL_PX = 52;
this.dialogDom_.ownerDocument.defaultView.addEventListener(
'resize', this.onResize_.bind(this));
this.defaultActionMenuItem_ = /** @type {!HTMLMenuItemElement} */
this.actionMenuItem_ = /** @type {!HTMLMenuItemElement} */
(queryRequiredElement(this.dialogDom_, '#default-action'));
this.openWithCommand_ = /** @type {cr.ui.Command} */
(this.dialogDom_.querySelector('#open-with'));
this.defaultActionMenuItem_.addEventListener('activate',
this.dispatchSelectionAction_.bind(this));
this.actionMenuItem_.addEventListener('activate',
this.onActionMenuItemActivated_.bind(this));
this.ui_.dialogFooter.initFileTypeFilter(
this.fileTypes_, this.params_.includeAllFiles);
......@@ -2276,7 +2276,8 @@ var BOTTOM_MARGIN_FOR_PREVIEW_PANEL_PX = 52;
selection.tasks.showTaskPicker(this.defaultTaskPicker,
loadTimeData.getString('CHANGE_DEFAULT_MENU_ITEM'),
strf('CHANGE_DEFAULT_CAPTION', format),
this.onDefaultTaskDone_.bind(this));
this.onDefaultTaskDone_.bind(this),
true);
}
};
......@@ -2536,6 +2537,17 @@ var BOTTOM_MARGIN_FOR_PREVIEW_PANEL_PX = 52;
return false;
};
/**
* Handles activate event of action menu item.
*
* @private
*/
FileManager.prototype.onActionMenuItemActivated_ = function() {
var tasks = this.getSelection().tasks;
if (tasks)
tasks.execute(this.actionMenuItem_.taskId);
};
/**
* Opens the suggest file dialog.
*
......@@ -3665,41 +3677,44 @@ var BOTTOM_MARGIN_FOR_PREVIEW_PANEL_PX = 52;
};
/**
* Updates default action menu item to match passed taskItem (icon,
* label and action).
* Updates action menu item to match passed task items.
*
* @param {Object} defaultItem - taskItem to match.
* @param {boolean} isMultiple - if multiple tasks available.
*/
FileManager.prototype.updateContextMenuActionItems = function(defaultItem,
isMultiple) {
if (defaultItem) {
if (defaultItem.iconType) {
this.defaultActionMenuItem_.style.backgroundImage = '';
this.defaultActionMenuItem_.setAttribute('file-type-icon',
defaultItem.iconType);
} else if (defaultItem.iconUrl) {
this.defaultActionMenuItem_.style.backgroundImage =
'url(' + defaultItem.iconUrl + ')';
* @param {Array.<Object>=} opt_items List of items.
*/
FileManager.prototype.updateContextMenuActionItems = function(opt_items) {
var items = opt_items || [];
// When only one task is available, show it as default item.
if (items.length === 1) {
var actionItem = items[0];
if (actionItem.iconType) {
this.actionMenuItem_.style.backgroundImage = '';
this.actionMenuItem_.setAttribute('file-type-icon',
actionItem.iconType);
} else if (actionItem.iconUrl) {
this.actionMenuItem_.style.backgroundImage =
'url(' + actionItem.iconUrl + ')';
} else {
this.defaultActionMenuItem_.style.backgroundImage = '';
this.actionMenuItem_.style.backgroundImage = '';
}
this.defaultActionMenuItem_.label = defaultItem.title;
this.defaultActionMenuItem_.disabled = !!defaultItem.disabled;
this.defaultActionMenuItem_.taskId = defaultItem.taskId;
this.actionMenuItem_.label = actionItem.title;
this.actionMenuItem_.disabled = !!actionItem.disabled;
this.actionMenuItem_.taskId = actionItem.taskId;
}
var defaultActionSeparator =
this.dialogDom_.querySelector('#default-action-separator');
this.actionMenuItem_.hidden = items.length !== 1;
// When multiple tasks are available, show them in open with.
this.openWithCommand_.canExecuteChange();
this.openWithCommand_.setHidden(!(defaultItem && isMultiple));
this.openWithCommand_.disabled =
defaultItem ? !!defaultItem.disabled : false;
this.openWithCommand_.setHidden(items.length < 2);
this.openWithCommand_.disabled = items.length < 2;
this.defaultActionMenuItem_.hidden = !defaultItem;
defaultActionSeparator.hidden = !defaultItem;
// Hide default action separator when there does not exist available task.
var defaultActionSeparator =
this.dialogDom_.querySelector('#default-action-separator');
defaultActionSeparator.hidden = items.length === 0;
};
/**
......
......@@ -735,7 +735,8 @@ CommandHandler.COMMANDS_['open-with'] = /** @type {Command} */ ({
'',
function(task) {
tasks.execute(task.taskId);
});
},
false);
}
},
/**
......
......@@ -278,10 +278,15 @@ FileTasks.prototype.processTasks_ = function(tasks) {
}
}
if (!this.defaultTask_ && this.tasks_.length > 0) {
// If we haven't picked a default task yet, then just pick the first one.
// This is not the preferred way we want to pick this, but better this than
// no default at all if the C++ code didn't set one.
this.defaultTask_ = this.tasks_[0];
// If we haven't picked a default task yet, then just pick the first one
// which is not generic file handler.
for (var i = 0; i < this.tasks_.length; i++) {
var task = this.tasks_[i];
if (!task.isGenericFileHandler) {
this.defaultTask_ = task;
break;
}
}
}
};
......@@ -603,6 +608,7 @@ FileTasks.prototype.mountArchivesInternal_ = function(entries) {
* @private
*/
FileTasks.prototype.display_ = function(combobutton) {
// If there does not exist available task, hide combobutton.
if (this.tasks_.length === 0) {
combobutton.hidden = true;
return;
......@@ -610,24 +616,35 @@ FileTasks.prototype.display_ = function(combobutton) {
combobutton.clear();
combobutton.hidden = false;
combobutton.defaultItem = this.createCombobuttonItem_(this.defaultTask_);
var items = this.createItems_();
// If there exist defaultTask show it on the combobutton.
if (this.defaultTask_) {
combobutton.defaultItem = this.createCombobuttonItem_(this.defaultTask_);
} else {
combobutton.defaultItem = {
label: loadTimeData.getString('MORE_ACTIONS')
};
}
if (items.length > 1) {
var defaultIdx = 0;
// If there exist 2 or more available tasks, show them in context menu
// (including defaultTask). If only one generic task is available, we also
// show it in the context menu.
var items = this.createItems_();
if (items.length > 1 || (items.length === 1 && this.defaultTask_ === null)) {
for (var j = 0; j < items.length; j++) {
combobutton.addDropDownItem(items[j]);
if (items[j].task.taskId === this.defaultTask_.taskId)
defaultIdx = j;
}
combobutton.addSeparator();
var changeDefaultMenuItem = combobutton.addDropDownItem({
label: loadTimeData.getString('CHANGE_DEFAULT_MENU_ITEM')
});
changeDefaultMenuItem.classList.add('change-default');
// If there exist non generic task (i.e. defaultTask is set), we show an
// item to change default action.
if (this.defaultTask_) {
combobutton.addSeparator();
var changeDefaultMenuItem = combobutton.addDropDownItem({
label: loadTimeData.getString('CHANGE_DEFAULT_MENU_ITEM')
});
changeDefaultMenuItem.classList.add('change-default');
}
}
};
......@@ -639,17 +656,33 @@ FileTasks.prototype.display_ = function(combobutton) {
*/
FileTasks.prototype.createItems_ = function() {
var items = [];
var title = this.defaultTask_.title + ' ' +
loadTimeData.getString('DEFAULT_ACTION_LABEL');
items.push(this.createCombobuttonItem_(this.defaultTask_, title, true));
// Create items.
for (var index = 0; index < this.tasks_.length; index++) {
var task = this.tasks_[index];
if (task !== this.defaultTask_)
if (task === this.defaultTask_) {
var title = task.title + ' ' +
loadTimeData.getString('DEFAULT_ACTION_LABEL');
items.push(this.createCombobuttonItem_(task, title, true, true));
} else {
items.push(this.createCombobuttonItem_(task));
}
}
// Sort items (Sort order: isDefault, isGenericFileHandler, label).
items.sort(function(a, b) {
// Sort by isDefaultTask.
var isDefault = (b.isDefault ? 1 : 0) - (a.isDefault ? 1 : 0);
if (isDefault !== 0)
return isDefault;
// Sort by isGenericFileHandler.
var isGenericFileHandler =
(a.isGenericFileHandler ? 1 : 0) - (b.isGenericFileHandler ? 1 : 0);
if (isGenericFileHandler !== 0)
return isGenericFileHandler;
// Sort by label.
return a.label.localeCompare(b.label);
});
......@@ -662,8 +695,7 @@ FileTasks.prototype.createItems_ = function() {
*/
FileTasks.prototype.updateMenuItem_ = function() {
this.fileManager_.updateContextMenuActionItems(this.defaultTask_,
this.tasks_.length > 1);
this.fileManager_.updateContextMenuActionItems(this.tasks_);
};
/**
......@@ -672,17 +704,21 @@ FileTasks.prototype.updateMenuItem_ = function() {
* @param {Object} task Task to convert.
* @param {string=} opt_title Title.
* @param {boolean=} opt_bold Make a menu item bold.
* @param {boolean=} opt_isDefault Mark the item as default item.
* @return {Object} Item appendable to combobutton drop-down list.
* @private
*/
FileTasks.prototype.createCombobuttonItem_ = function(task, opt_title,
opt_bold) {
opt_bold,
opt_isDefault) {
return {
label: opt_title || task.title,
iconUrl: task.iconUrl,
iconType: task.iconType,
task: task,
bold: opt_bold || false
bold: opt_bold || false,
isDefault: opt_isDefault || false,
isGenericFileHandler: task.isGenericFileHandler
};
};
......@@ -694,11 +730,18 @@ FileTasks.prototype.createCombobuttonItem_ = function(task, opt_title,
* @param {string} title Title to use.
* @param {string} message Message to use.
* @param {function(Object)} onSuccess Callback to pass selected task.
* @param {boolean=} opt_hideGenericFileHandler Whether to hide generic file
* handler or not.
*/
FileTasks.prototype.showTaskPicker = function(actionDialog, title, message,
onSuccess) {
onSuccess,
opt_hideGenericFileHandler) {
var hideGenericFileHandler = opt_hideGenericFileHandler || false;
var items = this.createItems_();
if (hideGenericFileHandler)
items = items.filter(function(item) { return !item.isGenericFileHandler; });
var defaultIdx = 0;
for (var j = 0; j < items.length; j++) {
if (items[j].task.taskId === this.defaultTask_.taskId)
......
......@@ -62,13 +62,18 @@ cr.define('cr.ui', function() {
this.actionNode_.textContent = defaultItem.label || '';
if (defaultItem.iconType) {
this.actionNode_.classList.add('with-icon');
this.actionNode_.style.backgroundImage = '';
this.actionNode_.setAttribute('file-type-icon', defaultItem.iconType);
} else if (defaultItem.iconUrl) {
this.actionNode_.classList.add('with-icon');
this.actionNode_.style.backgroundImage =
'url(' + defaultItem.iconUrl + ')';
this.actionNode_.removeAttribute('file-type-icon');
} else {
this.actionNode_.classList.remove('with-icon');
this.actionNode_.style.backgroundImage = '';
this.actionNode_.removeAttribute('file-type-icon');
}
},
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment