diff options
author | fukino@chromium.org <fukino@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-06-06 09:54:16 +0000 |
---|---|---|
committer | fukino@chromium.org <fukino@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-06-06 09:54:16 +0000 |
commit | 233a46224a1132cc6ecf965684e12f42b5592885 (patch) | |
tree | cb7bfc673a7726f1bd64fff796b84a526f471bf1 | |
parent | c05c390fce9cb137c0557e374832c45d01a657be (diff) | |
download | chromium_src-233a46224a1132cc6ecf965684e12f42b5592885.zip chromium_src-233a46224a1132cc6ecf965684e12f42b5592885.tar.gz chromium_src-233a46224a1132cc6ecf965684e12f42b5592885.tar.bz2 |
Change directory if the active list item on navigation list is changed.
We want to change current directory if the active item on navigation list is changed.
But we don't want to change current directory when listed items are spliced or permuted.
In both cases, 'change' event is dispatched from selection model and we can't distinguish these cases from event itself.
currentActiveItem_ is introduced to detect change of actual active item.
BUG=375663
TEST=go through steps in Issue 375663 and 361047
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273601
Review URL: https://codereview.chromium.org/303503004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275359 0039d316-1c4b-4281-b951-d872f2087c98
6 files changed, 465 insertions, 12 deletions
diff --git a/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc b/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc index 9571c3e..efc4d4f 100644 --- a/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc +++ b/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc @@ -900,6 +900,29 @@ INSTANTIATE_TEST_CASE_P( ::testing::Values(TestParameter(NOT_IN_GUEST_MODE, "traverseNavigationList"))); +// Unlike TEST/TEST_F, which are macros that expand to further macros, +// INSTANTIATE_TEST_CASE_P is a macro that expands directly to code that +// stringizes the arguments. As a result, macros passed as parameters (such as +// prefix or test_case_name) will not be expanded by the preprocessor. To work +// around this, indirect the macro for INSTANTIATE_TEST_CASE_P, so that the +// pre-processor will expand macros such as MAYBE_test_name before +// instantiating the test. +#define WRAPPED_INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator) \ + INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator) + +// This test is too slow to run in debug build. http://crbug.com/327719 +#if !defined(NDEBUG) +#define MAYBE_FolderShortcuts DISABLED_FolderShortcuts +#else +#define MAYBE_FolderShortcuts FolderShortcuts +#endif +WRAPPED_INSTANTIATE_TEST_CASE_P( + MAYBE_FolderShortcuts, + FileManagerBrowserTest, + ::testing::Values( + TestParameter(NOT_IN_GUEST_MODE, "traverseFolderShortcuts"), + TestParameter(NOT_IN_GUEST_MODE, "addRemoveFolderShortcuts"))); + INSTANTIATE_TEST_CASE_P( TabIndex, FileManagerBrowserTest, diff --git a/chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/folder_shortcuts.js b/chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/folder_shortcuts.js new file mode 100644 index 0000000..7282aa4 --- /dev/null +++ b/chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager/folder_shortcuts.js @@ -0,0 +1,384 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +'use strict'; + +/** + * Constants for selectors. + */ +var TREEITEM_DRIVE = '#directory-tree > div:nth-child(1) '; +var TREEITEM_A = TREEITEM_DRIVE + '> .tree-children > div:nth-child(1) '; +var TREEITEM_B = TREEITEM_A + '> .tree-children > div:nth-child(1) '; +var TREEITEM_C = TREEITEM_B + '> .tree-children > div:nth-child(1) '; +var TREEITEM_D = TREEITEM_DRIVE + '> .tree-children > div:nth-child(2) '; +var TREEITEM_E = TREEITEM_D + '> .tree-children > div:nth-child(1) '; +var EXPAND_ICON = '> .tree-row > .expand-icon'; +var VOLUME_ICON = '> .tree-row > .volume-icon'; +var EXPANDED_SUBTREE = '> .tree-children[expanded]'; + +/** + * Entry set which is used for this test. + * @type {Array.<TestEntryInfo>} + * @const + */ +var ENTRY_SET = [ + ENTRIES.directoryA, + ENTRIES.directoryB, + ENTRIES.directoryC, + ENTRIES.directoryD, + ENTRIES.directoryE, + ENTRIES.directoryF +]; + +/** + * Constants for each folders. + * @type {Array.<Object>} + * @const + */ +var DIRECTORY = { + Drive: { + contents: [ENTRIES.directoryA.getExpectedRow(), + ENTRIES.directoryD.getExpectedRow()], + name: 'Drive', + navItem: '#navigation-list-1', + treeItem: TREEITEM_DRIVE + }, + A: { + contents: [ENTRIES.directoryB.getExpectedRow()], + name: 'A', + navItem: '#navigation-list-4', + treeItem: TREEITEM_A + }, + B: { + contents: [ENTRIES.directoryC.getExpectedRow()], + name: 'B', + treeItem: TREEITEM_B + }, + C: { + contents: [], + name: 'C', + navItem: '#navigation-list-4', + treeItem: TREEITEM_C + }, + D: { + contents: [ENTRIES.directoryE.getExpectedRow()], + name: 'D', + navItem: '#navigation-list-3', + treeItem: TREEITEM_D + }, + E: { + contents: [ENTRIES.directoryF.getExpectedRow()], + name: 'E', + treeItem: TREEITEM_E + } +}; + +/** + * Opens two file manager windows. + * @return {Promise} Promise fulfilled with an array containing two window IDs. + */ +function openWindows() { + return Promise.all([ + openNewWindow(null, RootPath.DRIVE), + openNewWindow(null, RootPath.DRIVE) + ]).then(function(windowIds) { + return Promise.all([ + waitForElement(windowIds[0], '#detail-table'), + waitForElement(windowIds[1], '#detail-table') + ]).then(function() { + return windowIds; + }); + }); +} + +/** + * Expands tree item on the directory tree by clicking expand icon. + * @param {string} windowId ID of target window. + * @param {Object} directory Directory whose tree item should be expanded. + * @return {Promise} Promise fulfilled on success. + */ +function expandTreeItem(windowId, directory) { + return waitForElement( + windowId, directory.treeItem + EXPAND_ICON).then(function() { + return callRemoteTestUtil( + 'fakeMouseClick', windowId, [directory.treeItem + EXPAND_ICON]); + }).then(function(result) { + chrome.test.assertTrue(result); + return waitForElement(windowId, directory.treeItem + EXPANDED_SUBTREE); + }); +} + +/** + * Expands whole directory tree. + * @param {string} windowId ID of target window. + * @return {Promise} Promise fulfilled on success. + */ +function expandDirectoryTree(windowId) { + return expandTreeItem(windowId, DIRECTORY.Drive).then(function() { + return expandTreeItem(windowId, DIRECTORY.A); + }).then(function() { + return expandTreeItem(windowId, DIRECTORY.B); + }).then(function() { + return expandTreeItem(windowId, DIRECTORY.D); + }); +} + +/** + * Makes |directory| the current directory. + * @param {string} windowId ID of target window. + * @param {Object} directory Directory which should be a current directory. + * @return {Promise} Promise fulfilled on success. + */ +function navigateToDirectory(windowId, directory) { + return waitForElement( + windowId, directory.treeItem + VOLUME_ICON).then(function() { + return callRemoteTestUtil( + 'fakeMouseClick', windowId, [directory.treeItem + VOLUME_ICON]); + }).then(function(result) { + chrome.test.assertTrue(result); + return waitForFiles(windowId, directory.contents); + }); +} + +/** + * Creates folder shortcut to |directory|. + * The current directory must be a parent of the |directory|. + * @param {string} windowId ID of target window. + * @param {Object} directory Directory of shortcut to be created. + * @return {Promise} Promise fulfilled on success. + */ +function createShortcut(windowId, directory) { + return callRemoteTestUtil( + 'selectFile', windowId, [directory.name]).then(function(result) { + chrome.test.assertTrue(result); + return waitForElement(windowId, ['.table-row[selected]']); + }).then(function() { + return callRemoteTestUtil( + 'fakeMouseRightClick', windowId, ['.table-row[selected]']); + }).then(function(result) { + chrome.test.assertTrue(result); + return waitForElement(windowId, '#file-context-menu:not([hidden])'); + }).then(function() { + return waitForElement(windowId, '[command="#create-folder-shortcut"]'); + }).then(function() { + return callRemoteTestUtil( + 'fakeMouseClick', windowId, ['[command="#create-folder-shortcut"]']); + }).then(function(result) { + chrome.test.assertTrue(result); + return waitForElement(windowId, directory.navItem); + }); +} + +/** + * Removes folder shortcut to |directory|. + * The current directory must be a parent of the |directory|. + * @param {string} windowId ID of target window. + * @param {Object} directory Directory of shortcut ot be removed. + * @return {Promise} Promise fullfilled on success. + */ +function removeShortcut(windowId, directory) { + return callRemoteTestUtil( + 'fakeMouseRightClick', + windowId, + [directory.navItem]).then(function(result) { + chrome.test.assertTrue(result); + return waitForElement(windowId, '#roots-context-menu:not([hidden])'); + }).then(function() { + return waitForElement(windowId, '[command="#remove-folder-shortcut"]'); + }).then(function() { + return callRemoteTestUtil( + 'fakeMouseClick', windowId, ['[command="#remove-folder-shortcut"]']); + }).then(function(result) { + chrome.test.assertTrue(result); + return waitForElementLost(windowId, directory.navItem); + }); +} + +/** + * Waits until the current directory become |currentDir| and folder shortcut to + * |shortcutDir| is selected. + * @param {string} windowId ID of target window. + * @param {Object} currentDir Directory which should be a current directory. + * @param {Object} shortcutDir Directory whose shortcut should be selected. + * @return {Promise} Promise fullfilled on success. + */ +function expectSelection(windowId, currentDir, shortcutDir) { + return waitForFiles(windowId, currentDir.contents).then(function() { + return waitForElement(windowId, shortcutDir.navItem + '[selected]'); + }); +} + +/** + * Clicks folder shortcut to |directory|. + * @param {string} windowId ID of target window. + * @param {Object} directory Directory whose shortcut will be clicked. + * @return {Promise} Promise fullfilled with result of fakeMouseClick. + */ +function clickShortcut(windowId, directory) { + return waitForElement(windowId, directory.navItem).then(function() { + return callRemoteTestUtil('fakeMouseClick', windowId, [directory.navItem]); + }); +} + +/** + * Creates some shortcuts and traverse them and some other directories. + */ +testcase.traverseFolderShortcuts = function() { + var windowId; + StepsRunner.run([ + // Set up each window. + function() { + addEntries(['drive'], ENTRY_SET, this.next); + }, + function(result) { + chrome.test.assertTrue(result); + openNewWindow(null, RootPath.DRIVE).then(this.next); + }, + function(inWindowId) { + windowId = inWindowId; + waitForElement(windowId, '#detail-table').then(this.next); + }, + function() { + expandDirectoryTree(windowId).then(this.next); + }, + function() { + waitForFiles(windowId, DIRECTORY.Drive.contents).then(this.next); + }, + + // Create shortcut to D + function() { + createShortcut(windowId, DIRECTORY.D).then(this.next); + }, + + // Create sortcut to C + function() { + navigateToDirectory(windowId, DIRECTORY.B).then(this.next); + }, + function() { + createShortcut(windowId, DIRECTORY.C).then(this.next); + }, + + // Navigate to E. + // Current directory should be E. + // Shortcut to D should be selected. + function() { + navigateToDirectory(windowId, DIRECTORY.E).then(this.next); + }, + function() { + expectSelection(windowId, DIRECTORY.E, DIRECTORY.D).then(this.next); + }, + + // Click shortcut to drive. + // Current directory should be Drive root. + // Shortcut to Drive root should be selected. + function() { + clickShortcut(windowId, DIRECTORY.Drive).then(this.next); + }, + function() { + expectSelection( + windowId, DIRECTORY.Drive, DIRECTORY.Drive).then(this.next); + }, + + // Press Ctrl+4 to select 4th shortcut. + // Current directory should be D. + // Shortcut to C should be selected. + function() { + callRemoteTestUtil('fakeKeyDown', windowId, + ['#file-list', 'U+0034', true], this.next); + }, + function(result) { + chrome.test.assertTrue(result); + expectSelection(windowId, DIRECTORY.D, DIRECTORY.D).then(this.next); + }, + + // Press UP to select 3rd shortcut. + // Current directory should be C. + // Shortcut to C should be selected. + function() { + callRemoteTestUtil('fakeKeyDown', windowId, + ['#navigation-list', 'Up', false], this.next); + }, + function(result) { + chrome.test.assertTrue(result); + expectSelection(windowId, DIRECTORY.C, DIRECTORY.C).then(this.next); + }, + + function() { + checkIfNoErrorsOccured(this.next); + } + ]); +}; + +/** + * Adds and removes shortcuts from other window and check if the active + * directories and selected navigation items are correct. + */ +testcase.addRemoveFolderShortcuts = function() { + var windowId1; + var windowId2; + StepsRunner.run([ + // Set up each window. + function() { + addEntries(['drive'], ENTRY_SET, this.next); + }, + function(result) { + chrome.test.assertTrue(result); + openWindows().then(this.next); + }, + function(windowIds) { + windowId1 = windowIds[0]; + windowId2 = windowIds[1]; + expandDirectoryTree(windowId1).then(this.next); + }, + function() { + waitForFiles(windowId1, DIRECTORY.Drive.contents).then(this.next); + }, + function() { + waitForFiles(windowId2, DIRECTORY.Drive.contents).then(this.next); + }, + + // Create shortcut to D + function() { + createShortcut(windowId1, DIRECTORY.D).then(this.next); + }, + + // Navigate to E. + // Current directory should be E. + // Shortcut to E should be selected. + function() { + navigateToDirectory(windowId1, DIRECTORY.E).then(this.next); + }, + function() { + expectSelection(windowId1, DIRECTORY.E, DIRECTORY.D).then(this.next); + }, + + // Create shortcut to A in another window. + function() { + createShortcut(windowId2, DIRECTORY.A).then(this.next); + }, + + // The index of shortcut to D is changed. + // Current directory should remain E. + // Shortcut to D should keep selected. + function() { + expectSelection(windowId1, DIRECTORY.E, DIRECTORY.D).then(this.next); + }, + + // Remove shortcut to D in another window. + function() { + removeShortcut(windowId2, DIRECTORY.D).then(this.next); + }, + + // Current directory should remain E. + // Shortcut to Drive root should be selected. + function() { + expectSelection(windowId1, DIRECTORY.E, DIRECTORY.Drive).then(this.next); + }, + + function() { + checkIfNoErrorsOccured(this.next); + } + ]); +}; + diff --git a/chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager_test_manifest.json b/chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager_test_manifest.json index 0f4420e..8317645 100644 --- a/chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager_test_manifest.json +++ b/chrome/test/data/extensions/api_test/file_manager_browsertest/file_manager_test_manifest.json @@ -11,17 +11,18 @@ // namespace is defined here. "test_util.js", "file_manager/background.js", + "file_manager/copy_between_windows.js", "file_manager/create_new_folder.js", "file_manager/drive_specific.js", - "file_manager/copy_between_windows.js", "file_manager/execute_default_task.js", "file_manager/file_display.js", + "file_manager/folder_shortcuts.js", "file_manager/keyboard_operations.js", "file_manager/multi_profile.js", "file_manager/navigation_list.js", "file_manager/open_audio_files.js", - "file_manager/open_zip_files.js", "file_manager/open_video_files.js", + "file_manager/open_zip_files.js", "file_manager/restore_geometry.js", "file_manager/restore_prefs.js", "file_manager/share_dialog.js", diff --git a/chrome/test/data/extensions/api_test/file_manager_browsertest/test_util.js b/chrome/test/data/extensions/api_test/file_manager_browsertest/test_util.js index 79737ce..70449c5 100644 --- a/chrome/test/data/extensions/api_test/file_manager_browsertest/test_util.js +++ b/chrome/test/data/extensions/api_test/file_manager_browsertest/test_util.js @@ -263,6 +263,21 @@ var ENTRIES = { null, SharedOption.NONE, 'Jan 1, 2000 1:00 AM', 'C', '--', 'Folder'), + directoryD: new TestEntryInfo( + EntryType.DIRECTORY, null, 'D', + null, SharedOption.NONE, 'Jan 1, 2000 1:00 AM', + 'D', '--', 'Folder'), + + directoryE: new TestEntryInfo( + EntryType.DIRECTORY, null, 'D/E', + null, SharedOption.NONE, 'Jan 1, 2000 1:00 AM', + 'E', '--', 'Folder'), + + directoryF: new TestEntryInfo( + EntryType.DIRECTORY, null, 'D/E/F', + null, SharedOption.NONE, 'Jan 1, 2000 1:00 AM', + 'F', '--', 'Folder'), + zipArchive: new TestEntryInfo( EntryType.FILE, 'archive.zip', 'archive.zip', 'application/x-zip', SharedOption.NONE, 'Jan 1, 2014 1:00 AM', diff --git a/ui/file_manager/file_manager/background/js/test_util.js b/ui/file_manager/file_manager/background/js/test_util.js index c1cc448..62aec6e 100644 --- a/ui/file_manager/file_manager/background/js/test_util.js +++ b/ui/file_manager/file_manager/background/js/test_util.js @@ -410,6 +410,24 @@ test.util.sync.fakeMouseClick = function( }; /** + * Simulates a fake mouse click (right button, single click) on the element + * specified by |targetQuery|. + * + * @param {Window} contentWindow Window to be tested. + * @param {string} targetQuery Query to specify the element. + * @param {string=} opt_iframeQuery Optional iframe selector. + * @return {boolean} True if the event is sent to the target, false + * otherwise. + */ +test.util.sync.fakeMouseRightClick = function( + contentWindow, targetQuery, opt_iframeQuery) { + var contextMenuEvent = new MouseEvent('contextmenu', {bubbles: true}); + var result = test.util.sync.sendEvent( + contentWindow, targetQuery, contextMenuEvent, opt_iframeQuery); + return result; +}; + +/** * Simulates a fake double click event (left button) to the element specified by * |targetQuery|. * diff --git a/ui/file_manager/file_manager/foreground/js/ui/navigation_list.js b/ui/file_manager/file_manager/foreground/js/ui/navigation_list.js index 4368c9b..8de253d 100644 --- a/ui/file_manager/file_manager/foreground/js/ui/navigation_list.js +++ b/ui/file_manager/file_manager/foreground/js/ui/navigation_list.js @@ -201,6 +201,9 @@ NavigationList.prototype.decorate = function(volumeManager, directoryModel) { this.scrollBar_ = new ScrollBar(); this.scrollBar_.initialize(this.parentNode, this); + // Keeps track of selected model item to detect if it is changed actually. + this.currentModelItem_ = null; + // Overriding default role 'list' set by cr.ui.List.decorate() to 'listbox' // role for better accessibility on ChromeOS. this.setAttribute('role', 'listbox'); @@ -287,6 +290,7 @@ NavigationList.prototype.selectByIndex = function(index) { if (index < 0 || index > this.dataModel.length - 1) return false; + this.selectionModel.selectedIndex = index; this.activateModelItem_(this.dataModel.item(index)); return true; }; @@ -299,16 +303,11 @@ NavigationList.prototype.selectByIndex = function(index) { */ NavigationList.prototype.activateModelItem_ = function(modelItem) { var onEntryResolved = function(entry) { - // If the root item of active directory was same as newly activated - // root item, keep the active directory as it was. - var currentDir = this.directoryModel_.getCurrentDirEntry(); - if (util.isSameEntry(entry, currentDir) || - util.isDescendantEntry(entry, currentDir)) { - return; + // Changes directory to the model item's root directory if needed. + if (!util.isSameEntry(this.directoryModel_.getCurrentDirEntry(), entry)) { + metrics.recordUserAction('FolderShortcut.Navigate'); + this.directoryModel_.changeDirectoryEntry(entry); } - - metrics.recordUserAction('FolderShortcut.Navigate'); - this.directoryModel_.changeDirectoryEntry(entry); }.bind(this); if (modelItem.isVolume) { @@ -350,12 +349,25 @@ NavigationList.prototype.onBeforeSelectionChange_ = function(event) { * @private */ NavigationList.prototype.onSelectionChange_ = function(event) { + var index = this.selectionModel.selectedIndex; + if (index < 0 || index > this.dataModel.length - 1) + return; + + // If the selected model item is not changed actually, we don't change the + // current directory even if the selected index is changed. + var modelItem = this.dataModel.item(index); + if (modelItem === this.currentModelItem_) + return; + + // Remembers the selected model item. + this.currentModelItem_ = modelItem; + // This handler is invoked even when the navigation list itself changes the // selection. In such case, we shouldn't handle the event. if (this.dontHandleSelectionEvent_) return; - this.selectByIndex(this.selectionModel.selectedIndex); + this.activateModelItem_(modelItem); }; /** |